Changeset 142440 in webkit

Timestamp:

Feb 11, 2013 1:39:32 AM (11 years ago)

Author:

commit-queue@webkit.org

Message:

Web Inspector: Don't throw exceptions in WebInspector.Color
​https://bugs.webkit.org/show_bug.cgi?id=104835

Source/WebCore:

Patch by John J. Barton <​johnjbarton@chromium.org> on 2013-02-11
Reviewed by Vsevolod Vlasov.

WebInspector.Color.parse() returns a Color from a string, or null;
Ctor calls now call parse();
In the StylesSideBarPane, test null rather than catch(e).

Added case to inspector/styles/styles-invalid-color-values.html

• inspector/front-end/Color.js:

(WebInspector.Color):
(WebInspector.Color.parse):
(WebInspector.Color.fromRGBA):
(WebInspector.Color.fromRGB):
(WebInspector.Color.prototype.toString):
(WebInspector.Color.prototype._parse.this.alpha.set 0):
(WebInspector.Color.prototype._parse.this.nickname.set 2):
(WebInspector.Color.prototype._parse.this.hsla.set 1):
(WebInspector.Color.prototype._parse.this.rgba.set 0):
(WebInspector.Color.prototype._parse.set WebInspector):
(WebInspector.Color.prototype._parse):

• inspector/front-end/Spectrum.js:

(WebInspector.Spectrum.prototype.get color):

• inspector/front-end/StylesSidebarPane.js:

LayoutTests:

Patch by John J. Barton <​johnjbarton@chromium.org> on 2013-02-11
Reviewed by Vsevolod Vlasov.

Added case to test parsing 'none' from border style

• inspector/styles/styles-invalid-color-values-expected.txt:
• inspector/styles/styles-invalid-color-values.html:

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/inspector/styles/styles-invalid-color-values-expected.txt (modified) (1 diff)
• LayoutTests/inspector/styles/styles-invalid-color-values.html (modified) (2 diffs)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/inspector/front-end/Color.js (modified) (7 diffs)
• Source/WebCore/inspector/front-end/Spectrum.js (modified) (1 diff)
• Source/WebCore/inspector/front-end/StylesSidebarPane.js (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2013-02-11  John J. Barton  <johnjbarton@chromium.org>
+
+        Web Inspector: Don't throw exceptions in WebInspector.Color
+        https://bugs.webkit.org/show_bug.cgi?id=104835
+
+        Reviewed by Vsevolod Vlasov.
+        
+        Added case to test parsing 'none' from border style
+
+        * inspector/styles/styles-invalid-color-values-expected.txt:
+        * inspector/styles/styles-invalid-color-values.html:
+
 2013-02-11  Andrey Lushnikov  <lushnikov@chromium.org>
 

trunk/LayoutTests/inspector/styles/styles-invalid-color-values-expected.txt

@@ -74 +74 @@
   hsla: hsla(0, 100%, 50%, 1)
 
+Running: testInvalidColors
+
+SUCCESS: parsed invalid color none to null
+

trunk/LayoutTests/inspector/styles/styles-invalid-color-values.html

@@ -18 +18 @@
         'rgba(255, 0, 0, -5)', // clipped to rgba(255,0,0,0)
         'rgba(255, 0, 0, 5)',  // clipped to rgba(255,0,0,1)
+        ];
+
+    var invalidColors = [
+        // An invalid color, eg a value for a shorthand like 'border' which can have a color
+        'none',
     ];
 
@@ -27 +32 @@
             next();
         },
+        function testInvalidColors(next)
+        {
+            for (var i = 0; i < invalidColors.length; ++i)
+                dumpErrorsForInvalidColor(invalidColors[i]);
+            next();
+        },
     ]);
+
+    function dumpErrorsForInvalidColor(colorString) 
+    {
+        var color = WebInspector.Color.parse(colorString);
+        if (!color) {
+            InspectorTest.addResult("");
+            InspectorTest.addResult("SUCCESS: parsed invalid color " + colorString + " to null");
+            return;
+        } else {
+            InspectorTest.addResult("");
+            InspectorTest.addResult("FAIL: invalid color " + colorString + " did not parse to to null");
+        }
+    }
 
     function dumpColorRepresentationsForColor(colorString)
     {
-        try {
-            var color = new WebInspector.Color(colorString);
-        } catch (e) {
-            InspectorTest.addResult("FAIL: Error parsing color '" + colorString + "'.");
+        var color = WebInspector.Color.parse(colorString);
+        if (!color)
             return;
-        }
 
         InspectorTest.addResult("");

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2013-02-11  John J. Barton  <johnjbarton@chromium.org>
+
+        Web Inspector: Don't throw exceptions in WebInspector.Color
+        https://bugs.webkit.org/show_bug.cgi?id=104835
+
+        Reviewed by Vsevolod Vlasov.
+
+        WebInspector.Color.parse() returns a Color from a string, or null;
+        Ctor calls now call parse();
+        In the StylesSideBarPane, test null rather than catch(e).
+
+        Added case to inspector/styles/styles-invalid-color-values.html
+
+        * inspector/front-end/Color.js:
+        (WebInspector.Color):
+        (WebInspector.Color.parse):
+        (WebInspector.Color.fromRGBA):
+        (WebInspector.Color.fromRGB):
+        (WebInspector.Color.prototype.toString):
+        (WebInspector.Color.prototype._parse.this.alpha.set 0):
+        (WebInspector.Color.prototype._parse.this.nickname.set 2):
+        (WebInspector.Color.prototype._parse.this.hsla.set 1):
+        (WebInspector.Color.prototype._parse.this.rgba.set 0):
+        (WebInspector.Color.prototype._parse.set WebInspector):
+        (WebInspector.Color.prototype._parse):
+        * inspector/front-end/Spectrum.js:
+        (WebInspector.Spectrum.prototype.get color):
+        * inspector/front-end/StylesSidebarPane.js:
+
 2013-02-11  Andrey Lushnikov  <lushnikov@chromium.org>
 

trunk/Source/WebCore/inspector/front-end/Color.js

@@ -29 +29 @@
 
 /**
- * @constructor
+ * Don't call directly, use WebInspector.Color.parse(). 
+ * @constructor 
  */
 WebInspector.Color = function(str)
 {
     this.value = str;
-    this._parse();
 }
 
+WebInspector.Color.parse = function(str)
+{
+    var maybeColor = new WebInspector.Color(str);
+    return maybeColor._parse() ? maybeColor : null;
+}
 /**
  * @param {number=} a
@@ -42 +47 @@
 WebInspector.Color.fromRGBA = function(r, g, b, a)
 {
-    return new WebInspector.Color("rgba(" + r + "," + g + "," + b + "," + (typeof a === "undefined" ? 1 : a) + ")");
+    return WebInspector.Color.parse("rgba(" + r + "," + g + "," + b + "," + (typeof a === "undefined" ? 1 : a) + ")");
 }
 
 WebInspector.Color.fromRGB = function(r, g, b)
 {
-    return new WebInspector.Color("rgb(" + r + "," + g + "," + b + ")");
+    return WebInspector.Color.parse("rgb(" + r + "," + g + "," + b + ")");
 }
 
@@ -207 +212 @@
         }
 
-        throw "invalid color format";
+        return "invalid color format: " + format;
     },
 
@@ -433 +438 @@
             this.nickname = set[2];
             this.alpha = set[0][3];
-            return;
+            return true;
         }
 
@@ -462 +467 @@
                     this.hex = WebInspector.Color.Nicknames[nickname];
                 } else // unknown name
-                    throw "unknown color name";
+                    return false;
             } else if (match[4]) { // hsl
                 this.format = "hsl";
@@ -480 +485 @@
             }
 
-            return;
+            return true;
         }
 
@@ -500 +505 @@
             }
 
-            return;
+            return true;
         }
 
         // Could not parse as a valid color
-        throw "could not parse color";
+        return false;
     }
 }

trunk/Source/WebCore/inspector/front-end/Spectrum.js

@@ -282 +282 @@
         if (!colorValue)
             colorValue = color.toString(); // this.outputColorFormat can be invalid for current color (e.g. "nickname").
-        return new WebInspector.Color(colorValue);
+        return WebInspector.Color.parse(colorValue);
     },
 

trunk/Source/WebCore/inspector/front-end/StylesSidebarPane.js

@@ -1757 +1757 @@
             function processColor(text)
             {
-                try {
-                    var color = new WebInspector.Color(text);
-                } catch (e) {
+                var color = WebInspector.Color.parse(text);
+
+                // We can be called with valid non-color values of |text| (like 'none' from border style) 
+                if (!color) 
                     return document.createTextNode(text);
-                }
-
+                
                 var format = getFormat();
                 var hasSpectrum = self._parentPane;