Bug 236371 - [content-visibility] Parse and add experimental flag for content-visibility
Summary: [content-visibility] Parse and add experimental flag for content-visibility
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: CSS (show other bugs)
Version: WebKit Local Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Rob Buis
URL:
Keywords: InRadar
Depends on:
Blocks: 236238
  Show dependency treegraph
 
Reported: 2022-02-09 08:59 PST by cathiechen
Modified: 2022-08-26 09:55 PDT (History)
18 users (show)

See Also:


Attachments
Patch (34.25 KB, patch)
2022-02-09 09:26 PST, cathiechen
no flags Details | Formatted Diff | Diff
Patch (39.91 KB, patch)
2022-02-09 23:49 PST, cathiechen
no flags Details | Formatted Diff | Diff
Patch (39.37 KB, patch)
2022-02-14 04:14 PST, cathiechen
no flags Details | Formatted Diff | Diff
Patch (39.38 KB, patch)
2022-02-14 04:24 PST, cathiechen
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (38.90 KB, patch)
2022-02-14 04:32 PST, cathiechen
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (39.59 KB, patch)
2022-02-14 04:40 PST, cathiechen
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (39.59 KB, patch)
2022-02-14 04:51 PST, cathiechen
no flags Details | Formatted Diff | Diff
Patch (42.03 KB, patch)
2022-02-14 08:25 PST, cathiechen
no flags Details | Formatted Diff | Diff
Patch (55.20 KB, patch)
2022-02-28 02:59 PST, Rob Buis
no flags Details | Formatted Diff | Diff
Patch (41.92 KB, patch)
2022-02-28 09:41 PST, Rob Buis
no flags Details | Formatted Diff | Diff
Patch (41.87 KB, patch)
2022-03-03 00:09 PST, Rob Buis
no flags Details | Formatted Diff | Diff
Patch (40.73 KB, patch)
2022-03-09 01:27 PST, Rob Buis
no flags Details | Formatted Diff | Diff
Patch (45.72 KB, patch)
2022-03-10 01:14 PST, Rob Buis
no flags Details | Formatted Diff | Diff
Patch (45.11 KB, patch)
2022-03-29 01:32 PDT, Rob Buis
no flags Details | Formatted Diff | Diff
Patch (38.54 KB, patch)
2022-03-30 02:43 PDT, Rob Buis
no flags Details | Formatted Diff | Diff
Patch (35.82 KB, patch)
2022-03-30 05:13 PDT, Rob Buis
no flags Details | Formatted Diff | Diff
Patch (35.86 KB, patch)
2022-04-13 04:16 PDT, Rob Buis
no flags Details | Formatted Diff | Diff
Patch (36.64 KB, patch)
2022-04-13 06:38 PDT, Rob Buis
no flags Details | Formatted Diff | Diff
Patch (36.75 KB, patch)
2022-04-20 06:31 PDT, Rob Buis
no flags Details | Formatted Diff | Diff
Patch (36.84 KB, patch)
2022-04-27 01:59 PDT, Rob Buis
no flags Details | Formatted Diff | Diff
Patch (36.88 KB, patch)
2022-04-28 01:26 PDT, Rob Buis
no flags Details | Formatted Diff | Diff
Patch (36.21 KB, patch)
2022-05-05 01:34 PDT, Rob Buis
no flags Details | Formatted Diff | Diff
Patch (36.12 KB, patch)
2022-05-10 01:52 PDT, Rob Buis
no flags Details | Formatted Diff | Diff
Patch (32.45 KB, patch)
2022-05-18 01:42 PDT, Rob Buis
no flags Details | Formatted Diff | Diff
Patch (32.45 KB, patch)
2022-05-19 01:11 PDT, Rob Buis
no flags Details | Formatted Diff | Diff
Patch (31.92 KB, patch)
2022-05-25 01:50 PDT, Rob Buis
no flags Details | Formatted Diff | Diff
Patch (31.92 KB, patch)
2022-05-26 04:04 PDT, Rob Buis
no flags Details | Formatted Diff | Diff
Patch (33.75 KB, patch)
2022-05-26 06:14 PDT, Rob Buis
no flags Details | Formatted Diff | Diff
Patch (33.75 KB, patch)
2022-06-08 01:07 PDT, Rob Buis
no flags Details | Formatted Diff | Diff
Patch (5.35 KB, patch)
2022-08-25 11:08 PDT, Rob Buis
no flags Details | Formatted Diff | Diff
Patch (7.61 KB, patch)
2022-08-26 04:11 PDT, Rob Buis
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description cathiechen 2022-02-09 08:59:47 PST
Parse and add experimental flag for content-visibility.
Comment 1 cathiechen 2022-02-09 09:26:35 PST
Created attachment 451391 [details]
Patch
Comment 2 cathiechen 2022-02-09 23:49:36 PST
Created attachment 451499 [details]
Patch
Comment 3 Rob Buis 2022-02-10 01:20:00 PST
Comment on attachment 451499 [details]
Patch

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

> Source/WebCore/style/StyleBuilderCustom.h:84
> +    DECLARE_PROPERTY_CUSTOM_HANDLERS(ContentVisibility);

Do we actually need the custom stuff? Initially we had a really custom implementation but now it is pretty standard.
Comment 4 Rob Buis 2022-02-10 01:21:57 PST
Comment on attachment 451499 [details]
Patch

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

> Source/WebCore/rendering/RenderObject.h:949
> +        ADD_BOOLEAN_BITFIELD(layoutContainmentApplies, LayoutContainmentApplies);

This bitfield does not seem to be needed for this parsing patch.
Comment 5 cathiechen 2022-02-13 07:04:37 PST
Comment on attachment 451499 [details]
Patch

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

>> Source/WebCore/rendering/RenderObject.h:949
>> +        ADD_BOOLEAN_BITFIELD(layoutContainmentApplies, LayoutContainmentApplies);
> 
> This bitfield does not seem to be needed for this parsing patch.

Done, removed.

>> Source/WebCore/style/StyleBuilderCustom.h:84
>> +    DECLARE_PROPERTY_CUSTOM_HANDLERS(ContentVisibility);
> 
> Do we actually need the custom stuff? Initially we had a really custom implementation but now it is pretty standard.

Hmmm, do you mean all changes in this file is not needed?
Comment 6 cathiechen 2022-02-13 07:08:42 PST
Comment on attachment 451499 [details]
Patch

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

> Source/WebCore/style/StyleBuilderCustom.h:1296
> +inline void BuilderCustom::applyInheritContentVisibility(BuilderState& builderState)

The value of content-visiblity shouldn't be inherited, so we should remove this.
Comment 7 Rob Buis 2022-02-13 08:23:46 PST
Comment on attachment 451499 [details]
Patch

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

>>> Source/WebCore/style/StyleBuilderCustom.h:84
>>> +    DECLARE_PROPERTY_CUSTOM_HANDLERS(ContentVisibility);
>> 
>> Do we actually need the custom stuff? Initially we had a really custom implementation but now it is pretty standard.
> 
> Hmmm, do you mean all changes in this file is not needed?

I am not sure it is possible, but it would be easier if it was generated and not custom code.
Comment 8 cathiechen 2022-02-13 23:48:04 PST
Comment on attachment 451499 [details]
Patch

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

>>>> Source/WebCore/style/StyleBuilderCustom.h:84
>>>> +    DECLARE_PROPERTY_CUSTOM_HANDLERS(ContentVisibility);
>>> 
>>> Do we actually need the custom stuff? Initially we had a really custom implementation but now it is pretty standard.
>> 
>> Hmmm, do you mean all changes in this file is not needed?
> 
> I am not sure it is possible, but it would be easier if it was generated and not custom code.

Right, we should remove custom setting in CSSProperties.json. Then this could be removed.
Comment 9 cathiechen 2022-02-14 04:14:04 PST
Created attachment 451885 [details]
Patch
Comment 10 cathiechen 2022-02-14 04:24:59 PST
Created attachment 451886 [details]
Patch
Comment 11 cathiechen 2022-02-14 04:32:26 PST
Created attachment 451887 [details]
Patch
Comment 12 cathiechen 2022-02-14 04:40:40 PST
Created attachment 451889 [details]
Patch
Comment 13 cathiechen 2022-02-14 04:51:32 PST
Created attachment 451892 [details]
Patch
Comment 14 cathiechen 2022-02-14 08:25:25 PST
Created attachment 451904 [details]
Patch
Comment 15 Radar WebKit Bug Importer 2022-02-16 09:00:17 PST
<rdar://problem/89028501>
Comment 16 Rob Buis 2022-02-28 02:59:07 PST
Created attachment 453379 [details]
Patch
Comment 17 Rob Buis 2022-02-28 09:41:24 PST
Created attachment 453399 [details]
Patch
Comment 18 Rob Buis 2022-03-03 00:09:49 PST
Created attachment 453704 [details]
Patch
Comment 19 Rob Buis 2022-03-09 01:27:57 PST
Created attachment 454208 [details]
Patch
Comment 20 Rob Buis 2022-03-10 01:14:50 PST
Created attachment 454323 [details]
Patch
Comment 21 Rob Buis 2022-03-29 01:32:47 PDT
Created attachment 456006 [details]
Patch
Comment 22 Rob Buis 2022-03-30 02:43:06 PDT
Created attachment 456106 [details]
Patch
Comment 23 Rob Buis 2022-03-30 05:13:03 PDT
Created attachment 456113 [details]
Patch
Comment 24 Rob Buis 2022-04-13 04:16:03 PDT
Created attachment 457521 [details]
Patch
Comment 25 Rob Buis 2022-04-13 06:38:14 PDT
Created attachment 457527 [details]
Patch
Comment 26 Rob Buis 2022-04-20 06:31:01 PDT
Created attachment 457985 [details]
Patch
Comment 27 Rob Buis 2022-04-27 01:59:51 PDT
Created attachment 458433 [details]
Patch
Comment 28 Rob Buis 2022-04-28 01:26:13 PDT
Created attachment 458499 [details]
Patch
Comment 29 Rob Buis 2022-05-05 01:34:57 PDT
Created attachment 458859 [details]
Patch
Comment 30 Rob Buis 2022-05-10 01:52:36 PDT
Created attachment 459103 [details]
Patch
Comment 31 Rob Buis 2022-05-18 01:42:28 PDT
Created attachment 459531 [details]
Patch
Comment 32 Rob Buis 2022-05-19 01:11:59 PDT
Created attachment 459570 [details]
Patch
Comment 33 Rob Buis 2022-05-25 01:50:17 PDT
Created attachment 459748 [details]
Patch
Comment 34 Rob Buis 2022-05-26 04:04:50 PDT
Created attachment 459781 [details]
Patch
Comment 35 Rob Buis 2022-05-26 06:14:39 PDT
Created attachment 459785 [details]
Patch
Comment 36 Rob Buis 2022-06-08 01:07:39 PDT
Created attachment 460085 [details]
Patch
Comment 37 Rob Buis 2022-06-16 02:02:14 PDT
Pull request: https://github.com/WebKit/WebKit/pull/1573
Comment 38 Rob Buis 2022-06-22 00:32:06 PDT
Pull request: https://github.com/WebKit/WebKit/pull/1676
Comment 39 Rob Buis 2022-06-29 07:05:17 PDT
Pull request: https://github.com/WebKit/WebKit/pull/1893
Comment 40 Rob Buis 2022-08-09 10:55:29 PDT
Pull request: https://github.com/WebKit/WebKit/pull/3147
Comment 41 Rob Buis 2022-08-23 03:16:39 PDT
Pull request: https://github.com/WebKit/WebKit/pull/3158
Comment 42 EWS 2022-08-25 03:25:55 PDT
Committed 253768@main (fccee3186739): <https://commits.webkit.org/253768@main>

Reviewed commits have been landed. Closing PR #3158 and removing active labels.
Comment 43 Rob Buis 2022-08-25 11:08:34 PDT
Reopening to attach new patch.
Comment 44 Rob Buis 2022-08-25 11:08:40 PDT
Created attachment 461863 [details]
Patch
Comment 45 Darin Adler 2022-08-25 13:30:58 PDT
Comment on attachment 461863 [details]
Patch

How do we check these things with regression tests? Since all tests run with the experimental features enabled, I am not sure we end up testing "content-visibility is fully cleanly disabled by default"
Comment 46 Darin Adler 2022-08-25 13:31:32 PDT
Also, why use the same bug for multiple patches? Not a good system I don’t think.
Comment 47 Rob Buis 2022-08-26 04:11:42 PDT
Created attachment 461878 [details]
Patch
Comment 48 EWS 2022-08-26 09:36:41 PDT
Committed 253821@main (827856bf32d2): <https://commits.webkit.org/253821@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 461878 [details].
Comment 49 Rob Buis 2022-08-26 09:55:56 PDT
(In reply to Darin Adler from comment #45)
> Comment on attachment 461863 [details]
> Patch
> 
> How do we check these things with regression tests? Since all tests run with
> the experimental features enabled, I am not sure we end up testing
> "content-visibility is fully cleanly disabled by default"

Not sure if you were addressing me or Sam, but I included a test for this in the patch that landed (content-visibility-invalidate-if-disabled.html).