Fixes #526 - shorthand overriding into a function.
authorJakub Pawlowicz <contact@jakubpawlowicz.com>
Tue, 14 Apr 2015 07:29:15 +0000 (08:29 +0100)
committerJakub Pawlowicz <contact@jakubpawlowicz.com>
Tue, 14 Apr 2015 07:29:15 +0000 (08:29 +0100)
We should not merge into a shorthand that has functions as it will
reduce understandability.

History.md
lib/properties/override-compactor.js
test/integration-test.js
test/properties/override-compacting-test.js

index 123ab8a..ef06fad 100644 (file)
@@ -17,6 +17,7 @@
 * 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.
 * Fixed issue [#508](https://github.com/jakubpawlowicz/clean-css/issues/508) - removing duplicate media queries.
+* Fixed issue [#526](https://github.com/jakubpawlowicz/clean-css/issues/526) - shorthand overriding into a function.
 
 [3.1.9 / 2015-04-04](https://github.com/jakubpawlowicz/clean-css/compare/v3.1.8...v3.1.9)
 ==================
index 8bf5775..0942dc0 100644 (file)
@@ -121,6 +121,27 @@ function moreSameShorthands(properties, startAt, name) {
   return count > 1;
 }
 
+function mergingIntoFunction(left, right, validator) {
+  for (var i = 0, l = left.components.length; i < l; i++) {
+    if (anyValue(validator.isValidFunction, left.components[i]))
+      return true;
+  }
+
+  return false;
+}
+
+function anyValue(fn, property) {
+  for (var i = 0, l = property.value.length; i < l; i++) {
+    if (property.value[i][0] == MULTIPLEX_SEPARATOR)
+      continue;
+
+    if (fn(property.value[i][0]))
+      return true;
+  }
+
+  return false;
+}
+
 function wouldResultInLongerValue(left, right) {
   if (!left.multiplex && !right.multiplex || left.multiplex && right.multiplex)
     return false;
@@ -201,6 +222,9 @@ function compactOverrides(properties, compatibility, validator) {
         if (moreSameShorthands(properties, i - 1, left.name))
           continue;
 
+        if (mergingIntoFunction(left, right, validator))
+          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 9e6b679..ba9ef9d 100644 (file)
@@ -2010,7 +2010,7 @@ title']{display:block}",
   'background position': cssContext({
     'calc as a value': [
       '*{background:white calc(100% - 10px) center no-repeat;background-image:url(test.png)}',
-      '*{background:url(test.png)calc(100% - 10px)center no-repeat #fff}'
+      '*{background:calc(100% - 10px)center no-repeat #fff;background-image:url(test.png)}'
     ]
   }),
   'background-clip': cssContext({
index 0375486..e643b9d 100644 (file)
@@ -91,14 +91,6 @@ vows.describe(optimize)
         ]);
       }
     },
-    'shorthand then longhand - color into a function': {
-      'topic': 'p{background:linear-gradient();background-color:red}',
-      'into': function (topic) {
-        assert.deepEqual(_optimize(topic, { properties: { backgroundSizeMerging: false } }), [
-          [['background', false , false], ['linear-gradient()'], ['red']]
-        ]);
-      }
-    },
     'shorthand then longhand - color into a color - with merging off': {
       'topic': 'p{background:white;background-color:red}',
       'into': function (topic) {
@@ -145,6 +137,24 @@ vows.describe(optimize)
         ]);
       }
     },
+    'shorthand then longhand - non-function into a function': {
+      'topic': 'p{background:linear-gradient();background-color:red}',
+      'into': function (topic) {
+        assert.deepEqual(_optimize(topic), [
+          [['background', false , false], ['linear-gradient()']],
+          [['background-color', false , false], ['red']]
+        ]);
+      }
+    },
+    'shorthand then longhand - function into a non-function': {
+      'topic': 'p{background:repeat-x;background-image:-webkit-linear-gradient()}',
+      'into': function (topic) {
+        assert.deepEqual(_optimize(topic), [
+          [['background', false , false], ['repeat-x']],
+          [['background-image', false , false], ['-webkit-linear-gradient()']]
+        ]);
+      }
+    },
     'shorthand then shorthand - same values': {
       'topic': 'p{background:red;background:red}',
       'into': function (topic) {