Fixes #507 - merging longhand into many shorthands.
authorJakub Pawlowicz <contact@jakubpawlowicz.com>
Mon, 13 Apr 2015 07:16:40 +0000 (08:16 +0100)
committerJakub Pawlowicz <contact@jakubpawlowicz.com>
Mon, 13 Apr 2015 07:16:40 +0000 (08:16 +0100)
This disables such merges altogether as it is not an easy feat, since:

* Merged property may be default;
* Merged property can be merged into many shorthands resulting in
  longer value;
* There can be more properties waiting to be merged in.

Follow up in #527.

History.md
lib/properties/override-compactor.js
test/fixtures/issue-490-min.css
test/properties/override-compacting-test.js

index f56bbc6..7c0fffa 100644 (file)
@@ -14,6 +14,7 @@
 * Fixed issue [#480](https://github.com/jakubpawlowicz/clean-css/issues/480) - extracting uppercase property names.
 * Fixed issue [#490](https://github.com/jakubpawlowicz/clean-css/issues/490) - vendor prefixed multivalue `background`.
 * Fixed issue [#500](https://github.com/jakubpawlowicz/clean-css/issues/500) - merging duplicate adjacent properties.
+* Fixed issue [#507](https://github.com/jakubpawlowicz/clean-css/issues/507) - merging longhands into many shorthands.
 
 [3.1.9 / 2015-04-04](https://github.com/jakubpawlowicz/clean-css/compare/v3.1.8...v3.1.9)
 ==================
index 9d496c5..8bf5775 100644 (file)
@@ -105,6 +105,22 @@ function lengthOf(property) {
   return stringifyProperty([fakeAsArray], 0).length;
 }
 
+function moreSameShorthands(properties, startAt, name) {
+  // Since we run the main loop in `compactOverrides` backwards, at this point some
+  // properties may not be marked as unused.
+  // We should consider reverting the order if possible
+  var count = 0;
+
+  for (var i = startAt; i >= 0; i--) {
+    if (properties[i].name == name && !properties[i].unused)
+      count++;
+    if (count > 1)
+      break;
+  }
+
+  return count > 1;
+}
+
 function wouldResultInLongerValue(left, right) {
   if (!left.multiplex && !right.multiplex || left.multiplex && right.multiplex)
     return false;
@@ -181,6 +197,10 @@ function compactOverrides(properties, compatibility, validator) {
         if (right.important && !left.important)
           continue;
 
+        // Pending more clever algorithm in #527
+        if (moreSameShorthands(properties, i - 1, left.name))
+          continue;
+
         component = left.components.filter(nameMatchFilter(right))[0];
         if (everyCombination(mayOverride, component, right, validator)) {
           var disabledBackgroundSizeMerging = !compatibility.properties.backgroundSizeMerging && component.name.indexOf('background-size') > -1;
index 890adb1..f503dd0 100644 (file)
@@ -1 +1 @@
-div{background:url(image.png),-webkit-linear-gradient(160.57deg,#4393b8 0,#346aa9 100%);background:url(image.png),-moz-linear-gradient(83.68% 67.94% 160.57deg,#4393b8 0,#346aa9 100%);background:url(image.png)25% 60% no-repeat,linear-gradient(160.57deg,#4393b8 0,#346aa9 100%)25% 60% no-repeat;z-index:1;background-size:100%;height:auto;min-height:820px}
+div{background:url(image.png),-webkit-linear-gradient(160.57deg,#4393b8 0,#346aa9 100%);background:url(image.png),-moz-linear-gradient(83.68% 67.94% 160.57deg,#4393b8 0,#346aa9 100%);background:url(image.png),linear-gradient(160.57deg,#4393b8 0,#346aa9 100%);background-repeat:no-repeat;background-position:25% 60%;z-index:1;background-size:100%;height:auto;min-height:820px}
\ No newline at end of file
index 234bd86..0375486 100644 (file)
@@ -116,6 +116,35 @@ vows.describe(optimize)
         ]);
       }
     },
+    'shorthand then longhand - two shorthands - pending #527': {
+      'topic': 'p{background:-webkit-linear-gradient();background:linear-gradient();background-repeat:repeat-x}',
+      'into': function (topic) {
+        assert.deepEqual(_optimize(topic), [
+          [['background', false , false], ['-webkit-linear-gradient()']],
+          [['background', false , false], ['linear-gradient()']],
+          [['background-repeat', false , false], ['repeat-x']]
+        ]);
+      }
+    },
+    'shorthand then longhand - two shorthands and default - pending #527': {
+      'topic': 'p{background:-webkit-linear-gradient();background:linear-gradient();background-repeat:repeat}',
+      'into': function (topic) {
+        assert.deepEqual(_optimize(topic), [
+          [['background', false , false], ['-webkit-linear-gradient()']],
+          [['background', false , false], ['linear-gradient()']],
+          [['background-repeat', false , false], ['repeat']]
+        ]);
+      }
+    },
+    'shorthand then longhand - two mergeable shorthands and default - pending #527': {
+      'topic': 'p{background:__ESCAPED_URL_CLEAN_CSS0__;background:__ESCAPED_URL_CLEAN_CSS1__;background-repeat:repeat-x}',
+      'into': function (topic) {
+        assert.deepEqual(_optimize(topic), [
+          [['background', false , false], ['__ESCAPED_URL_CLEAN_CSS1__']],
+          [['background-repeat', false , false], ['repeat-x']]
+        ]);
+      }
+    },
     'shorthand then shorthand - same values': {
       'topic': 'p{background:red;background:red}',
       'into': function (topic) {
@@ -407,7 +436,7 @@ vows.describe(optimize)
         ]);
       }
     },
-    'two multiplex shorthands with vendor specific functions123': {
+    'two multiplex shorthands with vendor specific functions': {
       'topic': 'p{background:url(1.png),-webkit-linear-gradient();background:url(1.png),linear-gradient()}',
       'into': function (topic) {
         assert.deepEqual(_optimize(topic), [