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:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@142440 268f45cc-cd09-0410-ab3c-d52691b4dbfc

diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
index f5848a0e84e3e2187ae011f8cc02fa93e2115967..a91571df23f64849d8a2bb9ad45d0b8894eceea4 100644 (file)
--- a/LayoutTests/ChangeLog
+++ b/LayoutTests/ChangeLog
@@ -1,3 +1,15 @@
+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>
 
         Web Inspector: home button behaviour is wrong in DTE

diff --git a/LayoutTests/inspector/styles/styles-invalid-color-values-expected.txt b/LayoutTests/inspector/styles/styles-invalid-color-values-expected.txt
index dd4920ad35725b68749d377fe06539f413bce6da..8a55280e9a3a38e0c04ec24d3aca162c33fa7e04 100644 (file)
--- a/LayoutTests/inspector/styles/styles-invalid-color-values-expected.txt
+++ b/LayoutTests/inspector/styles/styles-invalid-color-values-expected.txt
@@ -73,3 +73,7 @@ color: rgba(255, 0, 0, 5)
   hsl: hsl(0, 100%, 50%)
   hsla: hsla(0, 100%, 50%, 1)
 
+Running: testInvalidColors
+
+SUCCESS: parsed invalid color none to null
+

diff --git a/LayoutTests/inspector/styles/styles-invalid-color-values.html b/LayoutTests/inspector/styles/styles-invalid-color-values.html
index 6ee4a2019f93d8d0a21219a1230ee637bdb40fd5..a0f7bd2b1100fa68b23b992437c6cabfc5d195ee 100644 (file)
--- a/LayoutTests/inspector/styles/styles-invalid-color-values.html
+++ b/LayoutTests/inspector/styles/styles-invalid-color-values.html
@@ -17,6 +17,11 @@ function test()
         // Each of these has their alpha clipped [0.0, 1.0].
         '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',
     ];
 
     InspectorTest.runTestSuite([
@@ -26,16 +31,32 @@ function test()
                 dumpColorRepresentationsForColor(colors[i]);
             next();
         },
+        function testInvalidColors(next)
+        {
+            for (var i = 0; i < invalidColors.length; ++i)
+                dumpErrorsForInvalidColor(invalidColors[i]);
+            next();
+        },
     ]);
 
-    function dumpColorRepresentationsForColor(colorString)
+    function dumpErrorsForInvalidColor(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) {
+            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)
+    {
+        var color = WebInspector.Color.parse(colorString);
+        if (!color)
+            return;
 
         InspectorTest.addResult("");
         InspectorTest.addResult("color: " + colorString);

diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
index b158a76932b96b89c0daf17a2003cc9de55cbcd8..e78937fc1079ac51327ab5af525e0313765f5f00 100644 (file)
--- a/Source/WebCore/ChangeLog
+++ b/Source/WebCore/ChangeLog
@@ -1,3 +1,32 @@
+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>
 
         Web Inspector: home button behaviour is wrong in DTE

diff --git a/Source/WebCore/inspector/front-end/Color.js b/Source/WebCore/inspector/front-end/Color.js
index 3b3bfd1692abaa71fa557972b0690a2214b75f9e..92741a35ed12187fd3a6d8e8540a3a5ede674b3f 100644 (file)
--- a/Source/WebCore/inspector/front-end/Color.js
+++ b/Source/WebCore/inspector/front-end/Color.js
@@ -28,25 +28,30 @@
  */
 
 /**
- * @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
  */
 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 + ")");
 }
 
 WebInspector.Color.prototype = {
@@ -206,7 +211,7 @@ WebInspector.Color.prototype = {
                 return this.nickname;
         }
 
-        throw "invalid color format";
+        return "invalid color format: " + format;
     },
 
     /**
@@ -432,7 +437,7 @@ WebInspector.Color.prototype = {
             this.hsla = set[1];
             this.nickname = set[2];
             this.alpha = set[0][3];
-            return;
+            return true;
         }
 
         // Simple - #hex, rgb(), nickname, hsl()
@@ -461,7 +466,7 @@ WebInspector.Color.prototype = {
                     this.format = "nickname";
                     this.hex = WebInspector.Color.Nicknames[nickname];
                 } else // unknown name
-                    throw "unknown color name";
+                    return false;
             } else if (match[4]) { // hsl
                 this.format = "hsl";
                 var hsl = match[4].replace(/%/g, "").split(/\s*,\s*/);
@@ -479,7 +484,7 @@ WebInspector.Color.prototype = {
                 this.nickname = set[2];
             }
 
-            return;
+            return true;
         }
 
         // Advanced - rgba(), hsla()
@@ -499,11 +504,11 @@ WebInspector.Color.prototype = {
                 this.rgba = this._hslaToRGBA(this.hsla, this.alpha);
             }
 
-            return;
+            return true;
         }
 
         // Could not parse as a valid color
-        throw "could not parse color";
+        return false;
     }
 }
 

diff --git a/Source/WebCore/inspector/front-end/Spectrum.js b/Source/WebCore/inspector/front-end/Spectrum.js
index bfeadb51775f1b064bc8b2aeed7bfbecb0bd21b2..0f34968a51835f7c16a3656e35d95d74902f8bba 100644 (file)
--- a/Source/WebCore/inspector/front-end/Spectrum.js
+++ b/Source/WebCore/inspector/front-end/Spectrum.js
@@ -281,7 +281,7 @@ WebInspector.Spectrum.prototype = {
         var colorValue = color.toString(this.outputColorFormat);
         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);
     },
 
     get outputColorFormat()

diff --git a/Source/WebCore/inspector/front-end/StylesSidebarPane.js b/Source/WebCore/inspector/front-end/StylesSidebarPane.js
index 5918b5c2cb1aead892bcbae4f5a2644bc8e50346..ce9026d99e579ea195c99f69a229705cc5845b53 100644 (file)
--- a/Source/WebCore/inspector/front-end/StylesSidebarPane.js
+++ b/Source/WebCore/inspector/front-end/StylesSidebarPane.js
@@ -1756,12 +1756,12 @@ WebInspector.StylePropertyTreeElement.prototype = {
 
             function processColor(text)
             {
-                try {
-                    var color = new WebInspector.Color(text);
-                } catch (e) {
-                    return document.createTextNode(text);
-                }
+                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;
                 var spectrumHelper = hasSpectrum ? self._parentPane._spectrumHelper : null;