Re-adds `background` merging based on length.

Adds merging `background` shorthand with longhands based on a result
length so it's done only if the result is shorter.

Also adds deep cloning in addition to shallow one.

diff --git a/lib/properties/clone.js b/lib/properties/clone.js
new file mode 100644 (file)
index 0000000..1f4e73c
--- /dev/null
+++ b/lib/properties/clone.js
@@ -0,0 +1,26 @@
+var wrapSingle = require('./wrap-for-optimizing').single;
+
+function deep(property) {
+  var cloned = shallow(property);
+  for (var i = property.components.length - 1; i >= 0; i--) {
+    var component = shallow(property.components[i]);
+    component.value = property.components[i].value;
+    cloned.components.unshift(component);
+  }
+
+  cloned.dirty = true;
+  cloned.value = property.value;
+
+  return cloned;
+}
+
+function shallow(property) {
+  var cloned = wrapSingle([[property.name, property.important, property.hack]]);
+  cloned.unused = false;
+  return cloned;
+}
+
+module.exports = {
+  deep: deep,
+  shallow: shallow
+};

diff --git a/lib/properties/override-compactor.js b/lib/properties/override-compactor.js
index 1e8143e..23442a5 100644 (file)
--- a/lib/properties/override-compactor.js
+++ b/lib/properties/override-compactor.js
@@ -1,6 +1,12 @@
 var canOverride = require('./can-override');
 var compactable = require('./compactable');
-var shallowClone = require('./shallow-clone');
+var deepClone = require('./clone').deep;
+var shallowClone = require('./clone').shallow;
+var restoreShorthands = require('./restore-shorthands');
+
+var stringifyProperty = require('../stringifier/one-time').property;
+
+var MULTIPLEX_SEPARATOR = ',';
 
 // Used when searching for a component that matches property
 function nameMatchFilter(to) {
@@ -29,22 +35,45 @@ function isComponentOf(shorthand, longhand) {
   return compactable[shorthand.name].components.indexOf(longhand.name) > -1;
 }
 
-function overrideSimple(property, by) {
+function overrideIntoMultiplex(property, by) {
   by.unused = true;
-  property.value = by.value;
+
+  for (var i = 0, l = property.value.length; i < l; i++) {
+    property.value[i] = by.value;
+  }
 }
 
-function overrideMultiplex(property, by) {
+function overrideByMultiplex(property, by) {
+  // FIXME: we store component multiplex values and normal multiplex property values' differently
+  // e.g [[['no-repeat']], [['no-repeat']]]
+  // vs
+  // [['no-repeat'], [','], ['no-repeat']]
+  // We should rather use the latter as it's the standard way
+
   by.unused = true;
+  property.value = [];
 
-  for (var i = 0; i < property.value.length; i++) {
-    property.value[i] = by.value;
+  for (var i = 0, propertyIndex = 0, l = by.value.length; i < l; i++) {
+    if (by.value[i] == MULTIPLEX_SEPARATOR) {
+      propertyIndex++;
+      continue;
+    }
+
+    property.value[propertyIndex] = property.value[propertyIndex] || [];
+    property.value[propertyIndex].push(by.value[i]);
   }
 }
 
+function overrideSimple(property, by) {
+  by.unused = true;
+  property.value = by.value;
+}
+
 function override(property, by) {
-  if (property.multiplex)
-    overrideMultiplex(property, by);
+  if (by.multiplex)
+    overrideByMultiplex(property, by);
+  else if (property.multiplex)
+    overrideIntoMultiplex(property, by);
   else
     overrideSimple(property, by);
 }
@@ -53,7 +82,7 @@ function overrideShorthand(property, by) {
   by.unused = true;
 
   for (var i = 0, l = property.components.length; i < l; i++) {
-    override(property.components[i], by.components[i]);
+    override(property.components[i], by.components[i], property.multiplex);
   }
 }
 
@@ -66,6 +95,68 @@ function hasInherits(property) {
   return false;
 }
 
+function turnIntoMultiplex(property, size) {
+  property.multiplex = true;
+
+  for (var i = 0, l = property.components.length; i < l; i++) {
+    var component = property.components[i];
+    var value = component.value;
+    component.value = [];
+
+    for (var j = 0; j < size; j++) {
+      component.value.push(value);
+    }
+  }
+}
+
+function multiplexSize(component) {
+  var size = 0;
+
+  for (var i = 0, l = component.value.length; i < l; i++) {
+    if (component.value[i] == MULTIPLEX_SEPARATOR)
+      size++;
+  }
+
+  return size + 1;
+}
+
+function lengthOf(property) {
+  var fakeAsArray = [[property.name]].concat(property.value);
+  return stringifyProperty([fakeAsArray], 0).length;
+}
+
+function wouldResultInLongerValue(left, right) {
+  if (!left.multiplex && !right.multiplex || left.multiplex && right.multiplex)
+    return false;
+
+  var multiplex = left.multiplex ? left : right;
+  var simple = left.multiplex ? right : left;
+  var component;
+
+  var multiplexClone = deepClone(multiplex);
+  restoreShorthands([multiplexClone]);
+
+  var simpleClone = deepClone(simple);
+  restoreShorthands([simpleClone]);
+
+  var lengthBefore = lengthOf(multiplexClone) + 1 + lengthOf(simpleClone);
+
+  if (left.multiplex) {
+    component = multiplexClone.components.filter(nameMatchFilter(simpleClone))[0];
+    overrideIntoMultiplex(component, simpleClone);
+  } else {
+    component = simpleClone.components.filter(nameMatchFilter(multiplexClone))[0];
+    turnIntoMultiplex(simpleClone, multiplexSize(multiplexClone));
+    overrideByMultiplex(component, multiplexClone);
+  }
+
+  restoreShorthands([simpleClone]);
+
+  var lengthAfter = lengthOf(simpleClone);
+
+  return lengthBefore < lengthAfter;
+}
+
 function compactOverrides(properties, compatibility) {
   var mayOverride, right, left, component;
   var i, j, k;
@@ -83,12 +174,6 @@ function compactOverrides(properties, compatibility) {
 
       if (!left.shorthand && right.shorthand && isComponentOf(right, left)) {
         // maybe `left` can be overridden by `right` which is a shorthand?
-        // TODO: this is actually more complex, as in some cases it's better to incorporate the value, e.g.
-        // background:url(...); background-repeat:no-repeat,no-repeat;
-        // background:url(...) no-repeat,no-repeat;
-        if (!right.multiplex && left.multiplex)
-          continue;
-
         if (!right.important && left.important)
           continue;
 
@@ -99,10 +184,6 @@ function compactOverrides(properties, compatibility) {
         }
       } else if (left.shorthand && !right.shorthand && isComponentOf(left, right)) {
         // maybe `right` can be pulled into `left` which is a shorthand?
-        // TODO - see above
-        if (right.multiplex && !left.multiplex)
-          continue;
-
         if (right.important && !left.important)
           continue;
 
@@ -120,6 +201,12 @@ function compactOverrides(properties, compatibility) {
           if (component.value[0][0] != right.value[0][0] && (hasInherits(left) || hasInherits(right)))
             continue;
 
+          if (wouldResultInLongerValue(left, right))
+            continue;
+
+          if (!left.multiplex && right.multiplex)
+            turnIntoMultiplex(left, multiplexSize(right));
+
           override(component, right);
           left.dirty = true;
         }

diff --git a/lib/properties/restore-shorthands.js b/lib/properties/restore-shorthands.js
index 579038b..eff2f86 100644 (file)
--- a/lib/properties/restore-shorthands.js
+++ b/lib/properties/restore-shorthands.js
@@ -7,6 +7,11 @@ function restoreShorthands(properties) {
 
     if (descriptor && descriptor.shorthand && property.dirty && !property.unused) {
       var restored = descriptor.restore(property, compactable);
+      property.value = restored;
+
+      if (!('all' in property))
+        continue;
+
       var current = property.all[property.position];
       current.splice(1, current.length - 1);
 

diff --git a/lib/properties/restore.js b/lib/properties/restore.js
index 52e2e33..61af307 100644 (file)
--- a/lib/properties/restore.js
+++ b/lib/properties/restore.js
@@ -1,4 +1,4 @@
-var shallowClone = require('./shallow-clone');
+var shallowClone = require('./clone').shallow;
 
 function background(property, compactable) {
   var components = property.components;

diff --git a/lib/properties/shallow-clone.js b/lib/properties/shallow-clone.js
deleted file mode 100644 (file)
index b810e90..0000000
--- a/lib/properties/shallow-clone.js
+++ /dev/null
@@ -1,9 +0,0 @@
-var wrapSingle = require('./wrap-for-optimizing').single;
-
-function shallowClone(property) {
-  var cloned = wrapSingle([[property.name, property.important, property.hack]]);
-  cloned.unused = false;
-  return cloned;
-}
-
-module.exports = shallowClone;

diff --git a/lib/stringifier/one-time.js b/lib/stringifier/one-time.js
index 2e8d8da..f70551d 100644 (file)
--- a/lib/stringifier/one-time.js
+++ b/lib/stringifier/one-time.js
@@ -17,6 +17,12 @@ function body(tokens) {
   return fakeContext.output.join('');
 }
 
+function property(tokens, position) {
+  var fakeContext = context();
+  helpers.property(tokens, position, true, fakeContext);
+  return fakeContext.output.join('');
+}
+
 function selectors(tokens) {
   var fakeContext = context();
   helpers.selectors(tokens, fakeContext);
@@ -31,6 +37,7 @@ function value(tokens, position) {
 
 module.exports = {
   body: body,
+  property: property,
   selectors: selectors,
   value: value
 };

diff --git a/test/fixtures/issue-466-min.css b/test/fixtures/issue-466-min.css
index c4bc93e..d0ad7ba 100644 (file)
--- a/test/fixtures/issue-466-min.css
+++ b/test/fixtures/issue-466-min.css
@@ -1 +1 @@
-div{background:url(image.png);background:linear-gradient(rgba(0,0,0,.1),rgba(0,0,0,.5)),url(image.png)}
+div{background:url(image.png);background-image:linear-gradient(rgba(0,0,0,.1),rgba(0,0,0,.5)),url(image.png)}

diff --git a/test/integration-test.js b/test/integration-test.js
index 11bf567..a0077a1 100644 (file)
--- a/test/integration-test.js
+++ b/test/integration-test.js
@@ -2083,11 +2083,6 @@ title']{display:block}",
       '.one{background:50% no-repeat}.one{background-image:url(/img.png)}',
       '.one{background:url(/img.png)50% no-repeat}'
     ],
-    // TODO: restore multiplex merging
-    // 'merging color with backgrounds': [
-    //   'p{background:red;background-image:url(1.png),url(2.png)}',
-    //   'p{background:url(1.png),url(2.png)red}'
-    // ],
     'unknown @ rule': '@unknown "test";h1{color:red}',
     'property without a value': [
       'a{color:}',

diff --git a/test/properties/override-compacting-test.js b/test/properties/override-compacting-test.js
index a3e9092..69edfb5 100644 (file)
--- a/test/properties/override-compacting-test.js
+++ b/test/properties/override-compacting-test.js
@@ -8,7 +8,7 @@ var SourceTracker = require('../../lib/utils/source-tracker');
 var Compatibility = require('../../lib/utils/compatibility');
 var addOptimizationMetadata = require('../../lib/selectors/optimization-metadata');
 
-function _optimize(source, compatibility) {
+function _optimize(source, compatibility, aggressiveMerging) {
   var tokens = new Tokenizer({
     options: {},
     sourceTracker: new SourceTracker(),
@@ -16,8 +16,13 @@ function _optimize(source, compatibility) {
   }).toTokens(source);
   compatibility = new Compatibility(compatibility).toOptions();
 
+  var options = {
+    aggressiveMerging: undefined === aggressiveMerging ? true : aggressiveMerging,
+    compatibility: compatibility,
+    shorthandCompacting: true
+  };
   addOptimizationMetadata(tokens);
-  optimize(tokens[0][1], tokens[0][2], false, { compatibility: compatibility, shorthandCompacting: true });
+  optimize(tokens[0][1], tokens[0][2], false, options);
 
   return tokens[0][2];
 }
@@ -41,23 +46,6 @@ vows.describe(optimize)
         ]);
       }
     },
-    'longhand then shorthand - multiplex then simple': {
-      'topic': 'p{background-repeat:no-repeat,no-repeat;background:__ESCAPED_URL_CLEAN_CSS0__}',
-      'into': function (topic) {
-        assert.deepEqual(_optimize(topic), [
-          [['background-repeat', false , false], ['no-repeat'], [','], ['no-repeat']],
-          [['background', false , false], ['__ESCAPED_URL_CLEAN_CSS0__']]
-        ]);
-      }
-    },
-    'longhand then shorthand - simple then multiplex': {
-      'topic': 'p{background-repeat:no-repeat;background:__ESCAPED_URL_CLEAN_CSS0__,__ESCAPED_URL_CLEAN_CSS1__}',
-      'into': function (topic) {
-        assert.deepEqual(_optimize(topic), [
-          [['background', false , false], ['__ESCAPED_URL_CLEAN_CSS0__'], [','], ['__ESCAPED_URL_CLEAN_CSS1__']]
-        ]);
-      }
-    },
     'shorthand then longhand': {
       'topic': 'p{background:__ESCAPED_URL_CLEAN_CSS0__ repeat;background-repeat:no-repeat}',
       'into': function (topic) {
@@ -83,23 +71,6 @@ vows.describe(optimize)
         ]);
       }
     },
-    'shorthand then longhand - multiple values': {
-      'topic': 'p{background:__ESCAPED_URL_CLEAN_CSS0__,__ESCAPED_URL_CLEAN_CSS1__;background-repeat:no-repeat}',
-      'into': function (topic) {
-        assert.deepEqual(_optimize(topic), [
-          [['background', false , false], ['__ESCAPED_URL_CLEAN_CSS0__'], ['no-repeat'], [','], ['__ESCAPED_URL_CLEAN_CSS1__'], ['no-repeat']]
-        ]);
-      }
-    },
-    'shorthand then longhand - single value then multi value': {
-      'topic': 'p{background:__ESCAPED_URL_CLEAN_CSS0__;background-repeat:no-repeat,no-repeat}',
-      'into': function (topic) {
-        assert.deepEqual(_optimize(topic), [
-          [['background', false , false], ['__ESCAPED_URL_CLEAN_CSS0__']],
-          [['background-repeat', false , false], ['no-repeat'], [','], ['no-repeat']]
-        ]);
-      }
-    },
     'shorthand then longhand - disabled background size merging': {
       'topic': 'p{background:__ESCAPED_URL_CLEAN_CSS0__;background-size:50%}',
       'into': function (topic) {
@@ -192,16 +163,125 @@ vows.describe(optimize)
           [['background', true , false], ['__ESCAPED_URL_CLEAN_CSS1__'], ['red']]
         ]);
       }
+    },
+    'with aggressive off': {
+      'topic': 'a{background:white;color:red;background:red}',
+      'into': function (topic) {
+        assert.deepEqual(_optimize(topic, null, false), [
+          [['background', false , false], ['red']],
+          [['color', false , false], ['red']]
+        ]);
+      }
+    }
+  })
+  .addBatch({
+    'shorthand then longhand multiplex': {
+      'topic': 'p{background:top left;background-repeat:no-repeat,no-repeat}',
+      'into': function (topic) {
+        assert.deepEqual(_optimize(topic), [
+          [['background', false , false], ['top'], ['left'], ['no-repeat'], [','], ['top'], ['left'], ['no-repeat']]
+        ]);
+      }
+    },
+    'shorthand multiplex then longhand': {
+      'topic': 'p{background:__ESCAPED_URL_CLEAN_CSS0__,__ESCAPED_URL_CLEAN_CSS1__;background-repeat:no-repeat}',
+      'into': function (topic) {
+        assert.deepEqual(_optimize(topic), [
+          [['background', false , false], ['__ESCAPED_URL_CLEAN_CSS0__'], ['no-repeat'], [','], ['__ESCAPED_URL_CLEAN_CSS1__'], ['no-repeat']]
+        ]);
+      }
+    },
+    'longhand then shorthand multiplex': {
+      'topic': 'p{background-repeat:no-repeat;background:__ESCAPED_URL_CLEAN_CSS0__,__ESCAPED_URL_CLEAN_CSS1__}',
+      'into': function (topic) {
+        assert.deepEqual(_optimize(topic), [
+          [['background', false , false], ['__ESCAPED_URL_CLEAN_CSS0__'], [','], ['__ESCAPED_URL_CLEAN_CSS1__']]
+        ]);
+      }
+    },
+    'longhand multiplex then shorthand': {
+      'topic': 'p{background-repeat:no-repeat,no-repeat;background:__ESCAPED_URL_CLEAN_CSS0__}',
+      'into': function (topic) {
+        assert.deepEqual(_optimize(topic), [
+          [['background', false , false], ['__ESCAPED_URL_CLEAN_CSS0__']]
+        ]);
+      }
+    },
+    'multiplex longhand into multiplex shorthand123': {
+      'topic': 'p{background:no-repeat,no-repeat;background-position:top left,bottom left}',
+      'into': function (topic) {
+        assert.deepEqual(_optimize(topic), [
+          [['background', false , false], ['top'], ['left'], ['no-repeat'], [','], ['bottom'], ['left'], ['no-repeat']]
+        ]);
+      }
+    },
+    'not too long into multiplex #1': {
+      'topic': 'p{background:top left;background-repeat:no-repeat,no-repeat}',
+      'into': function (topic) {
+        assert.deepEqual(_optimize(topic), [
+          [['background', false , false], ['top'], ['left'], ['no-repeat'], [','], ['top'], ['left'], ['no-repeat']]
+        ]);
+      }
+    },
+    'not too long into multiplex #2': {
+      'topic': 'p{background:repeat content-box;background-repeat:no-repeat,no-repeat}',
+      'into': function (topic) {
+        assert.deepEqual(_optimize(topic), [
+          [['background', false , false], ['no-repeat'], ['content-box'], [','], ['no-repeat'], ['content-box']]
+        ]);
+      }
+    },
+    'not too long into multiplex - twice': {
+      'topic': 'p{background:top left;background-repeat:no-repeat,no-repeat;background-image:__ESCAPED_URL_CLEAN_CSS0__,__ESCAPED_URL_CLEAN_CSS1__}',
+      'into': function (topic) {
+        assert.deepEqual(_optimize(topic), [
+          [['background', false , false], ['__ESCAPED_URL_CLEAN_CSS0__'], ['top'], ['left'], ['no-repeat'], [','], ['__ESCAPED_URL_CLEAN_CSS1__'], ['top'], ['left'], ['no-repeat']]
+        ]);
+      }
+    },
+    'not too long into multiplex - over a property': {
+      'topic': 'p{background:top left;background-repeat:no-repeat,no-repeat;background-image:__ESCAPED_URL_CLEAN_CSS0__}',
+      'into': function (topic) {
+        assert.deepEqual(_optimize(topic), [
+          [['background', false , false], ['__ESCAPED_URL_CLEAN_CSS0__'], ['top'], ['left']],
+          [['background-repeat', false , false], ['no-repeat'], [','], ['no-repeat']]
+        ]);
+      }
+    },
+    'too long into multiplex #1': {
+      'topic': 'p{background:__ESCAPED_URL_CLEAN_CSS0__;background-repeat:no-repeat,no-repeat}',
+      'into': function (topic) {
+        assert.deepEqual(_optimize(topic), [
+          [['background', false , false], ['__ESCAPED_URL_CLEAN_CSS0__']],
+          [['background-repeat', false , false], ['no-repeat'], [','], ['no-repeat']]
+        ]);
+      }
+    },
+    'too long into multiplex #2': {
+      'topic': 'p{background:content-box padding-box;background-repeat:no-repeat,no-repeat}',
+      'into': function (topic) {
+        assert.deepEqual(_optimize(topic), [
+          [['background', false , false], ['content-box'], ['padding-box']],
+          [['background-repeat', false , false], ['no-repeat'], [','], ['no-repeat']]
+        ]);
+      }
+    },
+    'too long into multiplex #3': {
+      'topic': 'p{background:top left / 20px 20px;background-repeat:no-repeat,no-repeat}',
+      'into': function (topic) {
+        assert.deepEqual(_optimize(topic), [
+          [['background', false , false], ['top'], ['left'], ['/'], ['20px'], ['20px']],
+          [['background-repeat', false , false], ['no-repeat'], [','], ['no-repeat']]
+        ]);
+      }
+    },
+    'background color into background': {
+      'topic': 'p{background:red;background-repeat:__ESCAPED_URL_CLEAN_CSS0__,__ESCAPED_URL_CLEAN_CSS1__}',
+      'into': function (topic) {
+        assert.deepEqual(_optimize(topic), [
+          [['background', false , false], ['__ESCAPED_URL_CLEAN_CSS0__'], ['red'], [','], ['__ESCAPED_URL_CLEAN_CSS1__'], ['red']],
+        ]);
+      }
     }
-
-      // 'aggressive off - overriddable': {
-      //   'topic': 'a{background:white;color:red;background:red}',
-      //   'into': function (topic) {
-      //     assert.deepEqual(optimize(topic, false, false), [
-      //       [['color', false , false], ['red']],
-      //       [['background', false , false], ['red']]
-      //     ]);
-      //   }
-      // }
   })
   .export(module);

diff --git a/test/properties/restore-shorthands-test.js b/test/properties/restore-shorthands-test.js
index 07670cd..7a85254 100644 (file)
--- a/test/properties/restore-shorthands-test.js
+++ b/test/properties/restore-shorthands-test.js
@@ -3,6 +3,7 @@ var assert = require('assert');
 
 var wrapForOptimizing = require('../../lib/properties/wrap-for-optimizing').all;
 var populateComponents = require('../../lib/properties/populate-components');
+var shallowClone = require('../../lib/properties/clone').shallow;
 
 var restoreShorthands = require('../../lib/properties/restore-shorthands');
 
@@ -36,6 +37,39 @@ vows.describe(restoreShorthands)
       'is same as source': function (properties) {
         assert.deepEqual(properties, ['/*comment */', [['background', false, false], ['url(image.png)']]]);
       }
+    },
+    'values': {
+      'topic': function () {
+        var properties = [[['background', false, false], ['url(image.png)']]];
+        var _properties = wrapForOptimizing(properties);
+        populateComponents(_properties);
+
+        _properties[0].value = [];
+        _properties[0].dirty = true;
+
+        restoreShorthands(_properties);
+        return _properties;
+      },
+      'updates value': function (_properties) {
+        assert.deepEqual(_properties[0].value, [['url(image.png)']]);
+      }
+    },
+    'in cloned without reference to `all`': {
+      'topic': function () {
+        var properties = [[['background', false, false], ['url(image.png)']]];
+        var _properties = wrapForOptimizing(properties);
+        populateComponents(_properties);
+
+        var cloned = shallowClone(_properties[0]);
+        cloned.components = _properties[0].components;
+        cloned.dirty = true;
+
+        restoreShorthands([cloned]);
+        return cloned;
+      },
+      'does not fail': function (cloned) {
+        assert.deepEqual(cloned.value, [['url(image.png)']]);
+      }
     }
   })
   .export(module);