236371 – [content-visibility] Parse and add experimental flag for content-visibility

RESOLVED FIXED Bug 236371

[content-visibility] Parse and add experimental flag for content-visibility

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

Summary [content-visibility] Parse and add experimental flag for content-visibility

cathiechen

Reported 2022-02-09 08:59:47 PST

Parse and add experimental flag for content-visibility.

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
Show Obsolete (30) View All Add attachment proposed patch, testcase, etc.

cathiechen

Comment 1 2022-02-09 09:26:35 PST

Created attachment 451391 [details] Patch

cathiechen

Comment 2 2022-02-09 23:49:36 PST

Created attachment 451499 [details] Patch

Rob Buis

Comment 3 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.

Rob Buis

Comment 4 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.

cathiechen

Comment 5 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?

cathiechen

Comment 6 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.

Rob Buis

Comment 7 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.

cathiechen

Comment 8 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.

cathiechen

Comment 9 2022-02-14 04:14:04 PST

Created attachment 451885 [details] Patch

cathiechen

Comment 10 2022-02-14 04:24:59 PST

Created attachment 451886 [details] Patch

cathiechen

Comment 11 2022-02-14 04:32:26 PST

Created attachment 451887 [details] Patch

cathiechen

Comment 12 2022-02-14 04:40:40 PST

Created attachment 451889 [details] Patch

cathiechen

Comment 13 2022-02-14 04:51:32 PST

Created attachment 451892 [details] Patch

cathiechen

Comment 14 2022-02-14 08:25:25 PST

Created attachment 451904 [details] Patch

Radar WebKit Bug Importer

Comment 15 2022-02-16 09:00:17 PST

<rdar://problem/89028501>

Rob Buis

Comment 16 2022-02-28 02:59:07 PST

Created attachment 453379 [details] Patch

Rob Buis

Comment 17 2022-02-28 09:41:24 PST

Created attachment 453399 [details] Patch

Rob Buis

Comment 18 2022-03-03 00:09:49 PST

Created attachment 453704 [details] Patch

Rob Buis

Comment 19 2022-03-09 01:27:57 PST

Created attachment 454208 [details] Patch

Rob Buis

Comment 20 2022-03-10 01:14:50 PST

Created attachment 454323 [details] Patch

Rob Buis

Comment 21 2022-03-29 01:32:47 PDT

Created attachment 456006 [details] Patch

Rob Buis

Comment 22 2022-03-30 02:43:06 PDT

Created attachment 456106 [details] Patch

Rob Buis

Comment 23 2022-03-30 05:13:03 PDT

Created attachment 456113 [details] Patch

Rob Buis

Comment 24 2022-04-13 04:16:03 PDT

Created attachment 457521 [details] Patch

Rob Buis

Comment 25 2022-04-13 06:38:14 PDT

Created attachment 457527 [details] Patch

Rob Buis

Comment 26 2022-04-20 06:31:01 PDT

Created attachment 457985 [details] Patch

Rob Buis

Comment 27 2022-04-27 01:59:51 PDT

Created attachment 458433 [details] Patch

Rob Buis

Comment 28 2022-04-28 01:26:13 PDT

Created attachment 458499 [details] Patch

Rob Buis

Comment 29 2022-05-05 01:34:57 PDT

Created attachment 458859 [details] Patch

Rob Buis

Comment 30 2022-05-10 01:52:36 PDT

Created attachment 459103 [details] Patch

Rob Buis

Comment 31 2022-05-18 01:42:28 PDT

Created attachment 459531 [details] Patch

Rob Buis

Comment 32 2022-05-19 01:11:59 PDT

Created attachment 459570 [details] Patch

Rob Buis

Comment 33 2022-05-25 01:50:17 PDT

Created attachment 459748 [details] Patch

Rob Buis

Comment 34 2022-05-26 04:04:50 PDT

Created attachment 459781 [details] Patch

Rob Buis

Comment 35 2022-05-26 06:14:39 PDT

Created attachment 459785 [details] Patch

Rob Buis

Comment 36 2022-06-08 01:07:39 PDT

Created attachment 460085 [details] Patch

Rob Buis

Comment 37 2022-06-16 02:02:14 PDT

Pull request: https://github.com/WebKit/WebKit/pull/1573

Rob Buis

Comment 38 2022-06-22 00:32:06 PDT

Pull request: https://github.com/WebKit/WebKit/pull/1676

Rob Buis

Comment 39 2022-06-29 07:05:17 PDT

Pull request: https://github.com/WebKit/WebKit/pull/1893

Rob Buis

Comment 40 2022-08-09 10:55:29 PDT

Pull request: https://github.com/WebKit/WebKit/pull/3147

Rob Buis

Comment 41 2022-08-23 03:16:39 PDT

Pull request: https://github.com/WebKit/WebKit/pull/3158

EWS

Comment 42 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.

Rob Buis

Comment 43 2022-08-25 11:08:34 PDT

Reopening to attach new patch.

Rob Buis

Comment 44 2022-08-25 11:08:40 PDT

Created attachment 461863 [details] Patch

Darin Adler

Comment 45 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"

Darin Adler

Comment 46 2022-08-25 13:31:32 PDT

Also, why use the same bug for multiple patches? Not a good system I don’t think.

Rob Buis

Comment 47 2022-08-26 04:11:42 PDT

Created attachment 461878 [details] Patch

EWS

Comment 48 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].

Rob Buis

Comment 49 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).

Note You need to log in before you can comment on or make changes to this bug.

Status RESOLVED

Resolution FIXED

Priority P2

Severity Normal

Classification Unclassified

Version WebKit Local Build

Hardware Unspecified

OS Unspecified

Product WebKit

Component CSS

Assignee

Rob Buis

Reported

2022-02-09 08:59 PST

Modified

2022-08-26 09:55 PDT History

CC List

18 users Show

URL

Keywords InRadar

Depends on

Blocks

236238

Dependencies

tree graph