Separate unknown -webkit-prefixed pseudo-elements from known #22274

annevk · 2023-12-31T11:19:35Z

`405fb5b`

Separate unknown -webkit-prefixed pseudo-elements from known
https://bugs.webkit.org/show_bug.cgi?id=266947

Reviewed by Tim Nguyen.

All UserAgentPartIds are moved into CSSPseudoSelectors.json and then
generated from there from now on.

This in combination with adding the WebKitUnknown pseudo-element
identifier corrects @supports so it no longer claims certain
-webkit-prefixed pseudo-elements are unsupported when they are in fact
supported.

Existing PseudoElement::UserAgentPart conditionals are amended to
account for PseudoElement::WebKitUnknown.

FIXMEs are added for areas that need further work.

* LayoutTests/TestExpectations:
* Source/WebCore/CMakeLists.txt:
* Source/WebCore/DerivedSources-output.xcfilelist:
* Source/WebCore/DerivedSources.make:
* Source/WebCore/Headers.cmake:
* Source/WebCore/Sources.txt:
* Source/WebCore/WebCore.xcodeproj/project.pbxproj:
* Source/WebCore/css/CSSPseudoSelectors.json:
* Source/WebCore/css/CSSSelector.cpp:
(WebCore::CSSSelector::pseudoId):
(WebCore::CSSSelector::parsePseudoElementName):
* Source/WebCore/css/SelectorChecker.cpp:
(WebCore::SelectorChecker::matchRecursively const):
* Source/WebCore/css/parser/CSSSelectorParser.cpp:
(WebCore::isPseudoClassValidAfterPseudoElement):
(WebCore::CSSSelectorParser::containsUnknownWebKitPseudoElements):
* Source/WebCore/css/parser/MutableCSSSelector.h:
(WebCore::MutableCSSSelector::needsImplicitShadowCombinatorForMatching const):
* Source/WebCore/css/process-css-pseudo-selectors.py:
(InputValidator.validate_fields):
* Source/WebCore/cssjit/SelectorCompiler.cpp:
(WebCore::SelectorCompiler::constructFragmentsInternal):

No need for an else branch here due to the early return usage.

* Source/WebCore/html/shadow/UserAgentPartIds.cpp: Removed.
* Source/WebCore/html/shadow/UserAgentPartIds.h: Removed.

Canonical link: https://commits.webkit.org/272726@main

498d79c

Misc	iOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	✅ 🛠 wincairo
✅ 🧪 bindings	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	✅ 🧪 wpe-wk2
✅ 🧪 webkitperl	✅ 🧪 ios-wk2	✅ 🧪 api-mac	✅ 🧪 api-wpe
	✅ 🧪 ios-wk2-wpt	✅ 🧪 mac-wk1	✅ 🛠 gtk
	✅ 🧪 api-ios	✅ 🧪 mac-wk2	✅ 🧪 gtk-wk2
	✅ 🛠 tv	✅ 🧪 mac-AS-debug-wk2	✅ 🧪 api-gtk
	✅ 🛠 tv-sim
	✅ 🛠 watch
✅ 🛠 🧪 unsafe-merge	✅ 🛠 watch-sim

annevk · 2023-12-31T12:26:01Z

I'm somewhat surprised there are no test failures here. I guess that means we have to add some WPTs for @supports().

nt1m · 2024-01-01T05:24:36Z

With #22279 landing, I think it would be good to try to generate ShadowPseudoIds from CSSPseudoSelectors.json before doing this to avoid ending in a state where the JSON file duplicates ShadowPseudoIds

Source/WebCore/css/CSSSelector.h

Source/WebCore/css/CSSPseudoSelectors.json

Source/WebCore/css/CSSSelector.cpp

Source/WebCore/css/SelectorChecker.cpp

Source/WebCore/css/parser/CSSSelectorParser.cpp

Source/WebCore/css/parser/MutableCSSSelector.h

Source/WebCore/css/process-css-pseudo-selectors.py

Source/WebCore/style/RuleSet.cpp

nt1m · 2024-01-03T22:32:21Z

Looks great overall! I mostly have nits

nt1m

You'll need to register your new C++ file like https://searchfox.org/wubkat/rev/e0c29e7e482a4a452c92c394677358659c71eace/Source/WebCore/CMakeLists.txt#2555 to fix the GTK build fwiw

Source/WebCore/css/process-css-pseudo-selectors.py

Source/WebCore/css/CSSPseudoSelectors.json

webkit-early-warning-system · 2024-01-05T17:37:26Z

EWS run on previous version of this PR (hash 8af8147)

Misc	iOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	✅ 🛠 wincairo
✅ 🧪 bindings	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	✅ 🧪 wpe-wk2
✅ 🧪 webkitperl	✅ 🧪 ios-wk2	✅ 🧪 api-mac	✅ 🧪 api-wpe
	✅ 🧪 ios-wk2-wpt	✅ 🧪 mac-wk1	✅ 🛠 gtk
	✅ 🧪 api-ios	✅ 🧪 mac-wk2	✅ 🧪 gtk-wk2
	✅ 🛠 tv	✅ 🧪 mac-AS-debug-wk2	✅ 🧪 api-gtk
	✅ 🛠 tv-sim
~~🛠 🧪 merge~~	✅ 🛠 watch
	✅ 🛠 watch-sim

annevk · 2024-01-05T20:59:41Z

Something is wrong. It looks like the test case is no longer passing. I thought it was just a Glib thing, but it seems to impact all builds, it's just not causing a failure for some reason. I suspect that one of the WebKitUnknown removals is the cause, but I haven't investigated.

Source/WebCore/css/process-css-pseudo-selectors.py

webkit-early-warning-system · 2024-01-06T07:11:44Z

EWS run on current version of this PR (hash 498d79c)

Misc	iOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	✅ 🛠 wincairo
✅ 🧪 bindings	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	✅ 🧪 wpe-wk2
✅ 🧪 webkitperl	✅ 🧪 ios-wk2	✅ 🧪 api-mac	✅ 🧪 api-wpe
	✅ 🧪 ios-wk2-wpt	✅ 🧪 mac-wk1	✅ 🛠 gtk
	✅ 🧪 api-ios	✅ 🧪 mac-wk2	✅ 🧪 gtk-wk2
	✅ 🛠 tv	✅ 🧪 mac-AS-debug-wk2	✅ 🧪 api-gtk
	✅ 🛠 tv-sim
	✅ 🛠 watch
✅ 🛠 🧪 unsafe-merge	✅ 🛠 watch-sim

nt1m · 2024-01-06T07:20:24Z

Source/WebCore/css/parser/MutableCSSSelector.h

+            || pseudoElement() == CSSSelector::PseudoElement::UserAgentPartLegacyAlias
+            || pseudoElement() == CSSSelector::PseudoElement::WebKitUnknown);


Can we move down pseudoElement() == CSSSelector::PseudoElement::UserAgentPart to have some alphabetic ordering?

Does that all compile down to the same thing? I thought these might be ordered this way for perf reasons.

We should probably add a comment if that's the case. The temptation of re-ordering is easy.

But I doubt it makes a noticeable difference tbh

I'm going to leave this as-is for now. There's already enough subtle changes in this patch. And there's a FIXME for this function anyway so once that's addressed this can be addressed at the same time. (Or this change simply gets undone.)

Sounds good!

https://bugs.webkit.org/show_bug.cgi?id=266947 Reviewed by Tim Nguyen. All UserAgentPartIds are moved into CSSPseudoSelectors.json and then generated from there from now on. This in combination with adding the WebKitUnknown pseudo-element identifier corrects @supports so it no longer claims certain -webkit-prefixed pseudo-elements are unsupported when they are in fact supported. Existing PseudoElement::UserAgentPart conditionals are amended to account for PseudoElement::WebKitUnknown. FIXMEs are added for areas that need further work. * LayoutTests/TestExpectations: * Source/WebCore/CMakeLists.txt: * Source/WebCore/DerivedSources-output.xcfilelist: * Source/WebCore/DerivedSources.make: * Source/WebCore/Headers.cmake: * Source/WebCore/Sources.txt: * Source/WebCore/WebCore.xcodeproj/project.pbxproj: * Source/WebCore/css/CSSPseudoSelectors.json: * Source/WebCore/css/CSSSelector.cpp: (WebCore::CSSSelector::pseudoId): (WebCore::CSSSelector::parsePseudoElementName): * Source/WebCore/css/SelectorChecker.cpp: (WebCore::SelectorChecker::matchRecursively const): * Source/WebCore/css/parser/CSSSelectorParser.cpp: (WebCore::isPseudoClassValidAfterPseudoElement): (WebCore::CSSSelectorParser::containsUnknownWebKitPseudoElements): * Source/WebCore/css/parser/MutableCSSSelector.h: (WebCore::MutableCSSSelector::needsImplicitShadowCombinatorForMatching const): * Source/WebCore/css/process-css-pseudo-selectors.py: (InputValidator.validate_fields): * Source/WebCore/cssjit/SelectorCompiler.cpp: (WebCore::SelectorCompiler::constructFragmentsInternal): No need for an else branch here due to the early return usage. * Source/WebCore/html/shadow/UserAgentPartIds.cpp: Removed. * Source/WebCore/html/shadow/UserAgentPartIds.h: Removed. Canonical link: https://commits.webkit.org/272726@main

webkit-commit-queue · 2024-01-06T08:56:30Z

Committed 272726@main (405fb5b): https://commits.webkit.org/272726@main

Reviewed commits have been landed. Closing PR #22274 and removing active labels.

philn · 2024-01-06T11:05:04Z

This broke CMake builds...

CMake Error at Source/cmake/WebKitMacros.cmake:118 (target_sources):
Cannot find source file:
/app/webkit/WebKitBuild/WPE/Release/WebCore/DerivedSources/UserAgentPartIds.cpp
Tried extensions .c .C .c++ .cc .cpp .cxx .cu .mpp .m .M .mm .ixx .cppm
.ccm .cxxm .c++m .h .hh .h++ .hm .hpp .hxx .in .txx .f .F .for .f77 .f90
.f95 .f03 .hip .ispc
Call Stack (most recent call first):
Source/cmake/WebKitMacros.cmake:322 (_WEBKIT_TARGET)
Source/WebCore/CMakeLists.txt:2870 (WEBKIT_FRAMEWORK)
CMake Error in Source/WebCore/CMakeLists.txt:
Cannot find source file:
/app/webkit/WebKitBuild/WPE/Release/WebCore/DerivedSources/UserAgentPartIds.h
Tried extensions .c .C .c++ .cc .cpp .cxx .cu .mpp .m .M .mm .ixx .cppm
.ccm .cxxm .c++m .h .hh .h++ .hm .hpp .hxx .in .txx .f .F .for .f77 .f90
.f95 .f03 .hip .ispc

philn · 2024-01-06T11:14:43Z

#22462

mcatanzaro · 2024-01-06T13:55:02Z

I was hoping this wouldn't happen in practice, but that was probably foolish. We are going to have to fix https://bugs.webkit.org/show_bug.cgi?id=266984 first, then.

mcatanzaro · 2024-01-06T13:59:18Z

Wait, sorry, I see you were able to fix it by just adding the missing OUTPUTs. Hmm... I was expecting that to have no effect. Interesting.

annevk · 2024-01-06T15:13:05Z

@mcatanzaro note that I had listed the dependencies already. The OUTPUTs were removed as I rebased incorrectly.

@philn thanks for fixing the build and sorry you had to do that!

annevk self-assigned this Dec 31, 2023

annevk added the CSS Cascading Style Sheets implementation label Dec 31, 2023

This comment was marked as outdated.

Sign in to view

webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Dec 31, 2023

annevk removed the merging-blocked Applied to prevent a change from being merged label Dec 31, 2023

annevk force-pushed the eng/Separate-unknown--webkit-prefixed-pseudo-elements-from-known branch from 254fd86 to 2e112e7 Compare December 31, 2023 15:38

This comment was marked as duplicate.

Sign in to view

This comment was marked as outdated.

Sign in to view

annevk mentioned this pull request Dec 31, 2023

Make pseudo attribute values of DateTimeFieldElements proper ShadowPseudoIds #22276

Merged

nt1m reviewed Jan 3, 2024

View reviewed changes

Source/WebCore/css/CSSSelector.h Outdated Show resolved Hide resolved

annevk force-pushed the eng/Separate-unknown--webkit-prefixed-pseudo-elements-from-known branch from 2e112e7 to 86f9f8e Compare January 3, 2024 22:20

This comment was marked as outdated.

Sign in to view

annevk force-pushed the eng/Separate-unknown--webkit-prefixed-pseudo-elements-from-known branch from 86f9f8e to 85e1b80 Compare January 3, 2024 22:22

This comment was marked as outdated.

Sign in to view

nt1m reviewed Jan 3, 2024

View reviewed changes

webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Jan 3, 2024

nt1m reviewed Jan 4, 2024

View reviewed changes

Source/WebCore/css/process-css-pseudo-selectors.py Outdated Show resolved Hide resolved

annevk removed the merging-blocked Applied to prevent a change from being merged label Jan 4, 2024

annevk force-pushed the eng/Separate-unknown--webkit-prefixed-pseudo-elements-from-known branch from 85e1b80 to b167f6e Compare January 4, 2024 10:24

This comment was marked as outdated.

Sign in to view

annevk marked this pull request as ready for review January 4, 2024 10:46

annevk requested review from cdumez and rniwa as code owners January 4, 2024 10:46

annevk force-pushed the eng/Separate-unknown--webkit-prefixed-pseudo-elements-from-known branch from b167f6e to cc80aea Compare January 4, 2024 10:46

This comment was marked as outdated.

Sign in to view

annevk requested a review from nt1m January 4, 2024 10:48

nt1m reviewed Jan 5, 2024

View reviewed changes

Source/WebCore/css/CSSPseudoSelectors.json Outdated Show resolved Hide resolved

annevk removed the merging-blocked Applied to prevent a change from being merged label Jan 5, 2024

annevk force-pushed the eng/Separate-unknown--webkit-prefixed-pseudo-elements-from-known branch from 78b321c to 8af8147 Compare January 5, 2024 17:37

This comment was marked as duplicate.

Sign in to view

annevk added the safe-merge-queue Applied to automatically send a pull-request to merge-queue after passing EWS checks label Jan 5, 2024

webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Jan 5, 2024

nt1m removed the merging-blocked Applied to prevent a change from being merged label Jan 5, 2024

nt1m reviewed Jan 6, 2024

View reviewed changes

Source/WebCore/css/process-css-pseudo-selectors.py Outdated Show resolved Hide resolved

Source/WebCore/css/process-css-pseudo-selectors.py Outdated Show resolved Hide resolved

annevk removed the merging-blocked Applied to prevent a change from being merged label Jan 6, 2024

annevk force-pushed the eng/Separate-unknown--webkit-prefixed-pseudo-elements-from-known branch from 8af8147 to 498d79c Compare January 6, 2024 07:11

nt1m reviewed Jan 6, 2024

View reviewed changes

annevk added the unsafe-merge-queue Applied to send a pull request to merge-queue, but skip building and testing label Jan 6, 2024

webkit-commit-queue force-pushed the eng/Separate-unknown--webkit-prefixed-pseudo-elements-from-known branch from 498d79c to 405fb5b Compare January 6, 2024 08:56

webkit-commit-queue merged commit 405fb5b into WebKit:main Jan 6, 2024

webkit-commit-queue removed the unsafe-merge-queue Applied to send a pull request to merge-queue, but skip building and testing label Jan 6, 2024

annevk deleted the eng/Separate-unknown--webkit-prefixed-pseudo-elements-from-known branch January 6, 2024 09:05

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Separate unknown -webkit-prefixed pseudo-elements from known #22274

Separate unknown -webkit-prefixed pseudo-elements from known #22274

annevk commented Dec 31, 2023 •

edited by webkit-early-warning-system

Loading

This comment was marked as outdated.

annevk commented Dec 31, 2023

This comment was marked as duplicate.

This comment was marked as outdated.

nt1m commented Jan 1, 2024

This comment was marked as outdated.

This comment was marked as outdated.

nt1m commented Jan 3, 2024

nt1m left a comment

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as duplicate.

webkit-early-warning-system commented Jan 5, 2024 •

edited

Loading

annevk commented Jan 5, 2024

webkit-early-warning-system commented Jan 6, 2024 •

edited

Loading

nt1m Jan 6, 2024

annevk Jan 6, 2024

nt1m Jan 6, 2024

annevk Jan 6, 2024

nt1m Jan 6, 2024

webkit-commit-queue commented Jan 6, 2024

philn commented Jan 6, 2024

philn commented Jan 6, 2024

mcatanzaro commented Jan 6, 2024

mcatanzaro commented Jan 6, 2024

annevk commented Jan 6, 2024

		\|\| pseudoElement() == CSSSelector::PseudoElement::UserAgentPartLegacyAlias
		\|\| pseudoElement() == CSSSelector::PseudoElement::WebKitUnknown);

Separate unknown -webkit-prefixed pseudo-elements from known #22274

Separate unknown -webkit-prefixed pseudo-elements from known #22274

Conversation

annevk commented Dec 31, 2023 • edited by webkit-early-warning-system Loading

405fb5b

This comment was marked as outdated.

annevk commented Dec 31, 2023

This comment was marked as duplicate.

This comment was marked as outdated.

nt1m commented Jan 1, 2024

This comment was marked as outdated.

This comment was marked as outdated.

nt1m commented Jan 3, 2024

nt1m left a comment

Choose a reason for hiding this comment

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as duplicate.

webkit-early-warning-system commented Jan 5, 2024 • edited Loading

annevk commented Jan 5, 2024

webkit-early-warning-system commented Jan 6, 2024 • edited Loading

nt1m Jan 6, 2024

Choose a reason for hiding this comment

annevk Jan 6, 2024

Choose a reason for hiding this comment

nt1m Jan 6, 2024

Choose a reason for hiding this comment

annevk Jan 6, 2024

Choose a reason for hiding this comment

nt1m Jan 6, 2024

Choose a reason for hiding this comment

webkit-commit-queue commented Jan 6, 2024

philn commented Jan 6, 2024

philn commented Jan 6, 2024

mcatanzaro commented Jan 6, 2024

mcatanzaro commented Jan 6, 2024

annevk commented Jan 6, 2024

annevk commented Dec 31, 2023 •

edited by webkit-early-warning-system

Loading

`405fb5b`

webkit-early-warning-system commented Jan 5, 2024 •

edited

Loading

webkit-early-warning-system commented Jan 6, 2024 •

edited

Loading