Bug 232031

Summary:

[macOS] Update appearance of <datalist> indicator

Product:

WebKit

Reporter:

Aditya Keerthi <akeerthi>

Component:

Forms

Assignee:

Aditya Keerthi <akeerthi>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

cdumez, changseok, esprehn+autocc, ews-watchlist, glenn, kondapallykalyan, pdr, thorton, wenson_hsieh

Priority:

Keywords:

InRadar

Version:

Other

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
Patch	none
Patch	wenson_hsieh: review+
Patch for landing	none

Description Aditya Keerthi 2021-10-20 12:28:13 PDT

The appearance of system combo boxes was updated in Big Sur. The <datalist> indicator should have the new button style.

Comment 1 Aditya Keerthi 2021-10-20 12:28:25 PDT

rdar://84474135

Comment 2 Aditya Keerthi 2021-10-20 12:31:02 PDT

Created attachment 441915 [details]
Patch

Comment 3 Aditya Keerthi 2021-10-20 13:02:33 PDT

Created attachment 441923 [details]
Patch

Comment 4 Wenson Hsieh 2021-10-21 09:27:45 PDT

Comment on attachment 441923 [details]
Patch

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

> Source/WebCore/rendering/RenderThemeMac.mm:1019
> +        if (auto* buttonElement = input.dataListButtonElement())
> +            coreUIState = (__bridge NSString *)(buttonElement->active() ? kCUIStatePressed : kCUIStateActive);
> +        else
> +            coreUIState = (__bridge NSString *)kCUIStateActive;

Nit - might look cleaner with a top-level `else if` like:

```
if (input.isPresentingAttachedView())
    coreUIState = (__bridge NSString *)kCUIStatePressed;
else if (auto* buttonElement = input.dataListButtonElement())
    coreUIState = (__bridge NSString *)(buttonElement->active() ? kCUIStatePressed : kCUIStateActive);
else
    coreUIState = (__bridge NSString *)kCUIStateActive;
```

Comment 5 Aditya Keerthi 2021-10-21 10:14:01 PDT

Created attachment 442045 [details]
Patch for landing

Comment 6 Aditya Keerthi 2021-10-21 10:14:51 PDT

(In reply to Wenson Hsieh from comment #4)
> Comment on attachment 441923 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=441923&action=review
> 
> > Source/WebCore/rendering/RenderThemeMac.mm:1019
> > +        if (auto* buttonElement = input.dataListButtonElement())
> > +            coreUIState = (__bridge NSString *)(buttonElement->active() ? kCUIStatePressed : kCUIStateActive);
> > +        else
> > +            coreUIState = (__bridge NSString *)kCUIStateActive;
> 
> Nit - might look cleaner with a top-level `else if` like:
> 
> ```
> if (input.isPresentingAttachedView())
>     coreUIState = (__bridge NSString *)kCUIStatePressed;
> else if (auto* buttonElement = input.dataListButtonElement())
>     coreUIState = (__bridge NSString *)(buttonElement->active() ?
> kCUIStatePressed : kCUIStateActive);
> else
>     coreUIState = (__bridge NSString *)kCUIStateActive;
> ```

Done.

Thanks for the review!

Comment 7 EWS 2021-10-21 11:05:31 PDT

Committed r284626 (243347@main): <https://commits.webkit.org/243347@main>

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