Fixes #933 - smarter longhand into shorthand merging.

Why:

* When merging in `background-*` longhand properties into `background`
  multiplex shorthand need to be expanded into a multiplex first.
  However all but `background-image` require their values to be copied
  instead of default ones, see https://jsfiddle.net/e8vkxyy4/2/

diff --git a/History.md b/History.md
index 83c737e..84122eb 100644 (file)
--- a/History.md
+++ b/History.md
@@ -1,6 +1,7 @@
 [4.1.0-pre / 20xx-xx-xx](https://github.com/jakubpawlowicz/clean-css/compare/4.0...HEAD)
 ==================
 
+* Fixed issue [#934](https://github.com/jakubpawlowicz/clean-css/issues/934) - smarter longhand into shorthand merging.
 * Fixed issue [#920](https://github.com/jakubpawlowicz/clean-css/issues/920) - allows skipping certain properties in level 2 optimizations.
 * Fixed issue [#916](https://github.com/jakubpawlowicz/clean-css/issues/916) - maximum number of merged selectors.
 * Fixed issue [#908](https://github.com/jakubpawlowicz/clean-css/issues/908) - improved `minify` method signature.

diff --git a/lib/optimizer/level-2/compactable.js b/lib/optimizer/level-2/compactable.js
index 5858aee..8fb5dc7 100644 (file)
--- a/lib/optimizer/level-2/compactable.js
+++ b/lib/optimizer/level-2/compactable.js
@@ -67,7 +67,8 @@ var compactable = {
     componentOf: [
       'background'
     ],
-    defaultValue: 'scroll'
+    defaultValue: 'scroll',
+    intoMultiplexMode: 'real'
   },
   'background-clip': {
     canOverride: canOverride.property.backgroundClip,
@@ -75,6 +76,7 @@ var compactable = {
       'background'
     ],
     defaultValue: 'border-box',
+    intoMultiplexMode: 'real',
     shortestValue: 'border-box'
   },
   'background-color': {
@@ -83,6 +85,7 @@ var compactable = {
       'background'
     ],
     defaultValue: 'transparent',
+    intoMultiplexMode: 'real', // otherwise real color will turn into default since color appears in last multiplex only
     multiplexLastOnly: true,
     nonMergeableValue: 'none',
     shortestValue: 'red'
@@ -92,7 +95,8 @@ var compactable = {
     componentOf: [
       'background'
     ],
-    defaultValue: 'none'
+    defaultValue: 'none',
+    intoMultiplexMode: 'default'
   },
   'background-origin': {
     canOverride: canOverride.property.backgroundOrigin,
@@ -100,6 +104,7 @@ var compactable = {
       'background'
     ],
     defaultValue: 'padding-box',
+    intoMultiplexMode: 'real',
     shortestValue: 'border-box'
   },
   'background-position': {
@@ -109,6 +114,7 @@ var compactable = {
     ],
     defaultValue: ['0', '0'],
     doubleValues: true,
+    intoMultiplexMode: 'real',
     shortestValue: '0'
   },
   'background-repeat': {
@@ -117,7 +123,8 @@ var compactable = {
       'background'
     ],
     defaultValue: ['repeat'],
-    doubleValues: true
+    doubleValues: true,
+    intoMultiplexMode: 'real'
   },
   'background-size': {
     canOverride: canOverride.property.backgroundSize,
@@ -126,6 +133,7 @@ var compactable = {
     ],
     defaultValue: ['auto'],
     doubleValues: true,
+    intoMultiplexMode: 'real',
     shortestValue: '0 0'
   },
   'bottom': {

diff --git a/lib/optimizer/level-2/properties/override-properties.js b/lib/optimizer/level-2/properties/override-properties.js
index 035a257..3749720 100644 (file)
--- a/lib/optimizer/level-2/properties/override-properties.js
+++ b/lib/optimizer/level-2/properties/override-properties.js
@@ -74,16 +74,44 @@ function overrideShorthand(property, by) {
 function turnIntoMultiplex(property, size) {
   property.multiplex = true;
 
-  for (var i = 0, l = property.components.length; i < l; i++) {
-    var component = property.components[i];
-    if (component.multiplex)
-      continue;
+  if (compactable[property.name].shorthand) {
+    turnShorthandValueIntoMultiplex(property, size);
+  } else {
+    turnLonghandValueIntoMultiplex(property, size);
+  }
+}
+
+function turnShorthandValueIntoMultiplex(property, size) {
+  var component;
+  var i, l;
 
-    var value = component.value.slice(0);
+  for (i = 0, l = property.components.length; i < l; i++) {
+    component = property.components[i];
 
-    for (var j = 1; j < size; j++) {
-      component.value.push([Token.PROPERTY_VALUE, Marker.COMMA]);
-      Array.prototype.push.apply(component.value, value);
+    if (!component.multiplex) {
+      turnLonghandValueIntoMultiplex(component, size);
+    }
+  }
+}
+
+function turnLonghandValueIntoMultiplex(property, size) {
+  var withRealValue = compactable[property.name].intoMultiplexMode == 'real';
+  var withValue = withRealValue ?
+    property.value.slice(0) :
+    compactable[property.name].defaultValue;
+  var i = multiplexSize(property);
+  var j;
+  var m = withValue.length;
+
+  for (; i < size; i++) {
+    property.value.push([Token.PROPERTY_VALUE, Marker.COMMA]);
+
+    if (Array.isArray(withValue)) {
+      for (j = 0; j < m; j++) {
+        property.value.push(withRealValue ? withValue[j] : [Token.PROPERTY_VALUE, withValue[j]]);
+      }
+    } else {
+      property.value.push(withRealValue ? withValue : [Token.PROPERTY_VALUE, withValue]);
     }
   }
 }

diff --git a/test/optimizer/level-2/properties/override-properties-test.js b/test/optimizer/level-2/properties/override-properties-test.js
index 6be1910..9ea5040 100644 (file)
--- a/test/optimizer/level-2/properties/override-properties-test.js
+++ b/test/optimizer/level-2/properties/override-properties-test.js
@@ -1283,7 +1283,24 @@ vows.describe(optimizeProperties)
         ]);
       }
     },
-    'shorthand multiplex then longhand': {
+    'shorthand then longhand multiplex - background-image': {
+      'topic': function () {
+        return _optimize('p{background:url(one.png);background-repeat:no-repeat,repeat-x}');
+      },
+      'into': function (properties) {
+        assert.deepEqual(properties, [
+          [
+            'property',
+            ['property-name', 'background', [[1, 2, undefined]]],
+            ['property-value', 'url(one.png)', [[1, 13, undefined]]],
+            ['property-value', 'no-repeat', [[1, 44, undefined]]],
+            ['property-value', ','],
+            ['property-value', 'repeat-x', [[1, 54, undefined]]]
+          ]
+        ]);
+      }
+    },
+    'shorthand multiplex then longhand repeat': {
       'topic': function () {
         return _optimize('p{background:url(image.png),url(image.jpg);background-repeat:no-repeat}');
       },
@@ -1301,6 +1318,23 @@ vows.describe(optimizeProperties)
         ]);
       }
     },
+    'shorthand multiplex then longhand image': {
+      'topic': function () {
+        return _optimize('p{background:no-repeat,no-repeat;background-image:url(image.png)}');
+      },
+      'into': function (properties) {
+        assert.deepEqual(properties, [
+          [
+            'property',
+            ['property-name', 'background', [[1, 2, undefined]]],
+            ['property-value', 'url(image.png)', [[1, 50, undefined]]],
+            ['property-value', 'no-repeat', [[1, 13, undefined]]],
+            ['property-value', ','],
+            ['property-value', 'no-repeat', [[1, 23, undefined]]]
+          ]
+        ]);
+      }
+    },
     'longhand then shorthand multiplex': {
       'topic': function () {
         return _optimize('p{background-repeat:no-repeat;background:url(image.png),url(image.jpg)}');
@@ -1445,13 +1479,11 @@ vows.describe(optimizeProperties)
             ['property-name', 'background', [[1, 2, undefined]]],
             ['property-value', 'url(image.png)', [[1, 77, undefined]]],
             ['property-value', 'top', [[1, 13, undefined]]],
-            ['property-value', 'left', [[1, 17, undefined]]]
-          ],
-          [
-            'property',
-            ['property-name', 'background-repeat', [[1, 22, undefined]]],
+            ['property-value', 'left', [[1, 17, undefined]]],
             ['property-value', 'no-repeat', [[1, 40, undefined]]],
-            ['property-value', ',', [[1, 49, undefined]]],
+            ['property-value', ','],
+            ['property-value', 'top', [[1, 13, undefined]]],
+            ['property-value', 'left', [[1, 17, undefined]]],
             ['property-value', 'no-repeat', [[1, 50, undefined]]]
           ]
         ]);
@@ -1459,21 +1491,25 @@ vows.describe(optimizeProperties)
     },
     'too long into multiplex #1': {
       'topic': function () {
-        return _optimize('p{background:url(/long/image/path.png);background-repeat:no-repeat,no-repeat}');
+        return _optimize('p{background:top left / 100px 20px;background-repeat:no-repeat,no-repeat}');
       },
       'into': function (properties) {
         assert.deepEqual(properties, [
           [
             'property',
             ['property-name', 'background', [[1, 2, undefined]]],
-            ['property-value', 'url(/long/image/path.png)', [[1, 13, undefined]]]
+            ['property-value', 'top', [[1, 13, undefined]]],
+            ['property-value', 'left', [[1, 17, undefined]]],
+            ['property-value', '/'],
+            ['property-value', '100px', [[1, 24, undefined]]],
+            ['property-value', '20px', [[1, 30, undefined]]]
           ],
           [
             'property',
-            ['property-name', 'background-repeat', [[1, 39, undefined]]],
-            ['property-value', 'no-repeat', [[1, 57, undefined]]],
-            ['property-value', ',', [[1, 66, undefined]]],
-            ['property-value', 'no-repeat', [[1, 67, undefined]]]
+            ['property-name', 'background-repeat', [[1, 35, undefined]]],
+            ['property-value', 'no-repeat', [[1, 53, undefined]]],
+            ['property-value', ',', [[1, 62, undefined]]],
+            ['property-value', 'no-repeat', [[1, 63, undefined]]]
           ]
         ]);
       }