Skip to content

Conversation

@annevk
Copy link
Contributor

@annevk annevk commented Dec 19, 2023

a23b77e

querySelector() throws exception for -webkit-prefixed pseudo elements
https://bugs.webkit.org/show_bug.cgi?id=149160
rdar://99299129

Reviewed by Antti Koivisto.

SelectorHasInvalidSelectorFunctor which is only used by querySelector()
and friends treated custom pseudo-elements as invalid. This is wrong.
querySelector("::placeholder") and querySelector("::-webkit-asdf") are
perfectly acceptable inputs, they are just supposed to return null.

Also do some minor code cleanup while in the general area.

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

de86f5a

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 annevk self-assigned this Dec 19, 2023
@annevk annevk added the DOM For bugs specific to XML/HTML DOM elements (including parsing). label Dec 19, 2023
@mdubet
Copy link
Contributor

mdubet commented Dec 19, 2023

We should add a test (WPT ideally) for this

@Ahmad-S792
Copy link
Contributor

We should add a test (WPT ideally) for this

Is this third test case about this? https://wpt.fyi/results/css/selectors/webkit-pseudo-element.html?label=master&label=experimental&aligned&q=webkit

@annevk
Copy link
Contributor Author

annevk commented Dec 19, 2023

It's a draft for a reason. 😉

@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Dec 19, 2023
@annevk annevk removed the merging-blocked Applied to prevent a change from being merged label Dec 20, 2023
@annevk annevk force-pushed the eng/querySelector-throws-exception-for--webkit-prefixed-pseudo-elements branch from 27f9178 to de86f5a Compare December 20, 2023 06:22
@annevk annevk marked this pull request as ready for review December 20, 2023 06:23
@annevk annevk requested review from cdumez and rniwa as code owners December 20, 2023 06:23
@annevk annevk requested a review from anttijk December 20, 2023 06:23
@annevk
Copy link
Contributor Author

annevk commented Dec 20, 2023

@anttijk should probably be a follow-up, but I wonder if this allows for even greater simplification as the notion of validity should now match that of CSS so presumably we can now reuse some code path that doesn't need for its own hasInvalidSelector().

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That would be a good opportunity to convert the whole functor class to a simple lambda.

@annevk annevk added the merge-queue Applied to send a pull request to merge-queue label Dec 20, 2023
https://bugs.webkit.org/show_bug.cgi?id=149160
rdar://99299129

Reviewed by Antti Koivisto.

SelectorHasInvalidSelectorFunctor which is only used by querySelector()
and friends treated custom pseudo-elements as invalid. This is wrong.
querySelector("::placeholder") and querySelector("::-webkit-asdf") are
perfectly acceptable inputs, they are just supposed to return null.

Also do some minor code cleanup while in the general area.

Canonical link: https://commits.webkit.org/272337@main
@webkit-commit-queue webkit-commit-queue force-pushed the eng/querySelector-throws-exception-for--webkit-prefixed-pseudo-elements branch from de86f5a to a23b77e Compare December 20, 2023 12:12
@webkit-commit-queue
Copy link
Collaborator

Committed 272337@main (a23b77e): https://commits.webkit.org/272337@main

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

@webkit-commit-queue webkit-commit-queue merged commit a23b77e into WebKit:main Dec 20, 2023
@webkit-commit-queue webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label Dec 20, 2023
@annevk annevk deleted the eng/querySelector-throws-exception-for--webkit-prefixed-pseudo-elements branch December 20, 2023 12:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

DOM For bugs specific to XML/HTML DOM elements (including parsing).

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants