Improves longhand-into-shorthand merging.

Why:

* In case of properties which are both longhands and shorthands at the
  same time, e.g. `border-style`, the merging was dropped prematurely;
* also inverts order of property optimizations, doing merging first
  followed by overriding, which results in more optimization
  opportunities.

diff --git a/History.md b/History.md
index 76830cb..7d14cfc 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)
 ==================
 
+* Improves longhand-into-shorthand merging mechanism in complex cases as with `border-*` shorthands.
 * Fixed issue [#254](https://github.com/jakubpawlowicz/clean-css/issues/254) - adds `font` property optimizer.
 * Fixed issue [#525](https://github.com/jakubpawlowicz/clean-css/issues/525) - restores `inherit`-based merging.
 * Fixed issue [#755](https://github.com/jakubpawlowicz/clean-css/issues/755) - adds custom handling of remote requests.

diff --git a/lib/optimizer/level-2/compactable.js b/lib/optimizer/level-2/compactable.js
index b076a0d..97e7e2a 100644 (file)
--- a/lib/optimizer/level-2/compactable.js
+++ b/lib/optimizer/level-2/compactable.js
@@ -374,7 +374,9 @@ var compactable = {
       canOverride.generic.color,
       canOverride.generic.color
     ]),
-    componentOf: ['border'],
+    componentOf: [
+      'border'
+    ],
     components: [
       'border-top-color',
       'border-right-color',
@@ -585,6 +587,9 @@ var compactable = {
       canOverride.generic.unit,
       canOverride.generic.unit
     ]),
+    componentOf: [
+      'border'
+    ],
     components: [
       'border-top-width',
       'border-right-width',

diff --git a/lib/optimizer/level-2/properties/merge-into-shorthands.js b/lib/optimizer/level-2/properties/merge-into-shorthands.js
index bce4ed8..2f27a50 100644 (file)
--- a/lib/optimizer/level-2/properties/merge-into-shorthands.js
+++ b/lib/optimizer/level-2/properties/merge-into-shorthands.js
@@ -52,6 +52,37 @@ function overridable(components, name, validator) {
   return true;
 }
 
+function mergeable(components) {
+  var lastCount = null;
+  var currentCount;
+  var componentName;
+  var component;
+  var descriptor;
+  var values;
+
+  for (componentName in components) {
+    component = components[componentName];
+    descriptor = compactable[componentName];
+
+    if (!('restore' in descriptor)) {
+      continue;
+    }
+
+    restoreFromOptimizing([component.all[component.position]], restoreWithComponents);
+    values = descriptor.restore(component, compactable);
+
+    currentCount = values.length;
+
+    if (lastCount !== null && currentCount !== lastCount) {
+      return false;
+    }
+
+    lastCount = currentCount;
+  }
+
+  return true;
+}
+
 function mixedInherit(components) {
   var lastValue = null;
   var currentValue;
@@ -303,30 +334,64 @@ function replaceWithShorthand(properties, candidateComponents, name, validator)
   properties.push(newProperty);
 }
 
+function invalidates(candidates, shorthandName, invalidatedBy) {
+  var shorthandDescriptor = compactable[shorthandName];
+  var invalidatedByDescriptor = compactable[invalidatedBy.name];
+  var componentName;
+
+  if ('overridesShorthands' in shorthandDescriptor && shorthandDescriptor.overridesShorthands.indexOf(invalidatedBy.name) > -1) {
+    return true;
+  }
+
+  if (invalidatedByDescriptor && 'componentOf' in invalidatedByDescriptor) {
+    for (componentName in candidates[shorthandName]) {
+      if (invalidatedByDescriptor.componentOf.indexOf(componentName) > -1) {
+        return true;
+      }
+    }
+  }
+
+  return false;
+}
+
 function invalidateOrCompact(properties, position, candidates, validator) {
-  var property = properties[position];
+  var invalidatedBy = properties[position];
+  var shorthandName;
+  var shorthandDescriptor;
+  var candidateComponents;
 
-  for (var name in candidates) {
-    if (undefined !== property && name == property.name)
+  for (shorthandName in candidates) {
+    if (undefined !== invalidatedBy && shorthandName == invalidatedBy.name) {
       continue;
+    }
 
-    var descriptor = compactable[name];
-    var candidateComponents = candidates[name];
-    if (descriptor.components.length > Object.keys(candidateComponents).length) {
-      delete candidates[name];
+    shorthandDescriptor = compactable[shorthandName];
+    candidateComponents = candidates[shorthandName];
+    if (invalidatedBy && invalidates(candidates, shorthandName, invalidatedBy)) {
+      delete candidates[shorthandName];
       continue;
     }
 
-    if (mixedImportance(candidateComponents))
+    if (shorthandDescriptor.components.length > Object.keys(candidateComponents).length) {
       continue;
+    }
 
-    if (!overridable(candidateComponents, name, validator))
+    if (mixedImportance(candidateComponents)) {
       continue;
+    }
+
+    if (!overridable(candidateComponents, shorthandName, validator)) {
+      continue;
+    }
+
+    if (!mergeable(candidateComponents)) {
+      continue;
+    }
 
     if (mixedInherit(candidateComponents)) {
-      replaceWithInheritBestFit(properties, candidateComponents, name, validator);
+      replaceWithInheritBestFit(properties, candidateComponents, shorthandName, validator);
     } else {
-      replaceWithShorthand(properties, candidateComponents, name, validator);
+      replaceWithShorthand(properties, candidateComponents, shorthandName, validator);
     }
   }
 }
@@ -344,6 +409,7 @@ function mergeIntoShorthands(properties, validator) {
 
   for (i = 0, l = properties.length; i < l; i++) {
     property = properties[i];
+    descriptor = compactable[property.name];
 
     if (property.unused)
       continue;
@@ -354,13 +420,9 @@ function mergeIntoShorthands(properties, validator) {
     if (property.block)
       continue;
 
-    descriptor = compactable[property.name];
-    if (!descriptor || !descriptor.componentOf)
-      continue;
+    invalidateOrCompact(properties, i, candidates, validator);
 
-    if (property.shorthand) {
-      invalidateOrCompact(properties, i, candidates, validator);
-    } else {
+    if (descriptor && descriptor.componentOf) {
       for (j = 0, m = descriptor.componentOf.length; j < m; j++) {
         componentOf = descriptor.componentOf[j];
 

diff --git a/lib/optimizer/level-2/properties/optimize.js b/lib/optimizer/level-2/properties/optimize.js
index 6e9182f..5dc4bfb 100644 (file)
--- a/lib/optimizer/level-2/properties/optimize.js
+++ b/lib/optimizer/level-2/properties/optimize.js
@@ -25,14 +25,14 @@ function optimizeProperties(properties, withOverriding, withMerging, context) {
     }
   }
 
-  if (withOverriding && levelOptions.overrideProperties) {
-    overrideProperties(_properties, withMerging, context.options.compatibility, context.validator);
-  }
-
   if (withMerging && levelOptions.mergeIntoShorthands) {
     mergeIntoShorthands(_properties, context.validator);
   }
 
+  if (withOverriding && levelOptions.overrideProperties) {
+    overrideProperties(_properties, withMerging, context.options.compatibility, context.validator);
+  }
+
   restoreFromOptimizing(_properties, restoreWithComponents);
   removeUnused(_properties);
 }

diff --git a/test/optimizer/level-2/properties/merge-into-shorthands-test.js b/test/optimizer/level-2/properties/merge-into-shorthands-test.js
index f92c789..2efa17f 100644 (file)
--- a/test/optimizer/level-2/properties/merge-into-shorthands-test.js
+++ b/test/optimizer/level-2/properties/merge-into-shorthands-test.js
@@ -110,6 +110,82 @@ vows.describe(optimizeProperties)
         ]);
       }
     },
+    'shorthand border': {
+      'topic': function () {
+        return _optimize('.block{border-width:1px;border-color:red;border-style:dotted}');
+      },
+      'into': function (properties) {
+        assert.deepEqual(properties, [
+          [
+            'property',
+            ['property-name', 'border', [
+              [1, 7, undefined],
+              [1, 24, undefined],
+              [1, 41, undefined]
+            ]],
+            ['property-value', '1px', [[1, 20, undefined]]],
+            ['property-value', 'dotted', [[1, 54, undefined]]],
+            ['property-value', 'red', [[1, 37, undefined]]]
+          ]
+        ]);
+      }
+    },
+    'shorthand border - mixed shorthands': {
+      'topic': function () {
+        return _optimize('.block{border-width:1px;border-color:red;border-bottom:2px solid;border-style:dotted}');
+      },
+      'into': function (properties) {
+        assert.deepEqual(properties, [
+          [
+            'property',
+            ['property-name', 'border-width', [[1, 7, undefined]]],
+            ['property-value', '1px', [[1, 20, undefined]]]
+          ],
+          [
+            'property',
+            ['property-name', 'border-color', [[1, 24, undefined]]],
+            ['property-value', 'red', [[1, 37, undefined]]]
+          ],
+          [
+            'property',
+            ['property-name', 'border-bottom', [[1, 41, undefined]]],
+            ['property-value', '2px', [[1, 55, undefined]]],
+            ['property-value', 'solid', [[1, 59, undefined]]]
+          ],
+          [
+            'property',
+            ['property-name', 'border-style', [[1, 65, undefined]]],
+            ['property-value', 'dotted', [[1, 78, undefined]]]
+          ]
+        ]);
+      }
+    },
+    'shorthand border - mixed shorthand and longhands': {
+      'topic': function () {
+        return _optimize('.block{border-width:1px;border-bottom-width:2px;border-color:red;border-style:dotted}');
+      },
+      'into': function (properties) {
+        assert.deepEqual(properties, [
+          [
+            'property',
+            ['property-name', 'border-width', [[1, 7, undefined]]],
+            ['property-value', '1px', [[1, 20, undefined]]],
+            ['property-value', '1px', [[1, 20, undefined]]],
+            ['property-value', '2px', [[1, 44, undefined]]]
+          ],
+          [
+            'property',
+            ['property-name', 'border-color', [[1, 48, undefined]]],
+            ['property-value', 'red', [[1, 61, undefined]]]
+          ],
+          [
+            'property',
+            ['property-name', 'border-style', [[1, 65, undefined]]],
+            ['property-value', 'dotted', [[1, 78, undefined]]]
+          ]
+        ]);
+      }
+    },
     'shorthand border-width': {
       'topic': function () {
         return _optimize('p{border-top-width:7px;border-bottom-width:7px;border-left-width:4px;border-right-width:4px}');
@@ -130,6 +206,33 @@ vows.describe(optimizeProperties)
         ]);
       }
     },
+    'shorthand border-width - multi-valued': {
+      'topic': function () {
+        return _optimize('.block{border-width:0 0 0 1px;border-color:red;border-style:dotted}');
+      },
+      'into': function (properties) {
+        assert.deepEqual(properties, [
+          [
+            'property',
+            ['property-name', 'border-width', [[1, 7, undefined]]],
+            ['property-value', '0', [[1, 20, undefined]]],
+            ['property-value', '0', [[1, 22, undefined]]],
+            ['property-value', '0', [[1, 24, undefined]]],
+            ['property-value', '1px', [[1, 26, undefined]]]
+          ],
+          [
+            'property',
+            ['property-name', 'border-color', [[1, 30, undefined]]],
+            ['property-value', 'red', [[1, 43, undefined]]]
+          ],
+          [
+            'property',
+            ['property-name', 'border-style', [[1, 47, undefined]]],
+            ['property-value', 'dotted', [[1, 60, undefined]]]
+          ]
+        ]);
+      }
+    },
     'shorthand border-color #1': {
       'topic': function () {
         return _optimize('p{border-top-color:#9fce00;border-bottom-color:#9fce00;border-left-color:#9fce00;border-right-color:#9fce00}');