Bug 206202

Summary: REGRESSION(r251487): Web Inspector: selected color in color picker has wrong lightness
Product: WebKit Reporter: Nikita Vasilyev <nvasilyev>
Component: Web InspectorAssignee: Nikita Vasilyev <nvasilyev>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, hi, inspector-bugzilla-changes, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: All   
OS: All   
Attachments:
Description Flags
[Image] Bug
none
Patch
none
Patch
hi: review+
Patch none

Nikita Vasilyev
Reported 2020-01-13 15:47:19 PST
Created attachment 387581 [details] [Image] Bug ColorSquare shows the wrong color selected.
Attachments
[Image] Bug (266.12 KB, image/png)
2020-01-13 15:47 PST, Nikita Vasilyev 		no flags
Details
Patch (2.88 KB, patch)
2020-01-13 15:57 PST, Nikita Vasilyev 		no flags
DetailsFormatted DiffDiff
Patch (2.86 KB, patch)
2020-01-13 16:00 PST, Nikita Vasilyev 		hi: review+
DetailsFormatted DiffDiff
Patch (2.83 KB, patch)
2020-01-16 16:48 PST, Nikita Vasilyev 		no flags
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Nikita Vasilyev
Comment 1 2020-01-13 15:57:53 PST
Created attachment 387582 [details] Patch
Radar WebKit Bug Importer
Comment 2 2020-01-13 15:58:14 PST
<rdar://problem/58548071>
Nikita Vasilyev
Comment 3 2020-01-13 16:00:45 PST
Created attachment 387584 [details] Patch Retitled.
Devin Rousso
Comment 4 2020-01-16 16:36:27 PST
Comment on attachment 387584 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=387584&action=review r=me > Source/WebInspectorUI/UserInterface/Views/ColorSquare.js:109 > + let hsv = WI.Color.rgb2hsv(...tintedColor.normalizedRGB); NIT: we can destructure this for better code clarity: ``` let [hue, saturation] = WI.Color.rgb2hsv(...tintedColor.normalizedRGB); let x = hue / 100 * this._dimension; let y = (1 - (saturation / 100)) * this._dimension; ``` > Source/WebInspectorUI/UserInterface/Views/ColorSquare.js:112 > + this._setCrosshairPosition(new WI.Point(x, y)); This is called again below. Please remove it, or add a comment explaining why it has to be called twice.
Nikita Vasilyev
Comment 5 2020-01-16 16:45:21 PST
Comment on attachment 387584 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=387584&action=review >> Source/WebInspectorUI/UserInterface/Views/ColorSquare.js:109 >> + let hsv = WI.Color.rgb2hsv(...tintedColor.normalizedRGB); > > NIT: we can destructure this for better code clarity: > ``` > let [hue, saturation] = WI.Color.rgb2hsv(...tintedColor.normalizedRGB); > let x = hue / 100 * this._dimension; > let y = (1 - (saturation / 100)) * this._dimension; > ``` I'm not using the 1st array's item, hue. >> Source/WebInspectorUI/UserInterface/Views/ColorSquare.js:112 >> + this._setCrosshairPosition(new WI.Point(x, y)); > > This is called again below. Please remove it, or add a comment explaining why it has to be called twice. This was a mistake. Thanks!
Nikita Vasilyev
Comment 6 2020-01-16 16:48:08 PST
Created attachment 387987 [details] Patch
WebKit Commit Bot
Comment 7 2020-01-16 17:08:26 PST
Comment on attachment 387987 [details] Patch Clearing flags on attachment: 387987 Committed r254726: <https://trac.webkit.org/changeset/254726>
WebKit Commit Bot
Comment 8 2020-01-16 17:08:28 PST
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.