Bug 223647 - Use enum classes and OptionSets for ControlStates::States
Summary: Use enum classes and OptionSets for ControlStates::States
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: Other
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Aditya Keerthi
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2021-03-23 11:02 PDT by Aditya Keerthi
Modified: 2021-03-29 09:32 PDT (History)
15 users (show)

See Also:


Attachments
Patch (32.65 KB, patch)
2021-03-26 11:38 PDT, Aditya Keerthi
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Aditya Keerthi 2021-03-23 11:02:25 PDT
...
Comment 1 Aditya Keerthi 2021-03-26 11:38:34 PDT
Created attachment 424383 [details]
Patch
Comment 2 Simon Fraser (smfr) 2021-03-26 11:57:30 PDT
Comment on attachment 424383 [details]
Patch

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

> Source/WebCore/platform/ControlStates.h:57
> +        SpinUp = 1 << 8, // Sub-state for HoverState and PressedState.

Should we instead just use { Hovered, Pressed } ?

> Source/WebCore/platform/adwaita/ThemeAdwaita.cpp:262
> +    else if (states.states().contains(ControlStates::States::Hovered))

So much states
Comment 3 Aditya Keerthi 2021-03-26 12:18:31 PDT
(In reply to Simon Fraser (smfr) from comment #2)
> Comment on attachment 424383 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=424383&action=review
> 
> > Source/WebCore/platform/ControlStates.h:57
> > +        SpinUp = 1 << 8, // Sub-state for HoverState and PressedState.
> 
> Should we instead just use { Hovered, Pressed } ?

{ SpinUp } != { Hovered, Pressed }.

From RenderTheme::extractControlStatesForRenderer:

    if (isHovered(o)) {
        states.add(ControlStates::States::Hovered);
        if (isSpinUpButtonPartHovered(o))
            states.add(ControlStates::States::SpinUp);
    }
    if (isPressed(o)) {
        states.add(ControlStates::States::Pressed);
        if (isSpinUpButtonPartPressed(o))
            states.add(ControlStates::States::SpinUp);
    }
Comment 4 EWS 2021-03-29 09:31:54 PDT
Committed r275161: <https://commits.webkit.org/r275161>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 424383 [details].
Comment 5 Radar WebKit Bug Importer 2021-03-29 09:32:14 PDT
<rdar://problem/75957697>