Fixes overriding shorthands by shorthands.

Previously a more understandable value could not override less
understandable one, e.g. function was not overridden by a unit or a color.

diff --git a/lib/properties/override-compactor.js b/lib/properties/override-compactor.js
index a7a682c..e93e040 100644 (file)
--- a/lib/properties/override-compactor.js
+++ b/lib/properties/override-compactor.js
@@ -229,7 +229,9 @@ function compactOverrides(properties, compatibility, validator) {
           var rightComponent = right.components[k];
 
           mayOverride = compactable[leftComponent.name].canOverride || canOverride.sameValue;
-          if (!mayOverride(leftComponent, rightComponent, validator) || !canOverride.twoOptionalFunctions(leftComponent, rightComponent, validator))
+          if (!mayOverride(leftComponent, rightComponent, validator))
+            continue propertyLoop;
+          if (!canOverride.twoOptionalFunctions(leftComponent, rightComponent, validator) && validator.isValidFunction(rightComponent))
             continue propertyLoop;
         }
 
@@ -258,7 +260,7 @@ function compactOverrides(properties, compatibility, validator) {
         component = left.components.filter(nameMatchFilter(right))[0];
         override(component, right);
         right.dirty = true;
-      } else if (!left.shorthand && !right.shorthand && left.name == right.name) {
+      } else if (left.name == right.name) {
         // two non-shorthands should be merged based on understandability
 
         if (left.important && !right.important) {

diff --git a/test/properties/override-compacting-test.js b/test/properties/override-compacting-test.js
index 78264c8..c5e5f19 100644 (file)
--- a/test/properties/override-compacting-test.js
+++ b/test/properties/override-compacting-test.js
@@ -141,15 +141,23 @@ vows.describe(optimize)
         ]);
       }
     },
-    'shorthand then shorthand - with function and url': {
+    'shorthand then shorthand - with function then url': {
       'topic': 'p{background:linear-gradient();background:__ESCAPED_URL_CLEAN_CSS0__}',
       'into': function (topic) {
         assert.deepEqual(_optimize(topic), [
-          [['background', false , false], ['linear-gradient()']],
           [['background', false , false], ['__ESCAPED_URL_CLEAN_CSS0__']]
         ]);
       }
     },
+    'shorthand then shorthand - with url then function': {
+      'topic': 'p{background:__ESCAPED_URL_CLEAN_CSS0__;background:linear-gradient()}',
+      'into': function (topic) {
+        assert.deepEqual(_optimize(topic), [
+          [['background', false , false], ['__ESCAPED_URL_CLEAN_CSS0__']],
+          [['background', false , false], ['linear-gradient()']]
+        ]);
+      }
+    },
     'shorthand then shorthand - important then non-important': {
       'topic': 'p{background:__ESCAPED_URL_CLEAN_CSS0__ no-repeat!important;background:__ESCAPED_URL_CLEAN_CSS1__ repeat red}',
       'into': function (topic) {
@@ -227,6 +235,32 @@ vows.describe(optimize)
           [['border-color', false, false], ['rgba(255,0,0,.5)']]
         ]);
       }
+    },
+    'border-color - hex then rgb': {
+      'topic': 'a{border-color:#000;border-color:rgba(255,0,0,.5)}',
+      'into': function (topic) {
+        assert.deepEqual(_optimize(topic), [
+          [['border-color', false, false], ['#000']],
+          [['border-color', false, false], ['rgba(255,0,0,.5)']]
+        ]);
+      }
+    },
+    'border-color - rgb then hex': {
+      'topic': 'a{border-color:rgba(255,0,0,.5);border-color:#000}',
+      'into': function (topic) {
+        assert.deepEqual(_optimize(topic), [
+          [['border-color', false, false], ['#000']]
+        ]);
+      }
+    },
+    'border-color - hex then rgb with multiple values123': {
+      'topic': 'a{border-color:red;border-color:#000 rgba(255,0,0,.5)}',
+      'into': function (topic) {
+        assert.deepEqual(_optimize(topic), [
+          [['border-color', false, false], ['red']],
+          [['border-color', false, false], ['#000'], ['rgba(255,0,0,.5)']]
+        ]);
+      }
     }
   })
   .addBatch({