From: Jakub Pawlowicz <contact@jakubpawlowicz.com>
Date: Mon, 13 Apr 2015 07:16:40 +0000 (+0100)
Subject: Fixes #507 - merging longhand into many shorthands.
X-Git-Url: https://git.ndcode.org/public/gitweb.cgi?a=commitdiff_plain;h=2d050b0b9dd94e2210c8c8f649121d228af59c1f;p=clean-css.git

Fixes #507 - merging longhand into many shorthands.

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.
---

diff --git a/History.md b/History.md
index f56bbc68..7c0fffae 100644
--- a/History.md
+++ b/History.md
@@ -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)
 ==================
diff --git a/lib/properties/override-compactor.js b/lib/properties/override-compactor.js
index 9d496c52..8bf57757 100644
--- a/lib/properties/override-compactor.js
+++ b/lib/properties/override-compactor.js
@@ -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;
diff --git a/test/fixtures/issue-490-min.css b/test/fixtures/issue-490-min.css
index 890adb11..f503dd04 100644
--- a/test/fixtures/issue-490-min.css
+++ b/test/fixtures/issue-490-min.css
@@ -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
diff --git a/test/properties/override-compacting-test.js b/test/properties/override-compacting-test.js
index 234bd869..03754865 100644
--- a/test/properties/override-compacting-test.js
+++ b/test/properties/override-compacting-test.js
@@ -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), [