From: Jakub Pawlowicz <contact@jakubpawlowicz.com>
Date: Sun, 8 Feb 2015 11:42:42 +0000 (+0000)
Subject: Simplifies selector traversal in advanced merging.
X-Git-Url: https://git.ndcode.org/public/gitweb.cgi?a=commitdiff_plain;h=4562c307ce307abf07c8c92d066a55281bb20198;p=clean-css.git

Simplifies selector traversal in advanced merging.

Unifies selector traversal from `@media` merge algorithm and one
merging selectors with same name.

Effectively solves `@media` merging issue when traversed property
can be a shorthand.
---

diff --git a/lib/selectors/optimizers/advanced.js b/lib/selectors/optimizers/advanced.js
index ed74b7e5..11c4d455 100644
--- a/lib/selectors/optimizers/advanced.js
+++ b/lib/selectors/optimizers/advanced.js
@@ -23,22 +23,41 @@ function unsafeSelector(value) {
   return /\.|\*| :/.test(value);
 }
 
-function extractProperties(token) {
-  return (token.metadata.body.match(/([a-z\-]+):/g) || [])
-    .join('/')
-    .replace(/\-[a-z]+/g, '')
-    .split('/');
+function allProperties(token) {
+  var properties = [];
+
+  if (token.kind == 'selector') {
+    for (var i = token.metadata.bodiesList.length - 1; i >= 0; i--) {
+      var property = token.metadata.bodiesList[i];
+      var splitAt = property.indexOf(':');
+      properties.push([
+        property.substring(0, splitAt).match(/([a-z]+)/)[0],
+        property.substring(splitAt + 1)
+      ]);
+    }
+  } else if (token.kind == 'block') {
+    for (var j = token.body.length - 1; j >= 0; j--) {
+      properties = properties.concat(allProperties(token.body[j]));
+    }
+  }
+
+  return properties;
 }
 
-function unsafeTraversal(token, allProperties) {
-  var properties = extractProperties(token);
+function canReorder(left, right) {
+  for (var i = right.length - 1; i >= 0; i--) {
+    for (var j = left.length - 1; j >= 0; j--) {
+      var rightName = right[i][0];
+      var rightValue = right[i][1];
+      var leftName = left[j][0];
+      var leftValue = left[j][1];
 
-  for (var j = 0; j < properties.length; j++) {
-    if (allProperties.indexOf(properties[j]) > -1)
-      return true;
+      if (rightName == leftName && rightValue != leftValue)
+        return false;
+    }
   }
 
-  return false;
+  return true;
 }
 
 AdvancedOptimizer.prototype.isSpecial = function (selector) {
@@ -307,11 +326,17 @@ AdvancedOptimizer.prototype.mergeNonAdjacentBySelector = function (tokens) {
       var targetToken = tokens[targetPosition];
       var movedPosition = positions[j];
       var movedToken = tokens[movedPosition];
-      var movedProperties = extractProperties(movedToken);
+      var movedProperties = allProperties(movedToken);
 
       for (var k = movedPosition - 1; k > targetPosition; k--) {
-        if (tokens[k].isFlatBlock === false || tokens[k].kind == 'selector' && unsafeTraversal(tokens[k], movedProperties))
+        if (tokens[k].isFlatBlock === false)
           continue selectorIterator;
+
+        if (tokens[k].kind == 'selector') {
+          var traversedProperties = allProperties(tokens[k]);
+          if (!canReorder(movedProperties, traversedProperties))
+            continue selectorIterator;
+        }
       }
 
       var joinAt = [movedToken.body.length];
@@ -353,41 +378,6 @@ AdvancedOptimizer.prototype.mergeMediaQueries = function (tokens) {
   var candidates = {};
   var reduced = [];
 
-  function allProperties(token) {
-    var properties = [];
-
-    if (token.kind == 'selector') {
-      for (var i = token.metadata.bodiesList.length - 1; i >= 0; i--) {
-        var property = token.metadata.bodiesList[i];
-        var splitAt = property.indexOf(':');
-        properties.push([
-          property.substring(0, splitAt),
-          property.substring(splitAt + 1)
-        ]);
-      }
-    } else if (token.kind == 'block') {
-      for (var j = token.body.length - 1; j >= 0; j--) {
-        properties = properties.concat(allProperties(token.body[j]));
-      }
-    }
-
-    return properties;
-  }
-
-  function breakingMove(moved, traversed) {
-    for (var i = traversed.length - 1; i >= 0; i--) {
-      for (var j = moved.length - 1; j >= 0; j--) {
-        var traversedName = traversed[i][0];
-        var traversedValue = traversed[i][1];
-        var movedName = moved[j][0];
-        var movedValue = moved[j][1];
-
-        if (traversedName == movedName && traversedValue != movedValue)
-          return true;
-      }
-    }
-  }
-
   for (var i = tokens.length - 1; i >= 0; i--) {
     var token = tokens[i];
     if (token.kind != 'block' || token.isFlatBlock === true)
@@ -413,7 +403,7 @@ AdvancedOptimizer.prototype.mergeMediaQueries = function (tokens) {
 
       for (var k = positions[j] + 1; k < positions[j - 1]; k++) {
         var traversedProperties = allProperties(tokens[k]);
-        if (breakingMove(movedProperties, traversedProperties))
+        if (!canReorder(movedProperties, traversedProperties))
           continue positionLoop;
       }
 
diff --git a/test/fixtures/big-min.css b/test/fixtures/big-min.css
index 069a3217..3ec3640c 100644
--- a/test/fixtures/big-min.css
+++ b/test/fixtures/big-min.css
@@ -982,7 +982,7 @@ label i{font-style:normal;display:none}
 .liste_reactions .references .date{color:#8b9299}
 .liste_reactions input[class=btn],.liste_reactions input[class=btn_abo]{margin:5px 0 10px}
 .reaction_identifier,.reaction_redaction{margin:20px 0;background:#f5f8f9;border-top:3px solid #e9ecf0}
-.reaction_redaction{padding:0 0 10px}
+.reaction_redaction{padding:0 0 10px;overflow:hidden}
 .reaction_identifier .deja_abo{float:left;width:275px;border-left:1px solid #ebeff0}
 .reaction_identifier .deja_abo .erreur{display:none;padding:7px 28px 7px 15px}
 .reaction_identifier .form{padding:10px 10px 10px 14px}
@@ -997,7 +997,6 @@ label i{font-style:normal;display:none}
 .reaction_identifier .deja_abo label+input{width:210px}
 .reaction_identifier .deja_abo p>label{float:left}
 .reaction_identifier .deja_abo p>label+a{float:left;margin:14px 0 0 20px}
-.reaction_redaction{overflow:hidden}
 .reaction_redaction>div{margin:0 16px;padding-top:10px}
 .reaction_redaction label+.annotation{float:right}
 .reaction_redaction textarea{width:495px;max-width:495px;height:144px;margin:10px 0}
diff --git a/test/fixtures/reset-min.css b/test/fixtures/reset-min.css
index 01f51325..92fc845d 100644
--- a/test/fixtures/reset-min.css
+++ b/test/fixtures/reset-min.css
@@ -6,7 +6,6 @@ table{border-collapse:separate;border-spacing:0}
 caption,td,th{text-align:left;font-weight:400}
 blockquote:after,blockquote:before,q:after,q:before{content:""}
 blockquote,q{quotes:"" ""}
-.clear{clear:both}
+.clear{clear:both;display:block}
 .clear:after,.container:after{content:".";display:block;height:0;clear:both;visibility:hidden}
-* html .clear{height:1%}
-.clear{display:block}
\ No newline at end of file
+* html .clear{height:1%}
\ No newline at end of file
diff --git a/test/integration-test.js b/test/integration-test.js
index 2b0e4493..70f1dc65 100644
--- a/test/integration-test.js
+++ b/test/integration-test.js
@@ -1124,8 +1124,8 @@ path")}',
       'a{color:red\\9;display:block;color:#fff\\9}',
       'a{display:block;color:#fff\\9}'
     ],
-    'overriding a star by a non-ajacent selector': 'a{color:red}.one{color:#fff}a{*color:#fff}',
-    'overriding an underscore by a non-ajacent selector': 'a{color:red}.one{color:#fff}a{_color:#fff}',
+    'overriding a star by a non-ajacent selector': 'a{color:red}.one{color:#000}a{*color:#fff}',
+    'overriding an underscore by a non-ajacent selector': 'a{color:red}.one{color:#000}a{_color:#fff}',
     'overriding a backslash by a non-ajacent selector': 'a{color:red}.one{color:#fff}a{color:#fff\\9}',
     'keeps rgba(0,0,0,0)': 'a{color:rgba(0,0,0,0)}',
     'keeps rgba(255,255,255,0)': 'a{color:rgba(255,255,255,0)}',
diff --git a/test/media-queries-test.js b/test/media-queries-test.js
index ea1b3179..abeff2e0 100644
--- a/test/media-queries-test.js
+++ b/test/media-queries-test.js
@@ -66,6 +66,12 @@ vows.describe('media queries')
         assert.equal(minified.styles, '@media screen{a{color:red}div{display:block}}@media (min-width:1024px){p{width:100%}body{height:100%}}');
       }
     },
+    'same two with overriding shorthand in between': {
+      topic: new CleanCSS().minify('@media screen{a{font-size:10px}}@media (min-width:1024px){.one{font:12px Helvetica}}@media screen{div{display:block}}'),
+      'get merged': function(minified) {
+        assert.equal(minified.styles, '@media screen{a{font-size:10px}}@media (min-width:1024px){.one{font:12px Helvetica}}@media screen{div{display:block}}');
+      }
+    },
     'same two with same values as moved in between': {
       topic: new CleanCSS().minify('@media screen{a{color:red}}@media (min-width:1024px){.one{color:red}}@media screen{div{display:block}}'),
       'get merged': function(minified) {