Bug 231334 - [css-ui] Parsing support for accent-color
Summary: [css-ui] Parsing support for accent-color
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: CSS (show other bugs)
Version: Other
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Aditya Keerthi
URL:
Keywords: InRadar
Depends on:
Blocks: 227587
  Show dependency treegraph
 
Reported: 2021-10-06 15:42 PDT by Aditya Keerthi
Modified: 2021-10-07 14:08 PDT (History)
15 users (show)

See Also:


Attachments
Patch (200.30 KB, patch)
2021-10-06 15:47 PDT, Aditya Keerthi
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (201.80 KB, patch)
2021-10-06 16:01 PDT, Aditya Keerthi
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (201.83 KB, patch)
2021-10-06 16:46 PDT, Aditya Keerthi
koivisto: review+
Details | Formatted Diff | Diff
Patch for landing (198.74 KB, patch)
2021-10-07 10:53 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-10-06 15:42:24 PDT
https://www.w3.org/TR/css-ui-4/#widget-accent
Comment 1 Radar WebKit Bug Importer 2021-10-06 15:42:59 PDT
<rdar://problem/83955508>
Comment 2 Aditya Keerthi 2021-10-06 15:47:06 PDT
Created attachment 440446 [details]
Patch
Comment 3 Aditya Keerthi 2021-10-06 16:01:54 PDT
Created attachment 440452 [details]
Patch
Comment 4 Aditya Keerthi 2021-10-06 16:36:20 PDT
Needs to be rebased after r283676.
Comment 5 Aditya Keerthi 2021-10-06 16:46:16 PDT
Created attachment 440456 [details]
Patch
Comment 6 Antti Koivisto 2021-10-07 09:47:43 PDT
Comment on attachment 440456 [details]
Patch

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

> Source/WebCore/css/CSSProperties.json:225
> +            "codegen-properties": {
> +                "color-property": true,
> +                "custom": "All",
> +                "settings-flag": "accentColorEnabled"
> +            },

Does this really need custom codegen? Wouldn't 

"auto-functions": true;

be sufficient?

> Source/WebCore/css/parser/CSSParserContext.cpp:159
> -        | context.aspectRatioEnabled                        << 4
> -        | context.colorContrastEnabled                      << 5
> -        | context.colorFilterEnabled                        << 6
> -        | context.colorMixEnabled                           << 7
> -        | context.constantPropertiesEnabled                 << 8
> -        | context.containmentEnabled                        << 9
> -        | context.cssColor4                                 << 10
> -        | context.deferredCSSParserEnabled                  << 11
> -        | context.individualTransformPropertiesEnabled      << 12
> +        | context.accentColorEnabled                        << 4

Could have just added this to the end and avoided all the movement. Not like these are consistently in sorted order.
Comment 7 Aditya Keerthi 2021-10-07 10:53:06 PDT
Created attachment 440513 [details]
Patch for landing
Comment 8 Aditya Keerthi 2021-10-07 10:59:42 PDT
(In reply to Antti Koivisto from comment #6)
> Comment on attachment 440456 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=440456&action=review
> 
> > Source/WebCore/css/CSSProperties.json:225
> > +            "codegen-properties": {
> > +                "color-property": true,
> > +                "custom": "All",
> > +                "settings-flag": "accentColorEnabled"
> > +            },
> 
> Does this really need custom codegen? Wouldn't 
> 
> "auto-functions": true;
> 
> be sufficient?

This doesn't currently work for color properties.

I'll add support here: https://bugs.webkit.org/show_bug.cgi?id=231376, and then make this change.

> > Source/WebCore/css/parser/CSSParserContext.cpp:159
> > -        | context.aspectRatioEnabled                        << 4
> > -        | context.colorContrastEnabled                      << 5
> > -        | context.colorFilterEnabled                        << 6
> > -        | context.colorMixEnabled                           << 7
> > -        | context.constantPropertiesEnabled                 << 8
> > -        | context.containmentEnabled                        << 9
> > -        | context.cssColor4                                 << 10
> > -        | context.deferredCSSParserEnabled                  << 11
> > -        | context.individualTransformPropertiesEnabled      << 12
> > +        | context.accentColorEnabled                        << 4
> 
> Could have just added this to the end and avoided all the movement. Not like
> these are consistently in sorted order.

Added to the end.
Comment 9 EWS 2021-10-07 14:08:18 PDT
Committed r283742 (242664@main): <https://commits.webkit.org/242664@main>

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