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

Aditya Keerthi

Reported 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.

Attachments
Patch (6.24 KB, patch) 2021-10-20 12:31 PDT, Aditya Keerthi	no flags	Details Formatted Diff Diff
Patch (6.18 KB, patch) 2021-10-20 13:02 PDT, Aditya Keerthi	wenson_hsieh: review+	Details Formatted Diff Diff
Patch for landing (6.15 KB, patch) 2021-10-21 10:14 PDT, Aditya Keerthi	no flags	Details Formatted Diff Diff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.

Aditya Keerthi

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

rdar://84474135

Aditya Keerthi

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

Created attachment 441915 [details] Patch

Aditya Keerthi

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

Created attachment 441923 [details] Patch

Wenson Hsieh

Comment 4 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; ```

Aditya Keerthi

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

Created attachment 442045 [details] Patch for landing

Aditya Keerthi

Comment 6 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!

EWS

Comment 7 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].

Note You need to log in before you can comment on or make changes to this bug.