Bug 227941 - Web Inspector: REGRESSION(?): alpha slider doesn't match alpha input for `transparent` in color picker
Summary: Web Inspector: REGRESSION(?): alpha slider doesn't match alpha input for `tra...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (show other bugs)
Version: WebKit Nightly Build
Hardware: All All
: P2 Normal
Assignee: Nikita Vasilyev
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2021-07-13 22:11 PDT by Devin Rousso
Modified: 2021-07-19 13:50 PDT (History)
4 users (show)

See Also:


Attachments
Patch (1.29 KB, patch)
2021-07-19 10:58 PDT, Nikita Vasilyev
no flags Details | Formatted Diff | Diff
Patch (1.29 KB, patch)
2021-07-19 11:23 PDT, Nikita Vasilyev
no flags Details | Formatted Diff | Diff
Patch (1.35 KB, patch)
2021-07-19 11:31 PDT, Nikita Vasilyev
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Devin Rousso 2021-07-13 22:11:26 PDT
# STEPS TO REPRODUCE
1. inspect any page
2. add CSS `color: transparent`
3. click the color swatch to show the color picker

# EXPECTED
the alpha slider would be at the bottom and the alpha input would say `0`

# ACTUAL
the alpha slider is at the top and the alpha input says `0`
Comment 1 Radar WebKit Bug Importer 2021-07-13 22:11:43 PDT
<rdar://problem/80557438>
Comment 2 Nikita Vasilyev 2021-07-19 10:56:01 PDT
Reduction:
data:text/html,<p%20style="color:transparent">color
Comment 3 Nikita Vasilyev 2021-07-19 10:58:18 PDT
Created attachment 433799 [details]
Patch
Comment 4 Devin Rousso 2021-07-19 11:05:05 PDT
Comment on attachment 433799 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=433799&action=review

> Source/WebInspectorUI/ChangeLog:11
> +        When the initial value was set to 0, the `set value` was exiting early when the passed value was 0.

Based on this, it seems like the real issue is that the initial value of `_knobY` is really the incorrect value?  Changing to `NaN` seems a bit scary since there's no handling of `isNaN` anywhere in this file.  Perhaps we just need to add a `recalculateKnobY` call inside the early return in `set value`?
Comment 5 Nikita Vasilyev 2021-07-19 11:22:55 PDT
This works as well.
Comment 6 Nikita Vasilyev 2021-07-19 11:23:09 PDT
Created attachment 433801 [details]
Patch
Comment 7 Devin Rousso 2021-07-19 11:24:36 PDT
Comment on attachment 433801 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=433801&action=review

> Source/WebInspectorUI/UserInterface/Views/Slider.js:61
> +        this.recalculateKnobY();

We should only need to do this inside the `if (value === this._value)`.  As you have it now, we'd call `set knobY` twice in the non-early-return path.
Comment 8 Nikita Vasilyev 2021-07-19 11:31:16 PDT
Created attachment 433804 [details]
Patch
Comment 9 Devin Rousso 2021-07-19 11:39:11 PDT
Comment on attachment 433804 [details]
Patch

r=me
Comment 10 EWS 2021-07-19 13:50:03 PDT
Committed r280045 (239780@main): <https://commits.webkit.org/239780@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 433804 [details].