Fixes #779 - merging `background-(position|size)`.
authorJakub Pawlowicz <contact@jakubpawlowicz.com>
Wed, 1 Jun 2016 06:43:28 +0000 (08:43 +0200)
committerJakub Pawlowicz <contact@jakubpawlowicz.com>
Wed, 1 Jun 2016 09:19:07 +0000 (11:19 +0200)
Those two properties were always merged but if vendor-prefixed
functions are used then they should not be.

Note we changed `everyCombination` checker to check for values
on same position when merging **longhand** properties which is what
browsers do, e.g.

```css
background-position: calc(100% - 1em) 1em;
background-position: calc(100% - 2em) 2em;
```

Now `everyCombination` checks if 1st and 2nd values can be merged
separately and ignores whether 1st can be merged with 2nd.

History.md
lib/properties/can-override.js
lib/properties/compactable.js
lib/properties/every-combination.js
test/properties/override-compacting-test.js

index 3b72db9..7c6cd74 100644 (file)
@@ -3,6 +3,11 @@
 
 * Requires Node.js 4.0+ to run.
 
+[3.4.15 / 2016-xx-xx](https://github.com/jakubpawlowicz/clean-css/compare/v3.4.14...3.4)
+==================
+
+* Fixed issue [#779](https://github.com/jakubpawlowicz/clean-css/issues/779) - merging `background-(position|size)`.
+
 [3.4.14 / 2016-05-31](https://github.com/jakubpawlowicz/clean-css/compare/v3.4.13...v3.4.14)
 ==================
 
index 3482b82..474e237 100644 (file)
@@ -10,6 +10,22 @@ function always() {
   return true;
 }
 
+function alwaysButIntoFunction(property1, property2, validator) {
+  var value1 = property1.value[0][0];
+  var value2 = property2.value[0][0];
+
+  var validFunction1 = validator.isValidFunction(value1);
+  var validFunction2 = validator.isValidFunction(value2);
+
+  if (validFunction1 && validFunction2) {
+    return validator.areSameFunction(value1, value2);
+  } else if (!validFunction1 && validFunction2) {
+    return false;
+  } else {
+    return true;
+  }
+}
+
 function backgroundImage(property1, property2, validator) {
   // The idea here is that 'more understandable' values override 'less understandable' values, but not vice versa
   // Understandability: (none | url | inherit) > (same function) > (same value)
@@ -115,6 +131,7 @@ function unit(property1, property2, validator) {
 
 module.exports = {
   always: always,
+  alwaysButIntoFunction: alwaysButIntoFunction,
   backgroundImage: backgroundImage,
   border: border,
   color: color,
index dce0238..9abc2df 100644 (file)
@@ -82,13 +82,13 @@ var compactable = {
     doubleValues: true
   },
   'background-position': {
-    canOverride: canOverride.always,
+    canOverride: canOverride.alwaysButIntoFunction,
     defaultValue: ['0', '0'],
     doubleValues: true,
     shortestValue: '0'
   },
   'background-size': {
-    canOverride: canOverride.always,
+    canOverride: canOverride.alwaysButIntoFunction,
     defaultValue: ['auto'],
     doubleValues: true,
     shortestValue: '0 0'
index f608ed6..6149597 100644 (file)
@@ -3,6 +3,7 @@ var shallowClone = require('./clone').shallow;
 var MULTIPLEX_SEPARATOR = ',';
 
 function everyCombination(fn, left, right, validator) {
+  var samePositon = !left.shorthand && !right.shorthand;
   var _left = shallowClone(left);
   var _right = shallowClone(right);
 
@@ -11,6 +12,9 @@ function everyCombination(fn, left, right, validator) {
       if (left.value[i][0] == MULTIPLEX_SEPARATOR || right.value[j][0] == MULTIPLEX_SEPARATOR)
         continue;
 
+      if (samePositon && i != j)
+        continue;
+
       _left.value = [left.value[i]];
       _right.value = [right.value[j]];
       if (!fn(_left, _right, validator))
index d3126db..8765818 100644 (file)
@@ -38,6 +38,49 @@ vows.describe(optimize)
         ]);
       }
     },
+    'longhand then longhand - background position as function': {
+      'topic': 'p{background-position:-moz-calc(100% - 1em) 0;background-position:calc(100% - 1em) 0}',
+      'into': function (topic) {
+        assert.deepEqual(_optimize(topic), [
+          [['background-position'], ['-moz-calc(100% - 1em)'], ['0']],
+          [['background-position'], ['calc(100% - 1em)'], ['0']]
+        ]);
+      }
+    },
+    'longhand then longhand - background position as same function': {
+      'topic': 'p{background-position:calc(100% - 1em) 0;background-position:calc(100% - 1em) 1em}',
+      'into': function (topic) {
+        assert.deepEqual(_optimize(topic), [
+          [['background-position'], ['calc(100% - 1em)'], ['1em']]
+        ]);
+      }
+    },
+    'longhand then longhand - background position as function by value': {
+      'topic': 'p{background-position:calc(100% - 1em) 0;background-position:1em 1em}',
+      'into': function (topic) {
+        assert.deepEqual(_optimize(topic), [
+          [['background-position'], ['1em'], ['1em']]
+        ]);
+      }
+    },
+    'longhand then longhand - background position as value by function': {
+      'topic': 'p{background-position:1em 0;background-position:calc(100% - 1em) 1em}',
+      'into': function (topic) {
+        assert.deepEqual(_optimize(topic), [
+          [['background-position'], ['1em'], ['0']],
+          [['background-position'], ['calc(100% - 1em)'], ['1em']]
+        ]);
+      }
+    },
+    'longhand then longhand - background size as function': {
+      'topic': 'p{background-size:-moz-calc(100% - 1em) 0;background-size:calc(100% - 1em) 0}',
+      'into': function (topic) {
+        assert.deepEqual(_optimize(topic), [
+          [['background-size'], ['-moz-calc(100% - 1em)'], ['0']],
+          [['background-size'], ['calc(100% - 1em)'], ['0']]
+        ]);
+      }
+    },
     'longhand then shorthand': {
       'topic': 'p{background-image:none;background:__ESCAPED_URL_CLEAN_CSS0__}',
       'into': function (topic) {