From: Jakub Pawlowicz Date: Wed, 22 Mar 2017 11:20:30 +0000 (+0100) Subject: Fixes #916 - maximum number of merged selectors. X-Git-Url: https://git.ndcode.org/public/gitweb.cgi?a=commitdiff_plain;h=c695827b16b044e0313f9a81df492e4d17a95049;p=clean-css.git Fixes #916 - maximum number of merged selectors. Why: * Apparently Chrome's Blink engine places a limit on a number of selectors in a single rule. Any number of selectors greater than 8191 causes a rule to be ignored; * Allows arbitrary selection of the limit via `{ compatibility: { selectors: { mergeLimit: } } }` --- diff --git a/History.md b/History.md index 4cbde2fc..5c3691c9 100644 --- a/History.md +++ b/History.md @@ -1,6 +1,7 @@ [4.1.0-pre / 20xx-xx-xx](https://github.com/jakubpawlowicz/clean-css/compare/4.0...HEAD) ================== +* Fixed issue [#916](https://github.com/jakubpawlowicz/clean-css/issues/916) - maximum number of merged selectors. * Fixed issue [#893](https://github.com/jakubpawlowicz/clean-css/issues/893) - `inline: false` as alias to `inline: 'none'`. * Fixed issue [#890](https://github.com/jakubpawlowicz/clean-css/issues/890) - adds toggle to disable empty tokens removal. * Fixed issue [#886](https://github.com/jakubpawlowicz/clean-css/issues/886) - better multi pseudo class / element merging. diff --git a/README.md b/README.md index 6f350137..675bf768 100644 --- a/README.md +++ b/README.md @@ -100,6 +100,7 @@ Once released clean-css 4.1 will introduce the following changes / features: * `multiplePseudoMerging` compatibility flag controlling merging of rules with multiple pseudo classes / elements; * `removeEmpty` flag in level 1 optimizations controlling removal of rules and nested blocks; * `removeEmpty` flag in level 2 optimizations controlling removal of rules and nested blocks; +* `compatibility: { selectors: { mergeLimit: } }` flag in compatibility settings controlling maximum number of selectors in a single rule; ## Constructor options @@ -154,6 +155,7 @@ new CleanCSS({ ie7Hack: true, // controls removal of IE7 selector hacks, e.g. `*+html...` mergeablePseudoClasses: [':active', ...], // controls a whitelist of mergeable pseudo classes mergeablePseudoElements: ['::after', ...], // controls a whitelist of mergeable pseudo elements + mergeLimit: 8191, // controls maximum number of selectors in a single rule multiplePseudoMerging: true // controls merging of rules with multiple pseudo classes / elements (since 4.1.0-pre) }, units: { diff --git a/lib/optimizer/level-2/merge-adjacent.js b/lib/optimizer/level-2/merge-adjacent.js index 70f92e9d..b148bacd 100644 --- a/lib/optimizer/level-2/merge-adjacent.js +++ b/lib/optimizer/level-2/merge-adjacent.js @@ -19,6 +19,7 @@ function mergeAdjacent(tokens, context) { var selectorsSortingMethod = options.level[OptimizationLevel.One].selectorsSortingMethod; var mergeablePseudoClasses = options.compatibility.selectors.mergeablePseudoClasses; var mergeablePseudoElements = options.compatibility.selectors.mergeablePseudoElements; + var mergeLimit = options.compatibility.selectors.mergeLimit; var multiplePseudoMerging = options.compatibility.selectors.multiplePseudoMerging; for (var i = 0, l = tokens.length; i < l; i++) { @@ -35,7 +36,8 @@ function mergeAdjacent(tokens, context) { token[2] = []; } else if (lastToken[0] == Token.RULE && serializeBody(token[2]) == serializeBody(lastToken[2]) && isMergeable(serializeRules(token[1]), mergeablePseudoClasses, mergeablePseudoElements, multiplePseudoMerging) && - isMergeable(serializeRules(lastToken[1]), mergeablePseudoClasses, mergeablePseudoElements, multiplePseudoMerging)) { + isMergeable(serializeRules(lastToken[1]), mergeablePseudoClasses, mergeablePseudoElements, multiplePseudoMerging) && + lastToken[1].length < mergeLimit) { lastToken[1] = tidyRules(lastToken[1].concat(token[1]), false, adjacentSpace, false, context.warnings); lastToken[1] = lastToken.length > 1 ? sortSelectors(lastToken[1], selectorsSortingMethod) : lastToken[1]; token[2] = []; diff --git a/lib/optimizer/level-2/restructure.js b/lib/optimizer/level-2/restructure.js index d21aebcd..90b8bfa6 100644 --- a/lib/optimizer/level-2/restructure.js +++ b/lib/optimizer/level-2/restructure.js @@ -25,6 +25,7 @@ function restructure(tokens, context) { var options = context.options; var mergeablePseudoClasses = options.compatibility.selectors.mergeablePseudoClasses; var mergeablePseudoElements = options.compatibility.selectors.mergeablePseudoElements; + var mergeLimit = options.compatibility.selectors.mergeLimit; var multiplePseudoMerging = options.compatibility.selectors.multiplePseudoMerging; var specificityCache = context.cache.specificity; var movableTokens = {}; @@ -324,7 +325,8 @@ function restructure(tokens, context) { for (k = 0; k < movedCount; k++) { var movedProperty = movedProperties[k]; - if (movedToBeDropped.indexOf(k) == -1 && !canReorderSingle(property, movedProperty, specificityCache) && !boundToAnotherPropertyInCurrrentToken(property, movedProperty, token)) { + if (movedToBeDropped.indexOf(k) == -1 && (!canReorderSingle(property, movedProperty, specificityCache) && !boundToAnotherPropertyInCurrrentToken(property, movedProperty, token) || + movableTokens[movedProperty[4]] && movableTokens[movedProperty[4]].length === mergeLimit)) { dropPropertiesAt(i + 1, movedProperty, token); if (movedToBeDropped.indexOf(k) == -1) { @@ -346,8 +348,16 @@ function restructure(tokens, context) { continue; var key = property[4]; - movableTokens[key] = movableTokens[key] || []; - movableTokens[key].push(token); + + if (movedSameProperty && movedProperties[samePropertyAt][5].length + property[5].length > mergeLimit) { + dropPropertiesAt(i + 1, movedProperties[samePropertyAt]); + movedProperties.splice(samePropertyAt, 1); + movableTokens[key] = [token]; + movedSameProperty = false; + } else { + movableTokens[key] = movableTokens[key] || []; + movableTokens[key].push(token); + } if (movedSameProperty) { movedProperties[samePropertyAt] = cloneAndMergeSelectors(movedProperties[samePropertyAt], property); diff --git a/lib/options/compatibility.js b/lib/options/compatibility.js index 3f365959..8e6a119a 100644 --- a/lib/options/compatibility.js +++ b/lib/options/compatibility.js @@ -57,6 +57,7 @@ var DEFAULTS = { '::first-letter', '::first-line' ], // selectors with these pseudo-elements can be merged as these are universally supported + mergeLimit: 8191, // number of rules that can be safely merged together multiplePseudoMerging: true }, units: { diff --git a/test/optimizer/level-2/optimize-test.js b/test/optimizer/level-2/optimize-test.js index 2b4fec31..5eee33ac 100644 --- a/test/optimizer/level-2/optimize-test.js +++ b/test/optimizer/level-2/optimize-test.js @@ -26,6 +26,30 @@ vows.describe('level 2 optimizer') ] }, { level: 2 }) ) + .addBatch( + optimizerContext('limit rule merging', { + 'adjacent with as many rules as limit': [ + '.block--1{color:red}.block--2{color:red}.block--3{color:red}', + '.block--1,.block--2,.block--3{color:red}' + ], + 'adjacent with extra rule': [ + '.block--1{color:red}.block--2{color:red}.block--3{color:red}.block--4{color:red}', + '.block--1,.block--2,.block--3{color:red}.block--4{color:red}' + ], + 'adjacent with extra two rules': [ + '.block--1{color:red}.block--2{color:red}.block--3{color:red}.block--4{color:red}.block--5{color:red}', + '.block--1,.block--2,.block--3{color:red}.block--4,.block--5{color:red}' + ], + 'adjacent with extra three rules': [ + '.block--1{color:red}.block--2{color:red}.block--3{color:red}.block--4{color:red}.block--5{color:red}.block--6{color:red}', + '.block--1,.block--2,.block--3{color:red}.block--4,.block--5,.block--6{color:red}' + ], + 'non-adjacent': [ + '.block--1{color:red}.other-block--1{width:0}.block--2{color:red}.other-block--2{height:0}.block--3{color:red}.other-block--3{opacity:0}.block--4{color:red}', + '.block--1{color:red}.other-block--1{width:0}.block--2,.block--3,.block--4{color:red}.other-block--2{height:0}.other-block--3{opacity:0}' + ] + }, { compatibility: { selectors: { mergeLimit: 3 } }, level: { 2: { all: true } } }) + ) .addBatch( optimizerContext('level 2 off', { 'repeated' : [ diff --git a/test/optimizer/level-2/restructure-test.js b/test/optimizer/level-2/restructure-test.js index f8b094ef..550a61f8 100644 --- a/test/optimizer/level-2/restructure-test.js +++ b/test/optimizer/level-2/restructure-test.js @@ -174,6 +174,30 @@ vows.describe('restructure') ] }, { level: { 2: { all: false, restructureRules: true, removeEmpty: true } } }) ) + .addBatch( + optimizerContext('only rule restructuring with rule merging limit', { + 'adjacent with as many rules as limit': [ + '.block--1{color:red}.block--2{color:red}.block--3{color:red}', + '.block--1,.block--2,.block--3{color:red}' + ], + 'adjacent with extra rule': [ + '.block--1{color:red}.block--2{color:red}.block--3{color:red}.block--4{color:red}', + '.block--1{color:red}.block--2,.block--3,.block--4{color:red}' + ], + 'adjacent with extra two rules 123': [ + '.block--1{color:red}.block--2{color:red}.block--3{color:red}.block--4{color:red}.block--5{color:red}', + '.block--1,.block--2{color:red}.block--3,.block--4,.block--5{color:red}' + ], + 'adjacent with extra three rules': [ + '.block--1{color:red}.block--2{color:red}.block--3{color:red}.block--4{color:red}.block--5{color:red}.block--6{color:red}', + '.block--1,.block--2,.block--3{color:red}.block--4,.block--5,.block--6{color:red}' + ], + 'non-adjacent': [ + '.block--1{color:red}.other-block--1{width:0}.block--2{color:red}.other-block--2{height:0}.block--3{color:red}.other-block--3{opacity:0}.block--4{color:red}', + '.block--1{color:red}.other-block--1{width:0}.block--2,.block--3,.block--4{color:red}.other-block--2{height:0}.other-block--3{opacity:0}' + ] + }, { compatibility: { selectors: { mergeLimit: 3 } }, level: { 2: { all: false, restructureRules: true, removeEmpty: true } } }) + ) .addBatch( optimizerContext('level 2 off', { 'up until changed': [