From 2c7a75504606fc9a853c9b7dd87b3a7128741667 Mon Sep 17 00:00:00 2001 From: Jakub Pawlowicz Date: Sun, 12 Apr 2015 20:20:16 +0100 Subject: [PATCH] Fixes #490 - multivalue vendor prefixed background. Basically we were not testing if all values were overriddable, but just the first one, which was failing if any further wasn't. Note - we still need to fix shorthands being pulled into the last shorthand only. It should likely be disabled altogether, which will solve --- History.md | 1 + lib/properties/every-combination.js | 24 +++++++++++++++++++++ lib/properties/override-compactor.js | 13 +++++------ lib/properties/shorthand-compactor.js | 3 ++- test/fixtures/issue-490-min.css | 1 + test/fixtures/issue-490.css | 11 ++++++++++ test/properties/override-compacting-test.js | 11 +++++++++- 7 files changed, 56 insertions(+), 8 deletions(-) create mode 100644 lib/properties/every-combination.js create mode 100644 test/fixtures/issue-490-min.css create mode 100644 test/fixtures/issue-490.css diff --git a/History.md b/History.md index 69b496f6..f56bbc68 100644 --- a/History.md +++ b/History.md @@ -12,6 +12,7 @@ * Fixed issue [#446](https://github.com/jakubpawlowicz/clean-css/issues/446) - `list-style` fuzzy matching. * Fixed issue [#468](https://github.com/jakubpawlowicz/clean-css/issues/468) - bumps `source-map` to 0.4.x. * 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. [3.1.9 / 2015-04-04](https://github.com/jakubpawlowicz/clean-css/compare/v3.1.8...v3.1.9) diff --git a/lib/properties/every-combination.js b/lib/properties/every-combination.js new file mode 100644 index 00000000..f608ed62 --- /dev/null +++ b/lib/properties/every-combination.js @@ -0,0 +1,24 @@ +var shallowClone = require('./clone').shallow; + +var MULTIPLEX_SEPARATOR = ','; + +function everyCombination(fn, left, right, validator) { + var _left = shallowClone(left); + var _right = shallowClone(right); + + for (var i = 0, l = left.value.length; i < l; i++) { + for (var j = 0, m = right.value.length; j < m; j++) { + if (left.value[i][0] == MULTIPLEX_SEPARATOR || right.value[j][0] == MULTIPLEX_SEPARATOR) + continue; + + _left.value = [left.value[i]]; + _right.value = [right.value[j]]; + if (!fn(_left, _right, validator)) + return false; + } + } + + return true; +} + +module.exports = everyCombination; diff --git a/lib/properties/override-compactor.js b/lib/properties/override-compactor.js index ca7f85b5..9d496c52 100644 --- a/lib/properties/override-compactor.js +++ b/lib/properties/override-compactor.js @@ -4,6 +4,7 @@ var deepClone = require('./clone').deep; var shallowClone = require('./clone').shallow; var hasInherit = require('./has-inherit'); var restoreShorthands = require('./restore-shorthands'); +var everyCombination = require('./every-combination'); var stringifyProperty = require('../stringifier/one-time').property; @@ -172,7 +173,7 @@ function compactOverrides(properties, compatibility, validator) { component = right.components.filter(nameMatchFilter(left))[0]; mayOverride = (compactable[left.name] && compactable[left.name].canOverride) || canOverride.sameValue; - if (mayOverride(left, component, validator)) { + if (everyCombination(mayOverride, left, component, validator)) { left.unused = true; } } else if (left.shorthand && !right.shorthand && isComponentOf(left, right)) { @@ -181,7 +182,7 @@ function compactOverrides(properties, compatibility, validator) { continue; component = left.components.filter(nameMatchFilter(right))[0]; - if (mayOverride(component, right, validator)) { + if (everyCombination(mayOverride, component, right, validator)) { var disabledBackgroundSizeMerging = !compatibility.properties.backgroundSizeMerging && component.name.indexOf('background-size') > -1; var nonMergeableValue = compactable[right.name].nonMergeableValue === right.value[0][0]; @@ -221,9 +222,9 @@ function compactOverrides(properties, compatibility, validator) { var rightComponent = right.components[k]; mayOverride = compactable[leftComponent.name].canOverride || canOverride.sameValue; - if (!mayOverride(leftComponent, rightComponent, validator)) + if (!everyCombination(mayOverride, leftComponent, rightComponent, validator)) continue propertyLoop; - if (!canOverride.twoOptionalFunctions(leftComponent, rightComponent, validator) && validator.isValidFunction(rightComponent)) + if (!everyCombination(canOverride.twoOptionalFunctions, leftComponent, rightComponent, validator) && validator.isValidFunction(rightComponent)) continue propertyLoop; } @@ -237,7 +238,7 @@ function compactOverrides(properties, compatibility, validator) { component = left.components.filter(nameMatchFilter(right))[0]; mayOverride = compactable[right.name].canOverride || canOverride.sameValue; - if (!mayOverride(component, right, validator)) + if (!everyCombination(mayOverride, component, right, validator)) continue; if (left.important && !right.important) { @@ -261,7 +262,7 @@ function compactOverrides(properties, compatibility, validator) { } mayOverride = compactable[right.name].canOverride || canOverride.sameValue; - if (!mayOverride(left, right, validator)) + if (!everyCombination(mayOverride, left, right, validator)) continue; left.unused = true; diff --git a/lib/properties/shorthand-compactor.js b/lib/properties/shorthand-compactor.js index 303c9dae..59637cbe 100644 --- a/lib/properties/shorthand-compactor.js +++ b/lib/properties/shorthand-compactor.js @@ -3,6 +3,7 @@ var deepClone = require('./clone').deep; var hasInherit = require('./has-inherit'); var populateComponents = require('./populate-components'); var wrapSingle = require('./wrap-for-optimizing').single; +var everyCombination = require('./every-combination'); function mixedImportance(components) { var important; @@ -50,7 +51,7 @@ function replaceWithShorthand(properties, candidateComponents, name, sourceMaps, if (hasInherit(component)) return; - if (!canOverride(newProperty.components[i], component, validator)) + if (!everyCombination(canOverride, newProperty.components[i], component, validator)) return; newProperty.components[i] = deepClone(component); diff --git a/test/fixtures/issue-490-min.css b/test/fixtures/issue-490-min.css new file mode 100644 index 00000000..890adb11 --- /dev/null +++ b/test/fixtures/issue-490-min.css @@ -0,0 +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} diff --git a/test/fixtures/issue-490.css b/test/fixtures/issue-490.css new file mode 100644 index 00000000..55f6d893 --- /dev/null +++ b/test/fixtures/issue-490.css @@ -0,0 +1,11 @@ +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; +} diff --git a/test/properties/override-compacting-test.js b/test/properties/override-compacting-test.js index c5e5f19c..234bd869 100644 --- a/test/properties/override-compacting-test.js +++ b/test/properties/override-compacting-test.js @@ -253,7 +253,7 @@ vows.describe(optimize) ]); } }, - 'border-color - hex then rgb with multiple values123': { + 'border-color - hex then rgb with multiple values': { 'topic': 'a{border-color:red;border-color:#000 rgba(255,0,0,.5)}', 'into': function (topic) { assert.deepEqual(_optimize(topic), [ @@ -407,6 +407,15 @@ vows.describe(optimize) ]); } }, + 'two multiplex shorthands with vendor specific functions123': { + 'topic': 'p{background:url(1.png),-webkit-linear-gradient();background:url(1.png),linear-gradient()}', + 'into': function (topic) { + assert.deepEqual(_optimize(topic), [ + [['background', false , false], ['url(1.png)'], [','], ['-webkit-linear-gradient()']], + [['background', false , false], ['url(1.png)'], [','], ['linear-gradient()']] + ]); + } + }, 'not too long into multiplex #1': { 'topic': 'p{background:top left;background-repeat:no-repeat,no-repeat}', 'into': function (topic) { -- 2.34.1