Bug 1384463 - only trim CSS-allowed whitespace in declaration parser; r?gl draft
authorTom Tromey <tom@tromey.com>
Thu, 19 Oct 2017 11:04:30 -0600
changeset 683486 fd5950f52a71254caa8f08adc0980a5306394192
parent 683485 b9a113790a15d9480f4426a9ee6fbdd40b83386d
child 736665 0111430d06a47b19d40841287978d7b7e88e2bd3
push id85396
push userbmo:ttromey@mozilla.com
push dateThu, 19 Oct 2017 20:00:14 +0000
reviewersgl
bugs1384463
milestone58.0a1
Bug 1384463 - only trim CSS-allowed whitespace in declaration parser; r?gl MozReview-Commit-ID: 7bnu2a9G1uq
devtools/client/shared/test/unit/test_parseDeclarations.js
devtools/shared/css/parsing-utils.js
--- a/devtools/client/shared/test/unit/test_parseDeclarations.js
+++ b/devtools/client/shared/test/unit/test_parseDeclarations.js
@@ -360,16 +360,25 @@ const TEST_DATA = [
   },
 
   // Regression test for bug 1297890 - don't paste tokens.
   {
     parseComments: true,
     input: "stroke-dasharray: 1/*ThisIsAComment*/2;",
     expected: [{name: "stroke-dasharray", value: "1 2", priority: "", offsets: [0, 39]}]
   },
+
+  // Regression test for bug 1384463 - don't trim significant
+  // whitespace.
+  {
+    // \u00a0 is non-breaking space.
+    input: "\u00a0vertical-align: top",
+    expected: [{name: "\u00a0vertical-align", value: "top", priority: "",
+                offsets: [0, 20]}]
+  },
 ];
 
 function run_test() {
   run_basic_tests();
   run_comment_tests();
   run_named_tests();
 }
 
--- a/devtools/shared/css/parsing-utils.js
+++ b/devtools/shared/css/parsing-utils.js
@@ -245,16 +245,38 @@ function parseCommentDeclarations(isCssP
 function getEmptyDeclaration() {
   return {name: "", value: "", priority: "",
           terminator: "",
           offsets: [undefined, undefined],
           colonOffsets: false};
 }
 
 /**
+ * Like trim, but only trims CSS-allowed whitespace.
+ */
+function cssTrim(str) {
+  let match = /^[ \t\r\n\f]*(.*?)[ \t\r\n\f]*$/.exec(str);
+  if (match) {
+    return match[1];
+  }
+  return str;
+}
+
+/**
+ * Like trimRight, but only trims CSS-allowed whitespace.
+ */
+function cssTrimRight(str) {
+  let match = /^(.*?)[ \t\r\n\f]*$/.exec(str);
+  if (match) {
+    return match[1];
+  }
+  return str;
+}
+
+/**
  * A helper function that does all the parsing work for
  * parseDeclarations.  This is separate because it has some arguments
  * that don't make sense in isolation.
  *
  * The return value and arguments are like parseDeclarations, with
  * these additional arguments.
  *
  * @param {Function} isCssPropertyKnown
@@ -305,17 +327,17 @@ function parseDeclarationsInternal(isCss
                !lastProp.priority && lastProp.colonOffsets[1]) {
       // Whitespace appearing after the ":" is attributed to it.
       lastProp.colonOffsets[1] = token.endOffset;
     }
 
     if (token.tokenType === "symbol" && token.text === ":") {
       if (!lastProp.name) {
         // Set the current declaration name if there's no name yet
-        lastProp.name = current.trim();
+        lastProp.name = cssTrim(current);
         lastProp.colonOffsets = [token.startOffset, token.endOffset];
         current = "";
         hasBang = false;
 
         // When parsing a comment body, if the left-hand-side is not a
         // valid property name, then drop it and stop parsing.
         if (inComment && !commentOverride &&
             !isCssPropertyKnown(lastProp.name)) {
@@ -331,17 +353,17 @@ function parseDeclarationsInternal(isCss
       lastProp.terminator = "";
       // When parsing a comment, if the name hasn't been set, then we
       // have probably just seen an ordinary semicolon used in text,
       // so drop this and stop parsing.
       if (inComment && !lastProp.name) {
         current = "";
         break;
       }
-      lastProp.value = current.trim();
+      lastProp.value = cssTrim(current);
       current = "";
       hasBang = false;
       declarations.push(getEmptyDeclaration());
       lastProp = declarations[declarations.length - 1];
     } else if (token.tokenType === "ident") {
       if (token.text === "important" && hasBang) {
         lastProp.priority = "important";
         hasBang = false;
@@ -379,21 +401,21 @@ function parseDeclarationsInternal(isCss
   }
 
   // Handle whatever trailing properties or values might still be there
   if (current) {
     if (!lastProp.name) {
       // Ignore this case in comments.
       if (!inComment) {
         // Trailing property found, e.g. p1:v1;p2:v2;p3
-        lastProp.name = current.trim();
+        lastProp.name = cssTrim(current);
       }
     } else {
       // Trailing value found, i.e. value without an ending ;
-      lastProp.value = current.trim();
+      lastProp.value = cssTrim(current);
       let terminator = lexer.performEOFFixup("", true);
       lastProp.terminator = terminator + ";";
       // If the input was unterminated, attribute the remainder to
       // this property.  This avoids some bad behavior when rewriting
       // an unterminated comment.
       if (terminator) {
         lastProp.offsets[1] = inputString.length;
       }
@@ -828,17 +850,17 @@ RuleRewriter.prototype = {
                                      decl.colonOffsets[1]);
       this.result += unescapeCSSComment(commentNamePart);
 
       // When uncommenting, we must be sure to sanitize the text, to
       // avoid things like /* decl: }; */, which will be accepted as
       // a property but which would break the entire style sheet.
       let newText = this.inputString.substring(decl.colonOffsets[1],
                                                decl.offsets[1]);
-      newText = unescapeCSSComment(newText).trimRight();
+      newText = cssTrimRight(unescapeCSSComment(newText));
       this.result += this.sanitizeText(newText, index) + ";";
 
       // See if the comment end can be deleted.
       let trailingText = this.inputString.substring(decl.offsets[1]);
       if (EMPTY_COMMENT_END_RX.test(trailingText)) {
         copyOffset = decl.commentOffsets[1];
       } else {
         this.result += " /*";