Bug 231464

Summary:

[macOS] Add support for accent-color

Product:

WebKit

Reporter:

Aditya Keerthi <akeerthi>

Component:

CSS

Assignee:

Aditya Keerthi <akeerthi>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

changseok, clopez, dvpdiner2, esprehn+autocc, ews-watchlist, glenn, kondapallykalyan, pdr, simon.fraser, thorton, webkit-bug-importer, wenson_hsieh, youennf

Priority:

Keywords:

InRadar, WebExposed

Version:

Other

Hardware:

Unspecified

OS:

Unspecified

See Also:

https://bugs.webkit.org/show_bug.cgi?id=227587

Attachments:

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

Description Aditya Keerthi 2021-10-08 17:22:43 PDT

Add support for accent-color when painting controls on macOS.

Comment 1 Aditya Keerthi 2021-10-08 17:22:57 PDT

rdar://84049511

Comment 2 Aditya Keerthi 2021-10-08 18:16:42 PDT

Created attachment 440697 [details]
Patch

Comment 3 EWS Watchlist 2021-10-08 18:17:35 PDT

This patch modifies the imported WPT tests. Please ensure that any changes on the tests (not coming from a WPT import) are exported to WPT. Please see https://trac.webkit.org/wiki/WPTExportProcess

Comment 4 Aditya Keerthi 2021-10-08 18:31:06 PDT

Created attachment 440698 [details]
Patch

Comment 5 Aditya Keerthi 2021-10-10 18:01:15 PDT

Created attachment 440748 [details]
Patch

Comment 6 Tim Horton 2021-10-12 14:07:38 PDT

Comment on attachment 440748 [details]
Patch

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

> Source/WebCore/platform/Theme.cpp:73
> +void Theme::paint(ControlPart, ControlStates&, GraphicsContext&, const FloatRect&, float, ScrollView*, float, float, bool, bool, const Color&)

At what point do we package this up in a struct or something?

> Source/WebCore/rendering/style/RenderStyle.cpp:2182
> +    if (!hasAppleColorFilter())
> +        return colorResolvingCurrentColor(accentColor());

Kinda would expect the weird case (color-filter) to be the early return (but this is fine too).

> LayoutTests/platform/mac/TestExpectations:2350
> +webkit.org/b/199350 fast/css/accent-color/datalist.html [ ImageOnlyFailure ]

Maybe these two failing ones go in their own paragraph that doesn't have a header suggesting these are passing because they're supported?

Comment 7 Aditya Keerthi 2021-10-12 15:26:34 PDT

Created attachment 441002 [details]
Patch for landing

Comment 8 Aditya Keerthi 2021-10-12 15:27:43 PDT

(In reply to Tim Horton from comment #6)
> Comment on attachment 440748 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=440748&action=review
> 
> > Source/WebCore/platform/Theme.cpp:73
> > +void Theme::paint(ControlPart, ControlStates&, GraphicsContext&, const FloatRect&, float, ScrollView*, float, float, bool, bool, const Color&)
> 
> At what point do we package this up in a struct or something?

Filed https://bugs.webkit.org/show_bug.cgi?id=231637.
 
> > Source/WebCore/rendering/style/RenderStyle.cpp:2182
> > +    if (!hasAppleColorFilter())
> > +        return colorResolvingCurrentColor(accentColor());
> 
> Kinda would expect the weird case (color-filter) to be the early return (but
> this is fine too).

Changed.
 
> > LayoutTests/platform/mac/TestExpectations:2350
> > +webkit.org/b/199350 fast/css/accent-color/datalist.html [ ImageOnlyFailure ]
> 
> Maybe these two failing ones go in their own paragraph that doesn't have a
> header suggesting these are passing because they're supported?

Done.

Comment 9 EWS 2021-10-13 11:05:28 PDT

Committed r284115 (242935@main): <https://commits.webkit.org/242935@main>

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

Comment 10 Aditya Keerthi 2021-10-13 11:20:15 PDT

Note that this is behind a disabled-by-default experimental flag.