Skip to content

Commit

Permalink
Fixes #507 - merging longhand into many shorthands.
Browse files Browse the repository at this point in the history
This disables such merges altogether as it is not an easy feat, since:

* Merged property may be default;
* Merged property can be merged into many shorthands resulting in
  longer value;
* There can be more properties waiting to be merged in.

Follow up in #527.
  • Loading branch information
jakubpawlowicz committed Apr 13, 2015
1 parent 2c7a755 commit 2d050b0
Show file tree
Hide file tree
Showing 4 changed files with 52 additions and 2 deletions.
1 change: 1 addition & 0 deletions History.md
Expand Up @@ -14,6 +14,7 @@
* 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.
* Fixed issue [#507](https://github.com/jakubpawlowicz/clean-css/issues/507) - merging longhands into many shorthands.

[3.1.9 / 2015-04-04](https://github.com/jakubpawlowicz/clean-css/compare/v3.1.8...v3.1.9)
==================
Expand Down
20 changes: 20 additions & 0 deletions lib/properties/override-compactor.js
Expand Up @@ -105,6 +105,22 @@ function lengthOf(property) {
return stringifyProperty([fakeAsArray], 0).length;
}

function moreSameShorthands(properties, startAt, name) {
// Since we run the main loop in `compactOverrides` backwards, at this point some
// properties may not be marked as unused.
// We should consider reverting the order if possible
var count = 0;

for (var i = startAt; i >= 0; i--) {
if (properties[i].name == name && !properties[i].unused)
count++;
if (count > 1)
break;
}

return count > 1;
}

function wouldResultInLongerValue(left, right) {
if (!left.multiplex && !right.multiplex || left.multiplex && right.multiplex)
return false;
Expand Down Expand Up @@ -181,6 +197,10 @@ function compactOverrides(properties, compatibility, validator) {
if (right.important && !left.important)
continue;

// Pending more clever algorithm in #527
if (moreSameShorthands(properties, i - 1, left.name))
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;
Expand Down
2 changes: 1 addition & 1 deletion test/fixtures/issue-490-min.css

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

31 changes: 30 additions & 1 deletion test/properties/override-compacting-test.js
Expand Up @@ -116,6 +116,35 @@ vows.describe(optimize)
]);
}
},
'shorthand then longhand - two shorthands - pending #527': {
'topic': 'p{background:-webkit-linear-gradient();background:linear-gradient();background-repeat:repeat-x}',
'into': function (topic) {
assert.deepEqual(_optimize(topic), [
[['background', false , false], ['-webkit-linear-gradient()']],
[['background', false , false], ['linear-gradient()']],
[['background-repeat', false , false], ['repeat-x']]
]);
}
},
'shorthand then longhand - two shorthands and default - pending #527': {
'topic': 'p{background:-webkit-linear-gradient();background:linear-gradient();background-repeat:repeat}',
'into': function (topic) {
assert.deepEqual(_optimize(topic), [
[['background', false , false], ['-webkit-linear-gradient()']],
[['background', false , false], ['linear-gradient()']],
[['background-repeat', false , false], ['repeat']]
]);
}
},
'shorthand then longhand - two mergeable shorthands and default - pending #527': {
'topic': 'p{background:__ESCAPED_URL_CLEAN_CSS0__;background:__ESCAPED_URL_CLEAN_CSS1__;background-repeat:repeat-x}',
'into': function (topic) {
assert.deepEqual(_optimize(topic), [
[['background', false , false], ['__ESCAPED_URL_CLEAN_CSS1__']],
[['background-repeat', false , false], ['repeat-x']]
]);
}
},
'shorthand then shorthand - same values': {
'topic': 'p{background:red;background:red}',
'into': function (topic) {
Expand Down Expand Up @@ -407,7 +436,7 @@ vows.describe(optimize)
]);
}
},
'two multiplex shorthands with vendor specific functions123': {
'two multiplex shorthands with vendor specific functions': {
'topic': 'p{background:url(1.png),-webkit-linear-gradient();background:url(1.png),linear-gradient()}',
'into': function (topic) {
assert.deepEqual(_optimize(topic), [
Expand Down

0 comments on commit 2d050b0

Please sign in to comment.