From 434d8725e6a71df5d4c059027bc2e2914d92de0f Mon Sep 17 00:00:00 2001 From: GoalSmashers Date: Tue, 3 Sep 2013 11:12:17 +0200 Subject: [PATCH] Fixes #129 - rebase urls relative to output file or to absolute path given by `root` option. --- History.md | 1 + bin/cleancss | 2 +- lib/clean.js | 4 + lib/images/relative-urls.js | 30 -------- lib/images/url-rebase.js | 24 ++++++ lib/images/url-rewriter.js | 56 ++++++++++++++ lib/imports/inliner.js | 8 +- test/binary-test.js | 76 +++++++++++++++++-- test/data/129-assets/assets/ui.css | 2 + .../components/bootstrap/css/bootstrap.css | 3 + .../components/bootstrap/images/glyphs.gif | 0 .../components/jquery-ui/css/style.css | 6 ++ .../components/jquery-ui/images/next.gif | 0 .../components/jquery-ui/images/prev.gif | 0 test/data/partials-relative/base.css | 3 + test/unit-test.js | 67 ++++++++++++++-- 16 files changed, 234 insertions(+), 48 deletions(-) delete mode 100644 lib/images/relative-urls.js create mode 100644 lib/images/url-rebase.js create mode 100644 lib/images/url-rewriter.js create mode 100644 test/data/129-assets/assets/ui.css create mode 100644 test/data/129-assets/components/bootstrap/css/bootstrap.css create mode 100644 test/data/129-assets/components/bootstrap/images/glyphs.gif create mode 100644 test/data/129-assets/components/jquery-ui/css/style.css create mode 100644 test/data/129-assets/components/jquery-ui/images/next.gif create mode 100644 test/data/129-assets/components/jquery-ui/images/prev.gif create mode 100644 test/data/partials-relative/base.css diff --git a/History.md b/History.md index fa8168a5..5c5fac66 100644 --- a/History.md +++ b/History.md @@ -5,6 +5,7 @@ * Fixed issue [#84](https://github.com/GoalSmashers/clean-css/issues/84) - support for @import with media queries. * Fixed issue [#124](https://github.com/GoalSmashers/clean-css/issues/124) - raise error on broken imports. * Fixed issue [#126](https://github.com/GoalSmashers/clean-css/issues/126) - proper CSS expressions handling. +* Fixed issue [#129](https://github.com/GoalSmashers/clean-css/issues/129) - rebasing imported urls. * Fixed issue [#130](https://github.com/GoalSmashers/clean-css/issues/130) - better code modularity. * Fixed issue [#135](https://github.com/GoalSmashers/clean-css/issues/135) - require node.js 0.8+. diff --git a/bin/cleancss b/bin/cleancss index ba8df430..5554efbd 100755 --- a/bin/cleancss +++ b/bin/cleancss @@ -55,7 +55,7 @@ if (!fromStdin && commands.args.length == 0) { // Now coerce commands into CleanCSS configuration... if (commands.output) - options.target = commands.output; + cleanOptions.target = options.target = commands.output; if (commands.removeEmpty) cleanOptions.removeEmpty = true; if (commands.keepLineBreaks) diff --git a/lib/clean.js b/lib/clean.js index f4302499..bb047922 100644 --- a/lib/clean.js +++ b/lib/clean.js @@ -12,6 +12,7 @@ var ColorLongToShortHex = require('./colors/long-to-short-hex'); var ShorthandNotations = require('./properties/shorthand-notations'); var ImportInliner = require('./imports/inliner'); +var UrlRebase = require('./images/url-rebase'); var CommentsProcessor = require('./text/comments'); var ExpressionsProcessor = require('./text/expressions'); @@ -244,6 +245,9 @@ var CleanCSS = { replace(function restoreUrls() { data = urlsProcessor.restore(data); }); + replace(function rebaseUrls() { + data = UrlRebase.process(data, options); + }); replace(function restoreFreeText() { data = freeTextProcessor.restore(data); }); diff --git a/lib/images/relative-urls.js b/lib/images/relative-urls.js deleted file mode 100644 index f74b0657..00000000 --- a/lib/images/relative-urls.js +++ /dev/null @@ -1,30 +0,0 @@ -var path = require('path'); - -module.exports = { - process: function(data, fromBase, toBase) { - var tempData = []; - var nextStart = 0; - var nextEnd = 0; - var cursor = 0; - - for (; nextEnd < data.length; ) { - nextStart = data.indexOf('url(', nextEnd); - if (nextStart == -1) - break; - nextEnd = data.indexOf(')', nextStart + 4); - if (nextEnd == -1) - break; - - tempData.push(data.substring(cursor, nextStart)); - var url = data.substring(nextStart + 4, nextEnd).replace(/['"]/g, ''); - if (url[0] != '/' && url.indexOf('data:') !== 0 && url.substring(url.length - 4) != '.css') - url = path.relative(toBase, path.join(fromBase, url)).replace(/\\/g, '/'); - tempData.push('url(' + url + ')'); - cursor = nextEnd + 1; - } - - return tempData.length > 0 ? - tempData.join('') + data.substring(cursor, data.length) : - data; - } -}; diff --git a/lib/images/url-rebase.js b/lib/images/url-rebase.js new file mode 100644 index 00000000..b3574705 --- /dev/null +++ b/lib/images/url-rebase.js @@ -0,0 +1,24 @@ +var path = require('path'); + +var UrlRewriter = require('./url-rewriter'); + +module.exports = { + process: function(data, options) { + var rebaseOpts = { + absolute: !!options.root, + relative: !options.root && !!options.target, + fromBase: options.relativeTo + }; + + if (rebaseOpts.absolute) + rebaseOpts.toBase = path.resolve(options.root); + + if (rebaseOpts.relative) + rebaseOpts.toBase = path.resolve(path.dirname(options.target)); + + if (rebaseOpts.absolute && (!rebaseOpts.fromBase || !rebaseOpts.toBase)) + return data; + + return UrlRewriter.process(data, rebaseOpts); + } +}; diff --git a/lib/images/url-rewriter.js b/lib/images/url-rewriter.js new file mode 100644 index 00000000..37a9aa29 --- /dev/null +++ b/lib/images/url-rewriter.js @@ -0,0 +1,56 @@ +var path = require('path'); + +module.exports = { + process: function(data, options) { + var tempData = []; + var nextStart = 0; + var nextEnd = 0; + var cursor = 0; + + for (; nextEnd < data.length; ) { + nextStart = data.indexOf('url(', nextEnd); + if (nextStart == -1) + break; + + nextEnd = data.indexOf(')', nextStart + 4); + if (nextEnd == -1) + break; + + tempData.push(data.substring(cursor, nextStart)); + var url = data.substring(nextStart + 4, nextEnd).replace(/['"]/g, ''); + tempData.push('url(' + this._rebased(url, options) + ')'); + cursor = nextEnd + 1; + } + + return tempData.length > 0 ? + tempData.join('') + data.substring(cursor, data.length) : + data; + }, + + _rebased: function(url, options) { + var specialUrl = url[0] == '/' || + url.substring(url.length - 4) == '.css' || + url.indexOf('data:') === 0 || + /^https?:\/\//.exec(url) !== null || + /__\w+__/.exec(url) !== null; + var rebased; + + if (specialUrl) + return url; + + if (!options.absolute && !options.relative) + throw new Error('Relative url found: \'' + url + '\' but there is no way to resolve it (hint: use `root` or `output` options)'); + + if (options.absolute) { + rebased = path + .resolve(path.join(options.fromBase, url)) + .replace(options.toBase, ''); + } else { + rebased = path.relative(options.toBase, path.join(options.fromBase, url)); + } + + return process.platform == 'win32' ? + rebased.replace(/\\/g, '/') : + rebased; + } +}; diff --git a/lib/imports/inliner.js b/lib/imports/inliner.js index 4ff50869..2a1ecd16 100644 --- a/lib/imports/inliner.js +++ b/lib/imports/inliner.js @@ -1,7 +1,7 @@ var fs = require('fs'); var path = require('path'); -var RelativeUrls = require('../images/relative-urls'); +var UrlRewriter = require('../images/url-rewriter'); module.exports = function Inliner() { var process = function(data, options) { @@ -66,7 +66,11 @@ module.exports = function Inliner() { var importedData = fs.readFileSync(fullPath, 'utf8'); var importRelativeTo = path.dirname(fullPath); - importedData = RelativeUrls.process(importedData, importRelativeTo, options._baseRelativeTo); + importedData = UrlRewriter.process(importedData, { + relative: true, + fromBase: importRelativeTo, + toBase: options._baseRelativeTo + }); var inlinedData = process(importedData, { root: options.root, diff --git a/test/binary-test.js b/test/binary-test.js index bd5d056f..e42a822f 100644 --- a/test/binary-test.js +++ b/test/binary-test.js @@ -27,6 +27,17 @@ var pipedContext = function(css, options, context) { return context; }; +var readFile = function(filename) { + return fs.readFileSync(filename, 'utf-8').replace(lineBreak, ''); +}; + +var deleteFile = function(filename) { + if (isWindows) + exec('del /q /f ' + filename); + else + exec('rm ' + filename); +}; + exports.commandsSuite = vows.describe('binary commands').addBatch({ 'no options': binaryContext('', { 'should output help': function(stdout) { @@ -95,25 +106,74 @@ exports.commandsSuite = vows.describe('binary commands').addBatch({ assert.equal(stdout, minimized); } }), - 'to file': binaryContext('-o ./reset-min.css ./test/data/reset.css', { + 'to file': binaryContext('-o ./reset1-min.css ./test/data/reset.css', { 'should give no output': function(error, stdout) { assert.equal(stdout, ''); }, 'should minimize': function() { - var minimized = fs.readFileSync('./test/data/reset-min.css', 'utf-8').replace(lineBreak, ''); - var target = fs.readFileSync('./reset-min.css', 'utf-8').replace(lineBreak, ''); + var minimized = readFile('./test/data/reset-min.css'); + var target = readFile('./reset1-min.css'); assert.equal(minimized, target); }, teardown: function() { - if (isWindows) - exec('del /q /f ./reset-min.css'); - else - exec('rm ./reset-min.css'); + deleteFile('./reset1-min.css'); } }), 'disable @import': binaryContext('-s ./test/data/imports.css', { 'should disable the import processing': function(error, stdout) { assert.equal(stdout, "@import url(./partials/one.css);@import url(./partials/two.css);.imports{color:#000}"); } - }) + }), + 'relative image paths': { + 'no root & output': binaryContext('./test/data/partials-relative/base.css', { + 'should raise error': function(error, stdout) { + assert.equal(stdout, ''); + assert.notEqual(error, null); + } + }), + 'root but no output': binaryContext('-r ./test ./test/data/partials-relative/base.css', { + 'should rewrite path relative to ./test': function(error, stdout) { + assert.equal(stdout, 'a{background:url(/data/partials/extra/down.gif) 0 0 no-repeat}'); + } + }), + 'no root but output': binaryContext('-o ./base1-min.css ./test/data/partials-relative/base.css', { + 'should rewrite path relative to current path': function() { + var minimized = readFile('./base1-min.css'); + assert.equal(minimized, 'a{background:url(test/data/partials/extra/down.gif) 0 0 no-repeat}'); + }, + teardown: function() { + deleteFile('./base1-min.css'); + } + }), + 'root and output': binaryContext('-r ./test/data -o ./base2-min.css ./test/data/partials-relative/base.css', { + 'should rewrite path relative to ./test/data/': function() { + var minimized = readFile('./base2-min.css'); + assert.equal(minimized, 'a{background:url(/partials/extra/down.gif) 0 0 no-repeat}'); + }, + teardown: function() { + deleteFile('./base2-min.css'); + } + }) + }, + 'complex import and url rebasing': { + absolute: binaryContext('-r ./test/data/129-assets ./test/data/129-assets/assets/ui.css', { + 'should rebase urls correctly': function(error, stdout) { + assert.equal(error, null); + assert.include(stdout, 'url(/components/bootstrap/images/glyphs.gif)'); + assert.include(stdout, 'url(/components/jquery-ui/images/prev.gif)'); + assert.include(stdout, 'url(/components/jquery-ui/images/next.gif)'); + } + }), + relative: binaryContext('-o ./test/data/129-assets/assets/ui.bundled.css ./test/data/129-assets/assets/ui.css', { + 'should rebase urls correctly': function() { + var minimized = readFile('./test/data/129-assets/assets/ui.bundled.css'); + assert.include(minimized, 'url(../components/bootstrap/images/glyphs.gif)'); + assert.include(minimized, 'url(../components/jquery-ui/images/prev.gif)'); + assert.include(minimized, 'url(../components/jquery-ui/images/next.gif)'); + }, + teardown: function() { + deleteFile('./test/data/129-assets/assets/ui.bundled.css'); + } + }) + } }); diff --git a/test/data/129-assets/assets/ui.css b/test/data/129-assets/assets/ui.css new file mode 100644 index 00000000..248fcf54 --- /dev/null +++ b/test/data/129-assets/assets/ui.css @@ -0,0 +1,2 @@ +@import url(../components/bootstrap/css/bootstrap.css); +@import url(../components/jquery-ui/css/style.css); diff --git a/test/data/129-assets/components/bootstrap/css/bootstrap.css b/test/data/129-assets/components/bootstrap/css/bootstrap.css new file mode 100644 index 00000000..1e57cca3 --- /dev/null +++ b/test/data/129-assets/components/bootstrap/css/bootstrap.css @@ -0,0 +1,3 @@ +.icon { + background:url(../images/glyphs.gif) 0 0 no-repeat; +} diff --git a/test/data/129-assets/components/bootstrap/images/glyphs.gif b/test/data/129-assets/components/bootstrap/images/glyphs.gif new file mode 100644 index 00000000..e69de29b diff --git a/test/data/129-assets/components/jquery-ui/css/style.css b/test/data/129-assets/components/jquery-ui/css/style.css new file mode 100644 index 00000000..d03309bc --- /dev/null +++ b/test/data/129-assets/components/jquery-ui/css/style.css @@ -0,0 +1,6 @@ +.prev { + background:url(../images/prev.gif) 0 0 no-repeat; +} +.next { + background:url(../images/next.gif) 0 0 no-repeat; +} diff --git a/test/data/129-assets/components/jquery-ui/images/next.gif b/test/data/129-assets/components/jquery-ui/images/next.gif new file mode 100644 index 00000000..e69de29b diff --git a/test/data/129-assets/components/jquery-ui/images/prev.gif b/test/data/129-assets/components/jquery-ui/images/prev.gif new file mode 100644 index 00000000..e69de29b diff --git a/test/data/partials-relative/base.css b/test/data/partials-relative/base.css new file mode 100644 index 00000000..989eae17 --- /dev/null +++ b/test/data/partials-relative/base.css @@ -0,0 +1,3 @@ +a { + background:url(../partials/extra/down.gif) 0 0 no-repeat; +} diff --git a/test/unit-test.js b/test/unit-test.js index 68a1d8e4..2348035d 100644 --- a/test/unit-test.js +++ b/test/unit-test.js @@ -624,13 +624,10 @@ vows.describe('clean-units').addBatch({ 'a{background:url("/images/long image name.png") 0 0 no-repeat}a{}a{background:url("/images/no-spaces.png") 0 0 no-repeat}', 'a{background:url("/images/long image name.png") 0 0 no-repeat}a{}a{background:url(/images/no-spaces.png) 0 0 no-repeat}' ], - 'not add a space before url\'s hash': [ - "url(\"../fonts/d90b3358-e1e2-4abb-ba96-356983a54c22.svg#d90b3358-e1e2-4abb-ba96-356983a54c22\")", - "url(../fonts/d90b3358-e1e2-4abb-ba96-356983a54c22.svg#d90b3358-e1e2-4abb-ba96-356983a54c22)" - ], + 'not add a space before url\'s hash': "url(/fonts/d90b3358-e1e2-4abb-ba96-356983a54c22.svg#d90b3358-e1e2-4abb-ba96-356983a54c22)", 'keep urls from being stripped down #1': 'a{background:url(/image-1.0.png)}', 'keep urls from being stripped down #2': "a{background:url(/image-white.png)}", - 'keep urls from being stripped down #3': "a{background:#eee url(libraries/jquery-ui-1.10.1.custom/images/ui-bg_highlight-soft_100_eeeeee_1x100.png) 50% top repeat-x}", + 'keep urls from being stripped down #3': "a{background:#eee url(/libraries/jquery-ui-1.10.1.custom/images/ui-bg_highlight-soft_100_eeeeee_1x100.png) 50% top repeat-x}", 'keep __URL__ in comments (so order is important)': '/*! __URL__ */a{}', 'strip new line in urls': [ 'a{background:url(/very/long/\ @@ -643,8 +640,64 @@ path")}', 'a{background:url(/very/long/path)}' ] }), + 'urls rewriting - no root or target': cssContext({ + 'no @import': [ + 'a{background:url(test/data/partials/extra/down.gif) 0 0 no-repeat}', + null + ], + 'relative @import': [ + '@import url(test/data/partials-relative/base.css);', + null + ], + 'absolute @import': [ + '@import url(/test/data/partials-relative/base.css);', + null + ] + }), + 'urls rewriting - root but no target': cssContext({ + 'no @import': [ + 'a{background:url(../partials/extra/down.gif) 0 0 no-repeat}', + 'a{background:url(/test/data/partials/extra/down.gif) 0 0 no-repeat}' + ], + 'relative @import': [ + '@import url(base.css);', + 'a{background:url(/test/data/partials/extra/down.gif) 0 0 no-repeat}' + ], + 'absolute @import': [ + '@import url(/test/data/partials-relative/base.css);', + 'a{background:url(/test/data/partials/extra/down.gif) 0 0 no-repeat}' + ] + }, { root: process.cwd(), relativeTo: path.join('test', 'data', 'partials-relative') }), + 'urls rewriting - no root but target': cssContext({ + 'no @import': [ + 'a{background:url(../partials/extra/down.gif) 0 0 no-repeat}', + 'a{background:url(test/data/partials/extra/down.gif) 0 0 no-repeat}' + ], + 'relative @import': [ + '@import url(base.css);', + 'a{background:url(test/data/partials/extra/down.gif) 0 0 no-repeat}' + ], + 'absolute @import': [ + '@import url(/test/data/partials-relative/base.css);', + 'a{background:url(test/data/partials/extra/down.gif) 0 0 no-repeat}' + ] + }, { target: path.join(process.cwd(), 'test.css'), relativeTo: path.join('test', 'data', 'partials-relative') }), + 'urls rewriting - root and target': cssContext({ + 'no @import': [ + 'a{background:url(../partials/extra/down.gif) 0 0 no-repeat}', + 'a{background:url(/test/data/partials/extra/down.gif) 0 0 no-repeat}' + ], + 'relative @import': [ + '@import url(base.css);', + 'a{background:url(/test/data/partials/extra/down.gif) 0 0 no-repeat}' + ], + 'absolute @import': [ + '@import url(/test/data/partials-relative/base.css);', + 'a{background:url(/test/data/partials/extra/down.gif) 0 0 no-repeat}' + ] + }, { root: process.cwd(), target: path.join(process.cwd(), 'test.css'), relativeTo: path.join('test', 'data', 'partials-relative') }), 'fonts': cssContext({ - 'keep format quotation': "@font-face{font-family:PublicVintage;src:url(./PublicVintage.otf) format('opentype')}", + 'keep format quotation': "@font-face{font-family:PublicVintage;src:url(/PublicVintage.otf) format('opentype')}", 'remove font family quotation': [ "a{font-family:\"Helvetica\",'Arial'}", "a{font-family:Helvetica,Arial}" @@ -918,7 +971,7 @@ title']", '@import url(test/data/partials/comment.css) screen and (device-height: 600px);', '@media screen and (device-height:600px){}' ] - }), + }, { root: process.cwd() }), '@import with absolute paths': cssContext({ 'of an unknown file': [ "@import url(/fake.css);", -- 2.34.1