Bug 198680 - Automate generation of computedProperties
Summary: Automate generation of computedProperties
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: CSS (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Devin Rousso
URL:
Keywords: InRadar
Depends on: 198742 198743
Blocks: 203943
  Show dependency treegraph
 
Reported: 2019-06-07 14:43 PDT by Devin Rousso
Modified: 2019-11-06 21:59 PST (History)
14 users (show)

See Also:


Attachments
Patch (17.45 KB, patch)
2019-06-08 14:55 PDT, Devin Rousso
no flags Details | Formatted Diff | Diff
Old computedPropertyIDs (11.02 KB, text/plain)
2019-06-08 14:56 PDT, Devin Rousso
no flags Details
New computedPropertyIDs (11.02 KB, text/plain)
2019-06-08 14:56 PDT, Devin Rousso
no flags Details
Diff (ordered) of computedPropertyIDs (12.51 KB, text/plain)
2019-06-08 14:56 PDT, Devin Rousso
no flags Details
Diff (alphabetized) of computedPropertyIDs (2.97 KB, text/plain)
2019-06-08 14:57 PDT, Devin Rousso
no flags Details
Patch (17.63 KB, patch)
2019-06-08 15:09 PDT, Devin Rousso
no flags Details | Formatted Diff | Diff
New computedPropertyIDs (11.64 KB, text/plain)
2019-06-08 15:14 PDT, Devin Rousso
no flags Details
Patch (17.95 KB, patch)
2019-06-08 15:25 PDT, Devin Rousso
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews102 for mac-highsierra (3.21 MB, application/zip)
2019-06-08 16:24 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews106 for mac-highsierra-wk2 (2.99 MB, application/zip)
2019-06-08 16:33 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews122 for ios-simulator-wk2 (2.67 MB, application/zip)
2019-06-08 17:19 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews113 for mac-highsierra (3.06 MB, application/zip)
2019-06-08 17:20 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews211 for win-future (13.81 MB, application/zip)
2019-06-08 19:55 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews215 for win-future (13.53 MB, application/zip)
2019-06-08 19:58 PDT, Build Bot
no flags Details
Patch (20.11 KB, patch)
2019-06-10 14:30 PDT, Devin Rousso
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews100 for mac-highsierra (3.38 MB, application/zip)
2019-06-10 15:47 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews106 for mac-highsierra-wk2 (3.22 MB, application/zip)
2019-06-10 15:55 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews112 for mac-highsierra (3.22 MB, application/zip)
2019-06-10 16:25 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews121 for ios-simulator-wk2 (2.79 MB, application/zip)
2019-06-10 16:37 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews212 for win-future (13.86 MB, application/zip)
2019-06-10 16:44 PDT, Build Bot
no flags Details
Patch (20.62 KB, patch)
2019-06-10 17:13 PDT, Devin Rousso
no flags Details | Formatted Diff | Diff
Patch (20.56 KB, patch)
2019-06-10 17:17 PDT, Devin Rousso
ews: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from ews106 for mac-highsierra-wk2 (3.73 MB, application/zip)
2019-06-10 18:24 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews100 for mac-highsierra (3.38 MB, application/zip)
2019-06-10 19:09 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews121 for ios-simulator-wk2 (2.82 MB, application/zip)
2019-06-10 19:23 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews115 for mac-highsierra (3.18 MB, application/zip)
2019-06-10 19:36 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews212 for win-future (13.92 MB, application/zip)
2019-06-10 20:19 PDT, Build Bot
no flags Details
Patch (18.63 KB, patch)
2019-06-11 17:21 PDT, Devin Rousso
no flags Details | Formatted Diff | Diff
computedPropertyIDs (after) (macOS) (11.25 KB, text/plain)
2019-06-11 17:40 PDT, Devin Rousso
no flags Details
Archive of layout-test-results from ews101 for mac-highsierra (3.43 MB, application/zip)
2019-06-11 18:37 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews104 for mac-highsierra-wk2 (3.31 MB, application/zip)
2019-06-11 18:44 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews113 for mac-highsierra (3.33 MB, application/zip)
2019-06-11 19:15 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews122 for ios-simulator-wk2 (3.01 MB, application/zip)
2019-06-11 19:16 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews214 for win-future (13.62 MB, application/zip)
2019-06-11 19:31 PDT, Build Bot
no flags Details
Patch (99.19 KB, patch)
2019-06-11 20:35 PDT, Devin Rousso
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews100 for mac-highsierra (3.35 MB, application/zip)
2019-06-11 21:53 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews106 for mac-highsierra-wk2 (3.15 MB, application/zip)
2019-06-11 22:00 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews114 for mac-highsierra (3.13 MB, application/zip)
2019-06-11 22:29 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews126 for ios-simulator-wk2 (2.70 MB, application/zip)
2019-06-11 22:42 PDT, Build Bot
no flags Details
CSSComputedStyleDeclaration.cpp.diff (macOS) (13.37 KB, text/plain)
2019-06-12 11:20 PDT, Devin Rousso
no flags Details
Patch (92.69 KB, patch)
2019-06-12 11:41 PDT, Devin Rousso
no flags Details | Formatted Diff | Diff
Patch (106.39 KB, patch)
2019-06-13 20:34 PDT, Devin Rousso
no flags Details | Formatted Diff | Diff
Patch (106.46 KB, patch)
2019-10-24 13:02 PDT, Devin Rousso
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Devin Rousso 2019-06-07 14:43:14 PDT
AFAICT, there's no good reason why we don't generate the `computedProperties` list in CSSComputedStyleDeclaration.cpp from CSSProperties.json (or some other source).  I think we should migrate to a blacklist (take the full list of `CSSPropertyID` and remove those that aren't a longhand property) instead of a whitelist (what we have now, where it's a hardcoded list of items to include).
Comment 1 Devin Rousso 2019-06-08 14:55:30 PDT
Created attachment 371670 [details]
Patch
Comment 2 Devin Rousso 2019-06-08 14:56:03 PDT
Created attachment 371671 [details]
Old computedPropertyIDs
Comment 3 Devin Rousso 2019-06-08 14:56:24 PDT
Created attachment 371672 [details]
New computedPropertyIDs
Comment 4 Devin Rousso 2019-06-08 14:56:59 PDT
Created attachment 371673 [details]
Diff (ordered) of computedPropertyIDs
Comment 5 Devin Rousso 2019-06-08 14:57:44 PDT
Created attachment 371674 [details]
Diff (alphabetized) of computedPropertyIDs
Comment 6 Devin Rousso 2019-06-08 15:09:33 PDT
Created attachment 371675 [details]
Patch
Comment 7 Devin Rousso 2019-06-08 15:14:55 PDT
Created attachment 371676 [details]
New computedPropertyIDs
Comment 8 Devin Rousso 2019-06-08 15:15:39 PDT
Comment on attachment 371675 [details]
Patch

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

> Source/WebCore/css/makeprop.pl:493
> +    my @longhands = @{$propertiesWithStyleBuilderOptions{$name}{"longhands"}};

Should we not skip properties that only have one longhand property?  This way, properties like `page-break-after` would also be shown with `break-after`.
Comment 9 Devin Rousso 2019-06-08 15:25:22 PDT
Created attachment 371677 [details]
Patch
Comment 10 Devin Rousso 2019-06-08 15:27:18 PDT
Comment on attachment 371677 [details]
Patch

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

> Source/WebCore/css/makeprop.pl:493
> +    if (scalar @longhands != 1) {

Adding this causes the following to also be shown:
 - CSSPropertyAll
 - CSSPropertyPageBreakAfter
 - CSSPropertyPageBreakBefore
 - CSSPropertyPageBreakInside
 - CSSPropertyWebkitColumnBreakAfter
 - CSSPropertyWebkitColumnBreakBefore
 - CSSPropertyWebkitColumnBreakInside
Comment 11 Build Bot 2019-06-08 16:24:30 PDT Comment hidden (obsolete)
Comment 12 Build Bot 2019-06-08 16:24:32 PDT Comment hidden (obsolete)
Comment 13 Build Bot 2019-06-08 16:33:07 PDT Comment hidden (obsolete)
Comment 14 Build Bot 2019-06-08 16:33:08 PDT Comment hidden (obsolete)
Comment 15 Build Bot 2019-06-08 17:19:25 PDT Comment hidden (obsolete)
Comment 16 Build Bot 2019-06-08 17:19:26 PDT Comment hidden (obsolete)
Comment 17 Build Bot 2019-06-08 17:20:54 PDT Comment hidden (obsolete)
Comment 18 Build Bot 2019-06-08 17:20:56 PDT Comment hidden (obsolete)
Comment 19 Build Bot 2019-06-08 19:54:57 PDT Comment hidden (obsolete)
Comment 20 Build Bot 2019-06-08 19:55:00 PDT Comment hidden (obsolete)
Comment 21 Build Bot 2019-06-08 19:58:56 PDT Comment hidden (obsolete)
Comment 22 Build Bot 2019-06-08 19:58:59 PDT Comment hidden (obsolete)
Comment 23 Devin Rousso 2019-06-10 11:38:57 PDT Comment hidden (obsolete)
Comment 24 Devin Rousso 2019-06-10 14:30:04 PDT
Created attachment 371775 [details]
Patch

Getting the results from the bots -.-
Comment 25 Joseph Pecoraro 2019-06-10 15:08:55 PDT
Comment on attachment 371775 [details]
Patch

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

Seems fine.

> Source/WebCore/ChangeLog:9
> +        A property should be listed as part of a `CSSStyleâDeclarationâ` if:

Weird characters?

> Source/WebCore/css/CSSComputedStyleDeclaration.cpp:-96
> -// List of all properties we know how to compute, omitting shorthands.
> -static const CSSPropertyID computedProperties[] = {

What is the diff between the old list and the new list? I assume touch-action, but did we add any others?

> Source/WebCore/css/makeprop.pl:487
> +print HEADER "static const CSSPropertyID computedPropertyIDs[] = {\n";
> +my $numComputedPropertyIDs = 0;
> +foreach my $name (@names) {

Should we sort this list? I think that would be convenient, it was semi-sorted before.

> Source/WebCore/css/makeprop.pl:491
> +  # Skip Shorthand properties if they have a non-internal longhand property.

Drop "Shorthand" and just say `Skip properties if they have a non-internal longhand property.`?

> Source/WebCore/css/makeprop.pl:509
> +  $numComputedPropertyIDs = $numComputedPropertyIDs + 1;

You can use ++ or just += in perl.

    $numComputedPropertyIDs += 1;

> LayoutTests/ChangeLog:9
> +        * imported/w3c/web-platform-tests/infrastructure/assumptions/html-elements-expected.txt:

Ew, I wonder what this test is doing...
Comment 26 Build Bot 2019-06-10 15:47:34 PDT Comment hidden (obsolete)
Comment 27 Build Bot 2019-06-10 15:47:36 PDT Comment hidden (obsolete)
Comment 28 Build Bot 2019-06-10 15:55:39 PDT Comment hidden (obsolete)
Comment 29 Build Bot 2019-06-10 15:55:41 PDT Comment hidden (obsolete)
Comment 30 Build Bot 2019-06-10 16:25:12 PDT Comment hidden (obsolete)
Comment 31 Build Bot 2019-06-10 16:25:14 PDT Comment hidden (obsolete)
Comment 32 Build Bot 2019-06-10 16:37:31 PDT Comment hidden (obsolete)
Comment 33 Build Bot 2019-06-10 16:37:33 PDT Comment hidden (obsolete)
Comment 34 Build Bot 2019-06-10 16:44:20 PDT Comment hidden (obsolete)
Comment 35 Build Bot 2019-06-10 16:44:23 PDT Comment hidden (obsolete)
Comment 36 Devin Rousso 2019-06-10 17:13:26 PDT
Created attachment 371796 [details]
Patch

Alphabetize the enumerated values
Comment 37 Devin Rousso 2019-06-10 17:17:16 PDT
Created attachment 371797 [details]
Patch

Rebase
Comment 38 Build Bot 2019-06-10 18:24:54 PDT Comment hidden (obsolete)
Comment 39 Build Bot 2019-06-10 18:24:56 PDT Comment hidden (obsolete)
Comment 40 Build Bot 2019-06-10 19:09:06 PDT Comment hidden (obsolete)
Comment 41 Build Bot 2019-06-10 19:09:08 PDT Comment hidden (obsolete)
Comment 42 Build Bot 2019-06-10 19:23:14 PDT Comment hidden (obsolete)
Comment 43 Build Bot 2019-06-10 19:23:16 PDT Comment hidden (obsolete)
Comment 44 Build Bot 2019-06-10 19:36:11 PDT Comment hidden (obsolete)
Comment 45 Build Bot 2019-06-10 19:36:13 PDT Comment hidden (obsolete)
Comment 46 Build Bot 2019-06-10 20:19:43 PDT Comment hidden (obsolete)
Comment 47 Build Bot 2019-06-10 20:19:46 PDT Comment hidden (obsolete)
Comment 48 Devin Rousso 2019-06-10 22:48:08 PDT
I've split the work for this into three separate bugs:
 - <https://webkit.org/b/198742> Include `touch-action` in the computed styles list
 - <https://webkit.org/b/198743> Sort the computed styles list
 - <https://webkit.org/b/198680> Automate generation of computedProperties
Comment 49 Radar WebKit Bug Importer 2019-06-10 22:49:31 PDT
<rdar://problem/51611113>
Comment 50 Devin Rousso 2019-06-11 17:21:10 PDT
Created attachment 371899 [details]
Patch
Comment 51 Devin Rousso 2019-06-11 17:40:47 PDT
Created attachment 371901 [details]
computedPropertyIDs (after) (macOS)
Comment 52 Build Bot 2019-06-11 18:37:15 PDT Comment hidden (obsolete)
Comment 53 Build Bot 2019-06-11 18:37:18 PDT Comment hidden (obsolete)
Comment 54 Build Bot 2019-06-11 18:44:52 PDT Comment hidden (obsolete)
Comment 55 Build Bot 2019-06-11 18:44:54 PDT Comment hidden (obsolete)
Comment 56 Build Bot 2019-06-11 19:15:04 PDT Comment hidden (obsolete)
Comment 57 Build Bot 2019-06-11 19:15:06 PDT Comment hidden (obsolete)
Comment 58 Build Bot 2019-06-11 19:16:31 PDT Comment hidden (obsolete)
Comment 59 Build Bot 2019-06-11 19:16:33 PDT Comment hidden (obsolete)
Comment 60 Build Bot 2019-06-11 19:31:34 PDT Comment hidden (obsolete)
Comment 61 Build Bot 2019-06-11 19:31:37 PDT Comment hidden (obsolete)
Comment 62 Devin Rousso 2019-06-11 20:35:17 PDT
Created attachment 371916 [details]
Patch

Check non-editing tests
Comment 63 Build Bot 2019-06-11 21:53:01 PDT Comment hidden (obsolete)
Comment 64 Build Bot 2019-06-11 21:53:03 PDT Comment hidden (obsolete)
Comment 65 Build Bot 2019-06-11 22:00:33 PDT Comment hidden (obsolete)
Comment 66 Build Bot 2019-06-11 22:00:35 PDT Comment hidden (obsolete)
Comment 67 Build Bot 2019-06-11 22:29:55 PDT Comment hidden (obsolete)
Comment 68 Build Bot 2019-06-11 22:29:57 PDT Comment hidden (obsolete)
Comment 69 Build Bot 2019-06-11 22:42:47 PDT Comment hidden (obsolete)
Comment 70 Build Bot 2019-06-11 22:42:50 PDT Comment hidden (obsolete)
Comment 71 Devin Rousso 2019-06-12 11:20:15 PDT
Created attachment 371975 [details]
CSSComputedStyleDeclaration.cpp.diff (macOS)
Comment 72 Devin Rousso 2019-06-12 11:41:46 PDT
Created attachment 371977 [details]
Patch

Turns out we need to include `-webkit-text-decorations-in-effect` as it's used all over editing code :(
Comment 73 Devin Rousso 2019-06-13 20:34:00 PDT
Created attachment 372101 [details]
Patch
Comment 74 Devin Rousso 2019-10-24 13:02:41 PDT
Created attachment 381834 [details]
Patch

Rebase
Comment 75 WebKit Commit Bot 2019-10-24 21:22:15 PDT
Comment on attachment 381834 [details]
Patch

Clearing flags on attachment: 381834

Committed r251581: <https://trac.webkit.org/changeset/251581>
Comment 76 WebKit Commit Bot 2019-10-24 21:22:18 PDT
All reviewed patches have been landed.  Closing bug.
Comment 77 Antti Koivisto 2019-10-25 01:37:56 PDT
Nice!