[webkit-reviews] review denied: [Bug 69152] Web Inspector: rgb() with percentages shows wrong hex/hsl values : [Attachment 109336] [PATCH] Fix RGB Percentage Values and Other Clamping

[bugzilla-daemon at webkit.org]

Pavel Feldman <pfeldman at chromium.org> has denied Joseph Pecoraro
<joepeck at webkit.org>'s request for review:
Bug 69152: Web Inspector: rgb() with percentages shows wrong hex/hsl values
https://bugs.webkit.org/show_bug.cgi?id=69152

Attachment 109336: [PATCH] Fix RGB Percentage Values and Other Clamping
https://bugs.webkit.org/attachment.cgi?id=109336&action=review

------- Additional Comments from Pavel Feldman <pfeldman at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=109336&action=review


r- for the test, otherwise looks good!

> LayoutTests/ChangeLog:8
> +	   THe test outputs the Original (Authored), RGB, Nickname, Hex, and
HSL

Nit: The test

> LayoutTests/ChangeLog:9
> +	   representations for a specified nodes. For some of the CSS values
their

Please consider rephrasing.

> LayoutTests/inspector/styles/styles-invalid-color-values.html:23
> +    InspectorTest.runTestSuite([

Style tests are slow and flaky. In this case, you should do a simple test suite
that would execute code snippets against Color object.

> LayoutTests/inspector/styles/styles-invalid-color-values.html:31
> +	      
WebInspector.settings.colorFormat.set(WebInspector.StylesSidebarPane.ColorForma
t.Original);

Mutating settings from tests is risky: we don't reset them between the test
runs + since this is localStorage, parallel test runner might suffer.