From 3b8e343f4f244f3b4007b2aee7eefd4446539f8f Mon Sep 17 00:00:00 2001 From: "commit-queue@webkit.org" Date: Mon, 11 Feb 2013 09:39:32 +0000 Subject: [PATCH] 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 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 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 --- LayoutTests/ChangeLog | 12 +++++++ .../styles-invalid-color-values-expected.txt | 4 +++ .../styles/styles-invalid-color-values.html | 31 ++++++++++++++++--- Source/WebCore/ChangeLog | 29 +++++++++++++++++ Source/WebCore/inspector/front-end/Color.js | 25 +++++++++------ .../WebCore/inspector/front-end/Spectrum.js | 2 +- .../inspector/front-end/StylesSidebarPane.js | 10 +++--- 7 files changed, 92 insertions(+), 21 deletions(-) diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog index f5848a0e84e3..a91571df23f6 100644 --- a/LayoutTests/ChangeLog +++ b/LayoutTests/ChangeLog @@ -1,3 +1,15 @@ +2013-02-11 John J. Barton + + 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 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 dd4920ad3572..8a55280e9a3a 100644 --- 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 6ee4a2019f93..a0f7bd2b1100 100644 --- 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 b158a76932b9..e78937fc1079 100644 --- a/Source/WebCore/ChangeLog +++ b/Source/WebCore/ChangeLog @@ -1,3 +1,32 @@ +2013-02-11 John J. Barton + + 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 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 3b3bfd1692ab..92741a35ed12 100644 --- 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 bfeadb51775f..0f34968a5183 100644 --- 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 5918b5c2cb1a..ce9026d99e57 100644 --- 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; -- 2.36.0