Fixes #129 - rebase urls relative to output file or to absolute path given by `root...

diff --git a/History.md b/History.md
index fa8168a..5c5fac6 100644 (file)
--- 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 ba8df43..5554efb 100755 (executable)
--- 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 f430249..bb04792 100644 (file)
--- 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 (file)
index f74b065..0000000
--- 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 (file)
index 0000000..b357470
--- /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 (file)
index 0000000..37a9aa2
--- /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 4ff5086..2a1ecd1 100644 (file)
--- 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 bd5d056..e42a822 100644 (file)
--- 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 (file)
index 0000000..248fcf5
--- /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 (file)
index 0000000..1e57cca
--- /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 (file)
index 0000000..e69de29

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 (file)
index 0000000..d03309b
--- /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 (file)
index 0000000..e69de29

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 (file)
index 0000000..e69de29

diff --git a/test/data/partials-relative/base.css b/test/data/partials-relative/base.css
new file mode 100644 (file)
index 0000000..989eae1
--- /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 68a1d8e..2348035 100644 (file)
--- 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);",