Bug 939098 - Stop markup view parsing color names out of context r=jwalker a=bbajaj
authorMichael Ratcliffe <mratcliffe@mozilla.com>
Fri, 06 Dec 2013 19:57:16 +0000
changeset 166693 f3d6a773ab72afd8c7a88f62898cca8840a04c0a
parent 166692 29331d8b292dfbb12c029bd6eec2ebd051cf39cd
child 166694 4fb95c60417e65f5e4d051d0072c770309fbeb09
push id3066
push userakeybl@mozilla.com
push dateMon, 09 Dec 2013 19:58:46 +0000
treeherdermozilla-beta@a31a0dce83aa [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjwalker, bbajaj
bugs939098
milestone27.0a2
Bug 939098 - Stop markup view parsing color names out of context r=jwalker a=bbajaj
browser/devtools/shared/test/browser_outputparser.js
toolkit/devtools/output-parser.js
--- a/browser/devtools/shared/test/browser_outputparser.js
+++ b/browser/devtools/shared/test/browser_outputparser.js
@@ -44,31 +44,46 @@ function testParseCssProperty() {
 
   testParseHTMLAttribute();
 }
 
 function testParseHTMLAttribute() {
   let attrib = "color:red; font-size: 12px; background-image: " +
                "url(chrome://branding/content/about-logo.png)";
   let frag = parser.parseHTMLAttribute(attrib, {
-    colorSwatchClass: "test-colorswatch",
     urlClass: "theme-link"
   });
 
   let target = doc.querySelector("div");
   ok(target, "captain, we have the div");
   target.appendChild(frag);
 
-  let expected = 'color:<span style="background-color:red" ' +
-                 'class="test-colorswatch"></span>#F00; font-size: 12px; ' +
+  let expected = 'color:#F00; font-size: 12px; ' +
                  'background-image: url(\'<a href="chrome://branding/content/about-logo.png" ' +
                  'class="theme-link" ' +
                  'target="_blank">chrome://branding/content/about-logo.png</a>\')';
 
   is(target.innerHTML, expected, "HTML Attribute correctly parsed");
+  target.innerHTML = "";
+  testParseNonCssHTMLAttribute();
+}
+
+function testParseNonCssHTMLAttribute() {
+  let attrib = "someclass background someotherclass red";
+  let frag = parser.parseHTMLAttribute(attrib);
+
+  let target = doc.querySelector("div");
+  ok(target, "captain, we have the div");
+  target.appendChild(frag);
+
+  let expected = 'someclass background someotherclass red';
+
+  is(target.innerHTML, expected, "Non-CSS HTML Attribute correctly parsed");
+  target.innerHTML = "";
   finishUp();
 }
 
+
 function finishUp() {
   Services = Loader = OutputParser = parser = doc = null;
   gBrowser.removeCurrentTab();
   finish();
 }
--- a/toolkit/devtools/output-parser.js
+++ b/toolkit/devtools/output-parser.js
@@ -108,16 +108,17 @@ OutputParser.prototype = {
    *         Text to parse.
    * @param  {Object} [options]
    *         Options object. For valid options and default values see
    *         _mergeOptions().
    * @return {DocumentFragment}
    *         A document fragment. Colors will not be parsed.
    */
   parseHTMLAttribute: function(value, options={}) {
+    options.isHTMLAttribute = true;
     options = this._mergeOptions(options);
 
     return this._parse(value, options);
   },
 
   /**
    * Parse a string.
    *
@@ -127,91 +128,141 @@ OutputParser.prototype = {
    *         Options object. For valid options and default values see
    *         _mergeOptions().
    * @return {DocumentFragment}
    *         A document fragment.
    */
   _parse: function(text, options={}) {
     text = text.trim();
     this.parsed.length = 0;
-    let dirty = false;
-    let matched = null;
     let i = 0;
 
-    let trimMatchFromStart = function(match) {
-      text = text.substr(match.length);
-      dirty = true;
-      matched = null;
-    };
+    while (text.length > 0) {
+      let matched = null;
 
-    while (text.length > 0) {
+      // Prevent this loop from slowing down the browser with too
+      // many nodes being appended into output. In practice it is very unlikely
+      // that this will ever happen.
+      i++;
+      if (i > MAX_ITERATIONS) {
+        this._appendTextNode(text);
+        text = "";
+        break;
+      }
+
       matched = text.match(REGEX_QUOTES);
       if (matched) {
         let match = matched[0];
-        trimMatchFromStart(match);
+
+        text = this._trimMatchFromStart(text, match);
         this._appendTextNode(match);
+        continue;
       }
 
       matched = text.match(REGEX_WHITESPACE);
       if (matched) {
         let match = matched[0];
-        trimMatchFromStart(match);
+
+        text = this._trimMatchFromStart(text, match);
         this._appendTextNode(match);
+        continue;
       }
 
       matched = text.match(REGEX_URL);
       if (matched) {
         let [match, url] = matched;
-        trimMatchFromStart(match);
+
+        text = this._trimMatchFromStart(text, match);
         this._appendURL(match, url, options);
+        continue;
       }
 
       matched = text.match(REGEX_ALL_CSS_PROPERTIES);
       if (matched) {
         let [match] = matched;
-        trimMatchFromStart(match);
+
+        text = this._trimMatchFromStart(text, match);
         this._appendTextNode(match);
 
-        dirty = true;
+        if (options.isHTMLAttribute) {
+          [text] = this._appendColorOnMatch(text, options);
+        }
+        continue;
       }
 
-      matched = text.match(REGEX_ALL_COLORS);
-      if (matched) {
-        let match = matched[0];
-        if (this._appendColor(match, options)) {
-          trimMatchFromStart(match);
+      if (!options.isHTMLAttribute) {
+        let dirty;
+
+        [text, dirty] = this._appendColorOnMatch(text, options);
+
+        if (dirty) {
+          continue;
         }
       }
 
-      if (!dirty) {
-        // This test must always be last as it indicates use of an unknown
-        // character that needs to be removed to prevent infinite loops.
-        matched = text.match(REGEX_FIRST_WORD_OR_CHAR);
-        if (matched) {
-          let match = matched[0];
-          trimMatchFromStart(match);
-          this._appendTextNode(match);
-        }
-      }
+      // This test must always be last as it indicates use of an unknown
+      // character that needs to be removed to prevent infinite loops.
+      matched = text.match(REGEX_FIRST_WORD_OR_CHAR);
+      if (matched) {
+        let match = matched[0];
 
-      dirty = false;
-
-      // Prevent this loop from slowing down the browser with too
-      // many nodes being appended into output.
-      i++;
-      if (i > MAX_ITERATIONS) {
-        trimMatchFromStart(text);
-        this._appendTextNode(text);
+        text = this._trimMatchFromStart(text, match);
+        this._appendTextNode(match);
       }
     }
 
     return this._toDOM();
   },
 
   /**
+   * Convenience function to make the parser a little more readable.
+   *
+   * @param  {String} text
+   *         Main text
+   * @param  {String} match
+   *         Text to remove from the beginning
+   *
+   * @return {String}
+   *         The string passed as 'text' with 'match' stripped from the start.
+   */
+  _trimMatchFromStart: function(text, match) {
+    return text.substr(match.length);
+  },
+
+  /**
+   * Check if there is a color match and append it if it is valid.
+   *
+   * @param  {String} text
+   *         Main text
+   * @param  {Object} options
+   *         Options object. For valid options and default values see
+   *         _mergeOptions().
+   *
+   * @return {Array}
+   *         An array containing the remaining text and a dirty flag. This array
+   *         is designed for deconstruction using [text, dirty].
+   */
+  _appendColorOnMatch: function(text, options) {
+    let dirty;
+    let matched = text.match(REGEX_ALL_COLORS);
+
+    if (matched) {
+      let match = matched[0];
+      if (this._appendColor(match, options)) {
+        text = this._trimMatchFromStart(text, match);
+        dirty = true;
+      }
+    } else {
+      dirty = false;
+    }
+
+    return [text, dirty];
+  },
+
+  /**
    * Check if a CSS property supports a specific value.
    *
    * @param  {String} name
    *         CSS Property name to check
    * @param  {String} value
    *         CSS Property value to check
    */
   _cssPropertySupportsValue: function(name, value) {
@@ -327,17 +378,17 @@ OutputParser.prototype = {
    * node then we append the text to that node.
    *
    * @param  {String} text
    *         Text to append
    */
   _appendTextNode: function(text) {
     let lastItem = this.parsed[this.parsed.length - 1];
     if (typeof lastItem === "string") {
-      this.parsed[this.parsed.length - 1] = lastItem + text
+      this.parsed[this.parsed.length - 1] = lastItem + text;
     } else {
       this.parsed.push(text);
     }
   },
 
   /**
    * Take all output and append it into a single DocumentFragment.
    *
@@ -366,26 +417,34 @@ OutputParser.prototype = {
    *
    * @param  {Object} overrides
    *         The option values to override e.g. _mergeOptions({colors: false})
    *
    *         Valid options are:
    *           - defaultColorType: true // Convert colors to the default type
    *                                    // selected in the options panel.
    *           - colorSwatchClass: ""   // The class to use for color swatches.
+   *           - isHTMLAttribute: false // This property indicates whether we
+   *                                    // are parsing an HTML attribute value.
+   *                                    // When the value is passed in from an
+   *                                    // HTML attribute we need to check that
+   *                                    // any CSS property values are supported
+   *                                    // by the property name before
+   *                                    // processing the property value.
    *           - urlClass: ""           // The class to be used for url() links.
    *           - baseURI: ""            // A string or nsIURI used to resolve
    *                                    // relative links.
    * @return {Object}
    *         Overridden options object
    */
   _mergeOptions: function(overrides) {
     let defaults = {
       defaultColorType: true,
       colorSwatchClass: "",
+      isHTMLAttribute: false,
       urlClass: "",
       baseURI: ""
     };
 
     if (typeof overrides.baseURI === "string") {
       overrides.baseURI = Services.io.newURI(overrides.baseURI, null, null);
     }