Bug 238345 - [css-cascade] makeprop.pl should sort deferred properties at the end
Summary: [css-cascade] makeprop.pl should sort deferred properties at the end
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: CSS (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Oriol Brufau
URL:
Keywords: InRadar
Depends on: 238350
Blocks: 238260
  Show dependency treegraph
 
Reported: 2022-03-24 13:06 PDT by Oriol Brufau
Modified: 2022-04-08 15:48 PDT (History)
11 users (show)

See Also:


Attachments
Patch (14.98 KB, patch)
2022-03-25 10:52 PDT, Oriol Brufau
no flags Details | Formatted Diff | Diff
Patch for EWS (48.15 KB, patch)
2022-03-25 10:53 PDT, Oriol Brufau
no flags Details | Formatted Diff | Diff
Patch (14.99 KB, patch)
2022-03-25 11:49 PDT, Oriol Brufau
no flags Details | Formatted Diff | Diff
Patch for EWS (48.16 KB, patch)
2022-03-25 11:50 PDT, Oriol Brufau
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Oriol Brufau 2022-03-24 13:06:36 PDT
Currently, the CSSPropertyID enum is sorted like this:

1. Special meaning: CSSPropertyInvalid and CSSPropertyCustom
2. High priority properties: firstCSSProperty..lastHighPriorityProperty
3. Low priority properties: (lastHighPriorityProperty+1)..lastCSSProperty

However, among low priority ones, there are some that are not cascaded with the others, they are deferred and applied in parse order.
These should be sorted into their own category at the end:

1. Special meaning: CSSPropertyInvalid and CSSPropertyCustom
2. High priority properties: firstCSSProperty..lastHighPriorityProperty
3. Low priority properties: (lastHighPriorityProperty+1)..lastLowPriorityProperty
4. Deferred properties: (lastLowPriorityProperty+1)..lastCSSProperty

This would help bug 238260.
Comment 1 Darin Adler 2022-03-24 13:55:44 PDT
Since this is a generated file, I suggest we generate first/last constants for each section.

    first/lastCSSProperty
    first/lastHighPriorityCSSProperty
    first/lastLowPriorityCSSProperty
    first/lastDeferredCSSProperty

That way, most code that wants to make checks can be written without depending on the ordering at all; the few places that do depend on the ordering can be written in the clearest fashion possible.

Somewhere we could even put functions so we don't have to use these constants directly so often:

    bool isHighPriority(CSSPropertyID);
    bool isLowPriority(CSSPropertyID);
    bool isDeferred(CSSPropertyID);

Or whatever predicates we need. I think it helps when code can write such things so they read cleanly even if they are just expanding to < and > checks.
Comment 2 Oriol Brufau 2022-03-24 14:22:34 PDT
Yes, actually StyleBuilder.cpp already has

  static const CSSPropertyID firstLowPriorityProperty = static_cast<CSSPropertyID>(lastHighPriorityProperty + 1);
Comment 3 Radar WebKit Bug Importer 2022-03-24 16:06:24 PDT
<rdar://problem/90799486>
Comment 4 Oriol Brufau 2022-03-25 10:52:16 PDT
Created attachment 455784 [details]
Patch
Comment 5 Oriol Brufau 2022-03-25 10:53:03 PDT
Created attachment 455785 [details]
Patch for EWS
Comment 6 Oriol Brufau 2022-03-25 11:49:18 PDT
Created attachment 455789 [details]
Patch
Comment 7 Oriol Brufau 2022-03-25 11:50:01 PDT
Created attachment 455791 [details]
Patch for EWS
Comment 8 EWS 2022-04-08 15:48:20 PDT
Committed r292639 (249459@main): <https://commits.webkit.org/249459@main>

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