Fixes #341 - ensures output is shorter than input.

* Was not the case for multival expressions (e.g. multiple backgrounds).

diff --git a/History.md b/History.md
index bc54729..8c289de 100644 (file)
--- a/History.md
+++ b/History.md
@@ -2,6 +2,7 @@
 ==================
 
 * Makes multival operations idempotent.
+* Fixed issue [#341](https://github.com/GoalSmashers/clean-css/issues/341) - ensure output is shorter than input.
 
 [2.2.13 / 2014-08-12](https://github.com/jakubpawlowicz/clean-css/compare/v2.2.12...v2.2.13)
 

diff --git a/lib/properties/optimizer.js b/lib/properties/optimizer.js
index ce90085..fef0720 100644 (file)
--- a/lib/properties/optimizer.js
+++ b/lib/properties/optimizer.js
@@ -249,7 +249,7 @@ module.exports = function Optimizer(compatibility, aggressiveMerging) {
 
     var tokens = Token.tokenize(input);
 
-    tokens = overrideCompactor.compactOverrides(tokens, processable);
+    tokens = overrideCompactor.compactOverrides(tokens, processable, Token);
     tokens = shorthandCompactor.compactShorthands(tokens, false, processable, Token);
     tokens = shorthandCompactor.compactShorthands(tokens, true, processable, Token);
 

diff --git a/lib/properties/override-compactor.js b/lib/properties/override-compactor.js
index 6d0759e..5eaf4ec 100644 (file)
--- a/lib/properties/override-compactor.js
+++ b/lib/properties/override-compactor.js
@@ -7,7 +7,7 @@ module.exports = (function () {
     return val1 === val2;
   };
 
-  var compactOverrides = function (tokens, processable) {
+  var compactOverrides = function (tokens, processable, Token) {
     var result, can, token, t, i, ii, oldResult, matchingComponent;
 
     // Used when searching for a component that matches token
@@ -19,6 +19,23 @@ module.exports = (function () {
       return x.prop === t.prop;
     };
 
+    function willResultInShorterValue (shorthand, token) {
+      var shorthandCopy = shorthand.clone();
+      shorthandCopy.isDirty = true;
+      shorthandCopy.isShorthand = true;
+      shorthandCopy.components = [];
+
+      shorthand.components.forEach(function (component) {
+        var componentCopy = component.clone();
+        if (component.prop == token.prop)
+          componentCopy.value = token.value;
+
+        shorthandCopy.components.push(componentCopy);
+      });
+
+      return Token.getDetokenizedLength([shorthand, token]) >= Token.getDetokenizedLength([shorthandCopy]);
+    }
+
     // Go from the end and always take what the current token can't override as the new result set
     // NOTE: can't cache result.length here because it will change with every iteration
     for (result = tokens, i = 0; (ii = result.length - 1 - i) >= 0; i++) {
@@ -71,7 +88,7 @@ module.exports = (function () {
           if (can(matchingComponent.value, token.value)) {
             // The component can override the matching component in the shorthand
 
-            if (!token.isImportant || token.isImportant && matchingComponent.isImportant) {
+            if ((!token.isImportant || token.isImportant && matchingComponent.isImportant) && willResultInShorterValue(t, token)) {
               // The overriding component is non-important which means we can simply include it into the shorthand
               // NOTE: stuff that can't really be included, like inherit, is taken care of at the final step, not here
               matchingComponent.value = token.value;

diff --git a/test/unit-test.js b/test/unit-test.js
index 73b62c3..533831a 100644 (file)
--- a/test/unit-test.js
+++ b/test/unit-test.js
@@ -2032,6 +2032,9 @@ title']{display:block}",
       'a{background:url(1.png) 0 0/28px 28px}'
     ]
   }),
+  'multiple backgrounds': cssContext({
+    'should not produce longer values': 'p{background:no-repeat;background-position:100% 0,0 100%,100% 100%,50% 50%}'
+  }),
   'misc advanced': cssContext({
     'outline auto': [
       'a{outline:5px auto -webkit-focus-ring-color}',