Bug 1221156 - make FilterWidget try to preserve URL quoting; r=pbrosset
authorTom Tromey <tom@tromey.com>
Wed, 11 Nov 2015 10:15:00 +0100
changeset 308691 4efdfcadd82f7022ff9b49555f81294f7b1b0d46
parent 308690 53f9d55997a54967c53f7a56f02ddff2c16741dd
child 308692 0788d3d500163670f1a01e02cd0c2e11414bfc33
push id7514
push users.kaspari@gmail.com
push dateFri, 13 Nov 2015 14:12:41 +0000
reviewerspbrosset
bugs1221156
milestone45.0a1
Bug 1221156 - make FilterWidget try to preserve URL quoting; r=pbrosset
devtools/client/shared/test/browser_filter-editor-01.js
devtools/client/shared/widgets/FilterWidget.js
--- a/devtools/client/shared/test/browser_filter-editor-01.js
+++ b/devtools/client/shared/test/browser_filter-editor-01.js
@@ -64,9 +64,24 @@ add_task(function *() {
   widget.setCssValue("unset");
   is(widget.getCssValue(), "unset", "setCssValue should handle 'unset'");
   info("Test parsing of 'initial'");
   widget.setCssValue("initial");
   is(widget.getCssValue(), "initial", "setCssValue should handle 'initial'");
   info("Test parsing of 'inherit'");
   widget.setCssValue("inherit");
   is(widget.getCssValue(), "inherit", "setCssValue should handle 'inherit'");
+
+  info("Test parsing of quoted URL");
+  widget.setCssValue("url('invalid ) when ) unquoted')");
+  is(widget.getCssValue(), "url('invalid ) when ) unquoted')",
+     "setCssValue should re-quote single-quoted URL contents");
+  widget.setCssValue("url(\"invalid ) when ) unquoted\")");
+  is(widget.getCssValue(), "url(\"invalid ) when ) unquoted\")",
+     "setCssValue should re-quote double-quoted URL contents");
+  widget.setCssValue("url(ordinary)");
+  is(widget.getCssValue(), "url(ordinary)",
+     "setCssValue should not quote ordinary unquoted URL contents");
+  widget.setCssValue("url(invalid\\ \\)\\ \\{\\ when\\ \\}\\ \\;\\ unquoted)");
+  is(widget.getCssValue(),
+     "url(invalid\\ \\)\\ \\{\\ when\\ \\}\\ \\;\\ unquoted)",
+     "setCssValue should re-quote weird unquoted URL contents");
 });
--- a/devtools/client/shared/widgets/FilterWidget.js
+++ b/devtools/client/shared/widgets/FilterWidget.js
@@ -711,44 +711,47 @@ CSSFilterEditorWidget.prototype = {
 
     if (SPECIAL_VALUES.has(cssValue)) {
       this._specialValue = cssValue;
       this.emit("updated", this.getCssValue());
       this.render();
       return;
     }
 
-    for (let {name, value} of tokenizeFilterValue(cssValue)) {
+    for (let {name, value, quote} of tokenizeFilterValue(cssValue)) {
       // If the specified value is invalid, replace it with the
       // default.
       if (name !== "url") {
         if (!DOMUtils.cssPropertyIsValid("filter", name + "(" + value + ")")) {
           value = null;
         }
       }
 
-      this.add(name, value);
+      this.add(name, value, quote);
     }
 
     this.emit("updated", this.getCssValue());
     this.render();
   },
 
   /**
     * Creates a new [name] filter record with value
     *
     * @param {String} name
     *        filter name (e.g. blur)
     * @param {String} value
     *        value of the filter (e.g. 30px, 20%)
     *        If this is |null|, then a default value may be supplied.
+    * @param {String} quote
+    *        For a url filter, the quoting style.  This can be a
+    *        single quote, a double quote, or empty.
     * @return {Number}
     *        The index of the new filter in the current list of filters
     */
-  add: function(name, value) {
+  add: function(name, value, quote) {
     const def = this._definition(name);
     if (!def) {
       return false;
     }
 
     if (value === null) {
       // UNIT_MAPPING[string] is an empty string (falsy), so
       // using || doesn't work here
@@ -757,16 +760,21 @@ CSSFilterEditorWidget.prototype = {
                                UNIT_MAPPING[def.type];
 
       // string-type filters have no default value but a placeholder instead
       if (!unitLabel) {
         value = "";
       } else {
         value = def.range[0] + unitLabel;
       }
+
+      if (name === "url") {
+        // Default quote.
+        quote = "\"";
+      }
     }
 
     let unit = def.type === "string"
                ? ""
                : (/[a-zA-Z%]+/.exec(value) || [])[0];
 
     if (def.type !== "string") {
       value = parseFloat(value);
@@ -780,17 +788,17 @@ CSSFilterEditorWidget.prototype = {
       const [min, max] = def.range;
       if (value < min) {
         value = min;
       } else if (value > max) {
         value = max;
       }
     }
 
-    const index = this.filters.push({value, unit, name}) - 1;
+    const index = this.filters.push({value, unit, name, quote}) - 1;
     this.emit("updated", this.getCssValue());
 
     return index;
   },
 
   /**
     * returns value + unit of the specified filter
     *
@@ -800,19 +808,32 @@ CSSFilterEditorWidget.prototype = {
     *        css value of filter
     */
   getValueAt: function(index) {
     let filter = this.filters[index];
     if (!filter) {
       return null;
     }
 
-    const {value, unit} = filter;
+    // Just return the value+unit for non-url functions.
+    if (filter.name !== "url") {
+      return filter.value + filter.unit;
+    }
 
-    return value + unit;
+    // url values need to be quoted and escaped.
+    if (filter.quote === "'") {
+      return "'" + filter.value.replace(/\'/g, "\\'") + "'";
+    } else if (filter.quote === "\"") {
+      return "\"" + filter.value.replace(/\"/g, "\\\"") + "\"";
+    }
+
+    // Unquoted.  This approach might change the original input -- for
+    // example the original might be over-quoted.  But, this is
+    // correct and probably good enough.
+    return filter.value.replace(/[ \t(){};]/g, "\\$&");
   },
 
   removeAt: function(index) {
     if (!this.filters[index]) {
       return null;
     }
 
     this.filters.splice(index, 1);
@@ -921,17 +942,21 @@ function tokenizeFilterValue(css) {
     switch (state) {
       case "initial":
         if (token.tokenType === "function") {
           name = token.text;
           contents = "";
           state = "function";
           depth = 1;
         } else if (token.tokenType === "url" || token.tokenType === "bad_url") {
-          filters.push({name: "url", value: token.text.trim()});
+          // Extract the quoting style from the url.
+          let originalText = css.substring(token.startOffset, token.endOffset);
+          let [, quote] = /^url\([ \t\r\n\f]*(["']?)/i.exec(originalText);
+
+          filters.push({name: "url", value: token.text.trim(), quote: quote});
           // Leave state as "initial" because the URL token includes
           // the trailing close paren.
         }
         break;
 
       case "function":
         if (token.tokenType === "symbol" && token.text === ")") {
           --depth;