[Webkit-unassigned] [Bug 104835] Web Inspector: Don't throw exceptions in WebInspector.Color

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=104835





--- Comment #13 from Yury Semikhatsky <yurys at chromium.org>  2013-01-16 05:52:00 PST ---
(From update of attachment 182597)
View in context: https://bugs.webkit.org/attachment.cgi?id=182597&action=review

>>> LayoutTests/inspector/styles/styles-invalid-color-values.html:39
>>> +            InspectorTest.addResult("invalid color '" + colorString + "' parses to null.");
>> 
>> I'd rather leave this function untouched and use another function (dumpInvalidColorRepresentations?) for invalid colors as they have inverted expectation compared to valid colors. With the current approach it is not clear if "invalid color 'rgba(255, 0, 0, 5)' parses to null." would be a wrong result.
> 
> Would another wording help? There is no way for the code to determine that "invalid color 'rgba(255, 0, 0, 5)' parses to null." is a wrong result: this code tests the parser and only the parser can make this determination. A human has to understand the output and validate that such a result is incorrect. To me, such output is certainly puzzling and I would check it. Since the test expected result would list a valid output for that input, it seems unlikely to me that this error would occur.
> 
> Leaving the current function would be misleading the try/catch cannot be reached since the parser no longer throws (by design).  Better to have the test code more closely resemble the way the API is used right?

My concern was that someone unfamiliar with the code might rebase the test with a wrong output as it would look fine. To avoid such situations it would make sense to produce either "SUCCESS: parsed invalid color to null" or "FAIL: parsed invalid color to not null" so that it is clear that the result is a failure.

You are right, there is not much sense in leaving try/catch block. My point was to split function testColors(next) into testValidColors and testInvalidColors that would dump results in different manners.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.