Merge pull request #1 from Automattic/master

Updated fork

.travis.yml

@@ -1,5 +1,8 @@
 language: node_js
 node_js:
-  - "0.10"
+  - "6"
+  - "5"
+  - "4.1"
+  - "4.0"
   - "0.12"
   - "node"

README.md

@@ -203,6 +203,6 @@ SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
 
 - Uses [cheerio](https://github.com/cheeriojs/cheerio) for the underlying DOM
 representation.
-- Uses [cssom](https://github.com/NV/CSSOM) to parse out CSS selectors and
+- Uses [mensch](https://github.com/brettstimmerman/mensch) to parse out CSS and
 [Slick](https://github.com/subtleGradient/slick) to tokenize them.
 - Icon by [UnheardSounds](http://unheardsounds.deviantart.com/gallery/26536908#/d2ngozi)

client.js

@@ -150,15 +150,21 @@ function inlineDocument($, css, options) {
       // go through the properties
       function addProps(style, selector) {
         for (var i = 0, l = style.length; i < l; i++) {
-          var name = style[i];
-          var value = style[name] + (options.preserveImportant && style._importants[name] ? ' !important' : '');
-          var prop = new utils.Property(name, value, selector, style._importants[name] ? 2 : 0);
-          var existing = el.styleProps[name];
-
-          // if property name is not in the excluded properties array
-          if (juiceClient.excludedProperties.indexOf(name) < 0) {
-            if (existing && existing.compare(prop) === prop || !existing) {
-              el.styleProps[name] = prop;
+          if (style[i].type == 'property') {
+            var name = style[i].name;
+            var value = style[i].value;
+            var important = style[i].value.match(/!important$/) !== null;
+            if (important && !options.preserveImportant) value = value.replace(/\s*!important$/, '');
+            var prop = new utils.Property(name, value, selector, important ? 2 : 0);
+            var existing = el.styleProps[name];
+
+            // if property name is not in the excluded properties array
+            if (juiceClient.excludedProperties.indexOf(name) < 0) {
+              if (existing && existing.compare(prop) === prop || !existing) {
+                // deleting a property let us change the order (move it to the end in the setStyleAttrs loop)
+                if (existing) delete el.styleProps[name];
+                el.styleProps[name] = prop;
+              }
             }
           }
         }
@@ -175,8 +181,7 @@ function inlineDocument($, css, options) {
     // sort properties by their originating selector's specificity so that
     // props like "padding" and "padding-bottom" are resolved as expected.
     props.sort(function(a, b) {
-      return a.selector.specificity().join('').localeCompare(
-        b.selector.specificity().join(''));
+      return a.compareFunc(b);
     });
     var string = props
       .filter(function(prop) {

lib/property.js

@@ -30,19 +30,23 @@ function Property(prop, value, selector, priority) {
  * @api public
  */
 
-Property.prototype.compare = function(property) {
-  var a = this.selector.specificity();
+Property.prototype.compareFunc = function(property) {
+  var a = [];
+  a.push.apply(a, this.selector.specificity());
   a[0] += this.priority;
-  var b = property.selector.specificity();
+  var b = [];
+  b.push.apply(b, property.selector.specificity());
   b[0] += property.priority;
-  var winner = utils.compare(a, b);
+  return utils.compareFunc(a, b);
+};
 
-  if (winner === a && a !== b) {
-    return this;
-  }
+Property.prototype.compare = function(property) {
+  var winner = this.compareFunc(property);
+  if (winner === 1) return this;
   return property;
 };
 
+
 /**
  * Returns CSS property
  *

lib/utils.js

@@ -4,7 +4,7 @@
  * Module dependencies.
  */
 
-var cssom = require('cssom');
+var mensch = require('mensch');
 var cheerio = require('cheerio');
 var own = {}.hasOwnProperty;
 var os = require('os');
@@ -18,7 +18,7 @@ exports.Property = Property;
  * Returns an array of the selectors.
  *
  * @license Sizzle CSS Selector Engine - MIT
- * @param {String} selectorText from cssom
+ * @param {String} selectorText from mensch
  * @api public
  */
 
@@ -61,33 +61,39 @@ exports.extract = function extract(selectorText) {
  */
 
 exports.parseCSS = function(css) {
-  var rules = cssom.parse(css).cssRules || [];
+  var parsed = mensch.parse(css);
+  var rules = typeof parsed.stylesheet != 'undefined' && parsed.stylesheet.rules ? parsed.stylesheet.rules : [];
   var ret = [];
 
   for (var i = 0, l = rules.length; i < l; i++) {
-    if (rules[i].selectorText) { // media queries don't have selectorText
+    if (rules[i].type == 'rule') {
       var rule = rules[i];
-      var selectors = exports.extract(rule.selectorText);
+      var selectors = rule.selectors;
 
       for (var ii = 0, ll = selectors.length; ii < ll; ii++) {
-        ret.push([selectors[ii], rule.style]);
+        ret.push([selectors[ii], rule.declarations]);
       }
     }
   }
 
   return ret;
 };
 
-
-var getStringifiedStyles = function(rule) {
-  var styles = [];
-  for (var style = 0; style < rule.style.length; style++) {
-    var property = rule.style[style];
-    var value = rule.style[property];
-    var important = rule.style._importants[property] ? ' !important' : '';
-    styles.push('    ' + property + ': ' + value + important + ';');
+var removeStyle = function(style, startPos, endPos, skipRows, startOffset, endOffset, insert) {
+  var styleRows = style.split("\n");
+  var start = startOffset;
+  var end = endOffset;
+  for (var r = 1 + skipRows; r < startPos.line; r++) {
+    start += styleRows[r - 1 - skipRows].length + 1;
+  }
+  start += startPos.col;
+  if (endPos !== null) {
+    for (var r2 = 1 + skipRows; r2 < endPos.line; r2++) end += styleRows[r2 - 1 - skipRows].length + 1;
+    end += endPos.col;
+  } else {
+    end += style.length + 1;
   }
-  return styles;
+  return style.substr(0, start - 1) + insert + style.substr(end - 1);
 };
 
 /**
@@ -99,52 +105,47 @@ var getStringifiedStyles = function(rule) {
  */
 
 exports.getPreservedText = function(css, options) {
-  var rules = cssom.parse(css).cssRules || [];
-  var preserved = [];
-
-  for (var i = 0, l = rules.length; i < l; i++) {
-    /* CSS types
-      STYLE: 1,
-      IMPORT: 3,
-      MEDIA: 4,
-      FONT_FACE: 5,
-    */
-
-    if (options.fontFaces && rules[i].type === cssom.CSSFontFaceRule.prototype.type) {
-      var fontFace = [ '' ];
-      fontFace.push('@font-face {');
-      fontFace = fontFace.concat(getStringifiedStyles(rules[i]));
-      fontFace.push('}');
-
-      if (fontFace.length) {
-        preserved.push(fontFace.length ? fontFace.join(os.EOL) + os.EOL : '');
-      }
-    }
-
-    if (options.mediaQueries && rules[i].type === cssom.CSSMediaRule.prototype.type) {
-      var query = rules[i];
-      var queryString = [];
-
-      queryString.push(os.EOL + '@media ' + query.media[0] + ' {');
-
-      for (var ii = 0, ll = query.cssRules.length; ii < ll; ii++) {
-        var rule = query.cssRules[ii];
-
-        if (rule.type === cssom.CSSStyleRule.prototype.type
-            || rule.type === cssom.CSSFontFaceRule.prototype.type) {
-          queryString.push('  '
-            + (rule.type === cssom.CSSStyleRule.prototype.type ? rule.selectorText : '@font-face') + ' {');
-          queryString = queryString.concat(getStringifiedStyles(rule));
-          queryString.push('  }');
-        }
-      }
-
-      queryString.push('}');
-      preserved.push(queryString.length ? queryString.join(os.EOL) + os.EOL : '');
+  var parsed = mensch.parse(css, {position: true, comments: true});
+  var rules = typeof parsed.stylesheet != 'undefined' && parsed.stylesheet.rules ? parsed.stylesheet.rules : [];
+  var preserved = css;
+  var preserved2 = [];
+  var lastStart = null;
+
+  for (var i = rules.length - 1; i >= 0; i--) {
+    if (options.fontFaces && rules[i].type === 'font-face') {
+      // preserve
+      // TODO remove the replace once we decide we don't need to mimic the previous outputs
+      // we'll also have to remove the multiline hack in normalizeLineEndings
+      preserved2.push(
+        mensch.stringify(
+          { stylesheet: { rules: [ rules[i] ] }}, 
+          { comments: false, indentation: '    ' }
+        ).replace(/}/g,os.EOL+'}')
+      );
+    } else if (options.mediaQueries && rules[i].type === 'media') {
+      // preserve
+      preserved2.push(
+        mensch.stringify(
+          { stylesheet: { rules: [ rules[i] ] }},
+          { comments: false, indentation: '  ' }
+        )
+      );
+    } else {
+      // remove
+      preserved = removeStyle(preserved, rules[i].position.start, lastStart, 0, 0, 0, '');
     }
+    lastStart = rules[i].position.start;
   }
-
-  return preserved.join(os.EOL);
+  // preserved works by removing unwanted stuff, preserved2 by generating a new style with "stuff to keep"
+  // We have to decide what is our strategy (the preserved2 produces the same output of the cssom based version <= 2.0.0)
+  /* method 1: clean up original style and keep the remaining "as is" */
+  /*
+    if (preserved.trim().length === 0) return false;
+    return preserved;
+  */
+  /* method 2: create style from AST */
+  if (preserved2.length === 0) return false;
+  return os.EOL+preserved2.join(os.EOL)+os.EOL;
 };
 
 /**
@@ -160,7 +161,8 @@ exports.cheerio = function(html, options) {
 };
 
 exports.normalizeLineEndings = function(text) {
-  return text.replace(/\r\n/g, '\n').replace(/\n/g, '\r\n');
+  // NOTE this consider multiple newlines the same as a single newline
+  return text.replace(/\r\n/g, '\n').replace(/\n/g, '\r\n').replace(/\r\n\r\n/g, '\r\n');
 };
 
 exports.encodeEJS = function(html) {
@@ -193,14 +195,18 @@ exports.decodeEntities = function(html) {
  * @api public
  */
 
-exports.compare = function(a, b) {
+exports.compareFunc = function(a, b) {
   for (var i = 0; i < 4; i++) {
     if (a[i] === b[i]) { continue; }
-    if (a[i] > b[i]) { return a; }
-    return b;
+    if (a[i] > b[i]) { return 1; }
+    return -1;
   }
 
-  return b;
+  return 0;
+};
+
+exports.compare = function(a, b) {
+  return exports.compareFunc(a, b) == 1 ? a : b;
 };
 
 exports.extend = function(obj, src) {
@@ -229,4 +235,4 @@ exports.getDefaultOptions = function(options) {
   result.webResources = result.webResources || {};
 
   return result;
-}
+};

package.json

@@ -40,7 +40,7 @@
     "cheerio": "0.20.0",
     "commander": "2.9.0",
     "cross-spawn-async": "^2.1.8",
-    "cssom": "0.3.1",
+    "mensch": "brettstimmerman/mensch#v0.3.2",
     "deep-extend": "^0.4.0",
     "slick": "1.12.2",
     "web-resource-inliner": "2.0.0"

test/cases/important-specificity.css

@@ -0,0 +1,5 @@
+.a {
+  padding-top: 5px !important;
+  padding-top: 7px;
+  padding-bottom: 5px;
+}

test/cases/important-specificity.html

@@ -0,0 +1 @@
+<div class="a" style="padding: 10px"></div>

test/cases/important-specificity.out

@@ -0,0 +1 @@
+<div class="a" style="padding-bottom: 5px; padding: 10px; padding-top: 5px;"></div>

test/cases/improper-syntax-affect-cascade-issue44.css

@@ -0,0 +1,10 @@
+body {
+  padding: 5px;
+}
+a {
+  color: ;
+}
+table {
+  padding: 5px;
+  color: #333;
+}

test/cases/improper-syntax-affect-cascade-issue44.html

@@ -0,0 +1,13 @@
+<html lang="en">
+<head>
+  <meta charset="UTF-8">
+  <title>Testing</title>
+</head>
+<body>
+  <table>
+    <tr>
+      <td><a href="sdf">a test!</a></td>
+    </tr>
+  </table>
+</body>
+</html>

test/cases/improper-syntax-affect-cascade-issue44.out

@@ -0,0 +1,13 @@
+<html lang="en">
+<head>
+  <meta charset="UTF-8">
+  <title>Testing</title>
+</head>
+<body style="padding: 5px;">
+  <table style="padding: 5px; color: #333;">
+    <tr>
+      <td><a href="sdf">a test!</a></td>
+    </tr>
+  </table>
+</body>
+</html>

test/cases/inline-specificity.css

@@ -0,0 +1,6 @@
+.a {
+  padding-top: 5px !important;
+}
+.a {
+  padding-bottom: 5px;
+}

test/cases/inline-specificity.html

@@ -0,0 +1 @@
+<div class="a" style="padding: 10px"></div>

test/cases/inline-specificity.out

@@ -0,0 +1 @@
+<div class="a" style="padding-bottom: 5px; padding: 10px; padding-top: 5px;"></div>