From 04a587f58c4fb4c545222e10e4d2174eca9f2f35 Mon Sep 17 00:00:00 2001 From: Jakub Pawlowicz Date: Wed, 1 Jun 2016 08:43:28 +0200 Subject: [PATCH] Fixes #779 - merging `background-(position|size)`. 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 | 5 +++ lib/properties/can-override.js | 17 ++++++++ lib/properties/compactable.js | 4 +- lib/properties/every-combination.js | 4 ++ test/properties/override-compacting-test.js | 43 +++++++++++++++++++++ 5 files changed, 71 insertions(+), 2 deletions(-) diff --git a/History.md b/History.md index 3b72db92..7c6cd74d 100644 --- a/History.md +++ b/History.md @@ -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) ================== diff --git a/lib/properties/can-override.js b/lib/properties/can-override.js index 3482b827..474e2373 100644 --- a/lib/properties/can-override.js +++ b/lib/properties/can-override.js @@ -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, diff --git a/lib/properties/compactable.js b/lib/properties/compactable.js index dce02380..9abc2df6 100644 --- a/lib/properties/compactable.js +++ b/lib/properties/compactable.js @@ -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' diff --git a/lib/properties/every-combination.js b/lib/properties/every-combination.js index f608ed62..61495979 100644 --- a/lib/properties/every-combination.js +++ b/lib/properties/every-combination.js @@ -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)) diff --git a/test/properties/override-compacting-test.js b/test/properties/override-compacting-test.js index d3126db1..8765818e 100644 --- a/test/properties/override-compacting-test.js +++ b/test/properties/override-compacting-test.js @@ -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) { -- 2.34.1