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: <value> } } }`

diff --git a/History.md b/History.md
index 4cbde2f..5c3691c 100644 (file)
--- 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 6f35013..675bf76 100644 (file)
--- 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: <number> } }` 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 70f92e9..b148bac 100644 (file)
--- 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 d21aebc..90b8bfa 100644 (file)
--- 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 3f36595..8e6a119 100644 (file)
--- 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 2b4fec3..5eee33a 100644 (file)
--- 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 f8b094e..550a61f 100644 (file)
--- 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': [