Skip to content

attachShadow throws type error with document-fragment ShadowRoot #47395

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 1 commit into from
Jul 1, 2025
Merged

attachShadow throws type error with document-fragment ShadowRoot #47395

merged 1 commit into from
Jul 1, 2025
+78 −4

Conversation

rniwa
Copy link
Member

@rniwa rniwa commented Jun 30, 2025 •
edited by webkit-early-warning-system
Loading

ec8e119

attachShadow throws type error with document-fragment ShadowRoot
https://bugs.webkit.org/show_bug.cgi?id=295174
rdar://154658449

Reviewed by Chris Dumez and Anne van Kesteren.

Allow customElementRegistry in ShadowRootInit to be null, in which case, we use the global registry.

* LayoutTests/imported/w3c/web-platform-tests/custom-elements/registries/ShadowRoot-init-customElementRegistry-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/custom-elements/registries/ShadowRoot-init-customElementRegistry.html:
* LayoutTests/imported/w3c/web-platform-tests/shadow-dom/attachShadow-with-ShadowRoot-expected.txt: Added.
* LayoutTests/imported/w3c/web-platform-tests/shadow-dom/attachShadow-with-ShadowRoot.html: Added.
* Source/WebCore/dom/ShadowRootInit.idl:

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

b8af91e

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 win
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🧪 wpe-wk2 ⏳ 🧪 win-tests
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🧪 api-wpe
✅ 🧪 ios-wk2-wpt ✅ 🧪 mac-wk1 ✅ 🛠 wpe-cairo
✅ 🧪 api-ios ✅ 🧪 mac-wk2 ✅ 🛠 gtk
✅ 🛠 vision ✅ 🧪 mac-AS-debug-wk2 ✅ 🧪 gtk-wk2
✅ 🛠 vision-sim ✅ 🧪 mac-wk2-stress ✅ 🧪 api-gtk
✅ 🧪 vision-wk2 ✅ 🧪 mac-intel-wk2 ✅ 🛠 playstation
✅ 🛠 🧪 unsafe-merge ✅ 🛠 tv ✅ 🛠 mac-safer-cpp
✅ 🛠 tv-sim
✅ 🛠 watch
✅ 🛠 watch-sim

@rniwa rniwa requested a review from cdumez as a code owner June 30, 2025 20:28
@rniwa rniwa self-assigned this Jun 30, 2025
@rniwa rniwa added the DOM For bugs specific to XML/HTML DOM elements (including parsing). label Jun 30, 2025
@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented Jun 30, 2025 •
edited
Loading

EWS run on current version of this PR (hash b8af91e)

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 win
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🧪 wpe-wk2 ⏳ 🧪 win-tests
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🧪 api-wpe
✅ 🧪 ios-wk2-wpt ✅ 🧪 mac-wk1 ✅ 🛠 wpe-cairo
✅ 🧪 api-ios ✅ 🧪 mac-wk2 ✅ 🛠 gtk
✅ 🛠 vision ✅ 🧪 mac-AS-debug-wk2 ✅ 🧪 gtk-wk2
✅ 🛠 vision-sim ✅ 🧪 mac-wk2-stress ✅ 🧪 api-gtk
✅ 🧪 vision-wk2 ✅ 🧪 mac-intel-wk2 ✅ 🛠 playstation
✅ 🛠 🧪 unsafe-merge ✅ 🛠 tv ✅ 🛠 mac-safer-cpp
✅ 🛠 tv-sim
✅ 🛠 watch
✅ 🛠 watch-sim

@rniwa rniwa requested a review from annevk June 30, 2025 20:28
annevk
annevk approved these changes Jul 1, 2025
View reviewed changes
Copy link
Contributor

@annevk annevk left a comment

Choose a reason for hiding this comment

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

Maybe clarify that the fallback is the same as with undefined, namely the shadow host's registry, not the global registry (although often they'll be the same).

@annevk
Copy link
Contributor

annevk commented Jul 1, 2025

Oh, and please don't forget to upstream these tests.

@rniwa
Copy link
Member Author

rniwa commented Jul 1, 2025

Maybe clarify that the fallback is the same as with undefined, namely the shadow host's registry, not the global registry (although often they'll be the same).

That's not happens with this code right now. I guess we should fix it in a separate PR.

@rniwa rniwa added the unsafe-merge-queue Applied to send a pull request to merge-queue, but skip building and testing label Jul 1, 2025
@webkit-commit-queue webkit-commit-queue force-pushed the fix295174-attach-shadow-null-registry branch from b8af91e to 4931ef5 Compare July 1, 2025 06:08
@rniwa
attachShadow throws type error with document-fragment ShadowRoot
ec8e119 
https://bugs.webkit.org/show_bug.cgi?id=295174
rdar://154658449

Reviewed by Chris Dumez and Anne van Kesteren.

Allow customElementRegistry in ShadowRootInit to be null, in which case, we use the global registry.

* LayoutTests/imported/w3c/web-platform-tests/custom-elements/registries/ShadowRoot-init-customElementRegistry-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/custom-elements/registries/ShadowRoot-init-customElementRegistry.html:
* LayoutTests/imported/w3c/web-platform-tests/shadow-dom/attachShadow-with-ShadowRoot-expected.txt: Added.
* LayoutTests/imported/w3c/web-platform-tests/shadow-dom/attachShadow-with-ShadowRoot.html: Added.
* Source/WebCore/dom/ShadowRootInit.idl:

Canonical link: https://commits.webkit.org/296853@main
@webkit-commit-queue webkit-commit-queue force-pushed the fix295174-attach-shadow-null-registry branch from 4931ef5 to ec8e119 Compare July 1, 2025 06:10
@webkit-commit-queue
Copy link
Collaborator

Committed 296853@main (ec8e119): https://commits.webkit.org/296853@main

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

@webkit-commit-queue webkit-commit-queue merged commit ec8e119 into WebKit:main Jul 1, 2025
@webkit-commit-queue webkit-commit-queue removed the unsafe-merge-queue Applied to send a pull request to merge-queue, but skip building and testing label Jul 1, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cdumez cdumez cdumez approved these changes

@annevk annevk annevk approved these changes

Assignees

@rniwa rniwa

Labels
DOM For bugs specific to XML/HTML DOM elements (including parsing).
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@rniwa @webkit-early-warning-system @annevk @webkit-commit-queue @cdumez