Skip to content
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

Add mutation events tests for ContainerNode::replaceChildren and refactor the code #17328

Merged
merged 1 commit into from
Sep 1, 2023
Merged

Add mutation events tests for ContainerNode::replaceChildren and refactor the code #17328

merged 1 commit into from
Sep 1, 2023

Conversation

rniwa
Copy link
Member

@rniwa rniwa commented Sep 1, 2023 •
edited by webkit-early-warning-system
Loading

c177199

Add mutation events tests for ContainerNode::replaceChildren and refactor the code
https://bugs.webkit.org/show_bug.cgi?id=261016

Reviewed by Chris Dumez.

Added some tests for mutating DOM during replaceChildren's tree mutating operations.

Also refactor the code in ContainerNode::replaceChildren by merging some aspects of
Node::convertNodesOrStringsIntoNode into the function so that all node removal &
insertion the logic will be in one place.

* LayoutTests/fast/dom/replace-children-mutation-events-expected.txt: Added.
* LayoutTests/fast/dom/replace-children-mutation-events.html: Added.
* Source/WebCore/dom/ContainerNode.cpp:
(WebCore::ContainerNode::replaceChildren):
* Source/WebCore/dom/Node.cpp:
(WebCore::Node::convertNodesOrStringsIntoNodeVector): Extracted from convertNodesOrStringsIntoNode.
(WebCore::Node::convertNodesOrStringsIntoNode):
* Source/WebCore/dom/Node.h:

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

5d3520b

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 ✅ 🛠 gtk
✅ 🧪 ios-wk2-wpt ✅ 🧪 mac-wk1 ✅ 🧪 gtk-wk2
✅ 🧪 api-ios ✅ 🧪 mac-wk2 ✅ 🧪 api-gtk
✅ 🛠 tv ✅ 🧪 mac-AS-debug-wk2
✅ 🛠 tv-sim ✅ 🧪 mac-wk2-stress
✅ 🛠 watch
✅ 🛠 🧪 unsafe-merge ✅ 🛠 watch-sim

@rniwa rniwa requested a review from cdumez as a code owner September 1, 2023 05:53
@rniwa rniwa self-assigned this Sep 1, 2023
@rniwa rniwa added the DOM For bugs specific to XML/HTML DOM elements (including parsing). label Sep 1, 2023
@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented Sep 1, 2023 •
edited
Loading

EWS run on previous version of this PR (hash ca389af)

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 ✅ 🛠 gtk
  🧪 ios-wk2-wpt   🧪 mac-wk1 ⏳ 🧪 gtk-wk2
✅ 🧪 api-ios   🧪 mac-wk2   🧪 api-gtk
✅ 🛠 tv ⏳ 🧪 mac-AS-debug-wk2
✅ 🛠 tv-sim
✅ 🛠 watch
✅ 🛠 watch-sim

@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented Sep 1, 2023 •
edited
Loading

EWS run on previous version of this PR (hash 0052ec7)

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 ✅ 🛠 gtk
  🧪 ios-wk2-wpt   🧪 mac-wk1 ⏳ 🧪 gtk-wk2
⏳ 🛠 🧪 jsc   🧪 api-ios   🧪 mac-wk2   🧪 api-gtk
✅ 🛠 tv ⏳ 🧪 mac-AS-debug-wk2
✅ 🛠 tv-sim
✅ 🛠 watch
✅ 🛠 watch-sim

@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented Sep 1, 2023 •
edited
Loading

EWS run on previous version of this PR (hash 24c18e6)

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 ✅ 🛠 gtk
✅ 🧪 ios-wk2-wpt ✅ 🧪 mac-wk1 ⏳ 🧪 gtk-wk2
✅ 🧪 api-ios ✅ 🧪 mac-wk2   🧪 api-gtk
✅ 🛠 tv ⏳ 🧪 mac-AS-debug-wk2
✅ 🛠 tv-sim
✅ 🛠 watch
✅ 🛠 watch-sim

@rniwa rniwa changed the title ContainerNode::replaceChildren creates a phantom DocumentFragment Add mutation events tests for ContainerNode::replaceChildren and cleanup the code Sep 1, 2023
@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented Sep 1, 2023 •
edited
Loading

EWS run on previous version of this PR (hash 96360e1)

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 ✅ 🛠 gtk
✅ 🧪 ios-wk2-wpt ✅ 🧪 mac-wk1   🧪 gtk-wk2
✅ 🧪 api-ios ✅ 🧪 mac-wk2   🧪 api-gtk
✅ 🛠 tv ⏳ 🧪 mac-AS-debug-wk2
✅ 🛠 tv-sim ✅ 🧪 mac-wk2-stress
✅ 🛠 watch
✅ 🛠 watch-sim

@rniwa rniwa changed the title Add mutation events tests for ContainerNode::replaceChildren and cleanup the code Add mutation events tests for ContainerNode::replaceChildren and refactor the code Sep 1, 2023
@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented Sep 1, 2023 •
edited
Loading

EWS run on previous version of this PR (hash 6b7d205)

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 ✅ 🛠 gtk
✅ 🧪 ios-wk2-wpt ✅ 🧪 mac-wk1 ⏳ 🧪 gtk-wk2
✅ 🧪 api-ios ✅ 🧪 mac-wk2 ✅ 🧪 api-gtk
✅ 🛠 tv ✅ 🧪 mac-AS-debug-wk2
✅ 🛠 tv-sim ✅ 🧪 mac-wk2-stress
✅ 🛠 watch
✅ 🛠 watch-sim

cdumez
cdumez approved these changes Sep 1, 2023
View reviewed changes
Copy link
Contributor

@cdumez cdumez left a comment

Choose a reason for hiding this comment

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

LGTM though we should closely monitor perf bots after landing.

LayoutTests/fast/dom/replace-children-mutation-events.html Show resolved Hide resolved
Source/WebCore/dom/Node.h Outdated Show resolved Hide resolved
Source/WebCore/dom/Node.cpp Outdated Show resolved Hide resolved
@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented Sep 1, 2023 •
edited
Loading

EWS run on current version of this PR (hash 5d3520b)

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 ✅ 🛠 gtk
✅ 🧪 ios-wk2-wpt ✅ 🧪 mac-wk1 ✅ 🧪 gtk-wk2
✅ 🧪 api-ios ✅ 🧪 mac-wk2 ✅ 🧪 api-gtk
✅ 🛠 tv ✅ 🧪 mac-AS-debug-wk2
✅ 🛠 tv-sim ✅ 🧪 mac-wk2-stress
✅ 🛠 watch
✅ 🛠 🧪 unsafe-merge ✅ 🛠 watch-sim

@rniwa rniwa added the unsafe-merge-queue Applied to send a pull request to merge-queue, but skip building and testing label Sep 1, 2023
@rniwa
Add mutation events tests for ContainerNode::replaceChildren and refa…
c177199 
…ctor the code

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

Reviewed by Chris Dumez.

Added some tests for mutating DOM during replaceChildren's tree mutating operations.

Also refactor the code in ContainerNode::replaceChildren by merging some aspects of
Node::convertNodesOrStringsIntoNode into the function so that all node removal &
insertion the logic will be in one place.

* LayoutTests/fast/dom/replace-children-mutation-events-expected.txt: Added.
* LayoutTests/fast/dom/replace-children-mutation-events.html: Added.
* Source/WebCore/dom/ContainerNode.cpp:
(WebCore::ContainerNode::replaceChildren):
* Source/WebCore/dom/Node.cpp:
(WebCore::Node::convertNodesOrStringsIntoNodeVector): Extracted from convertNodesOrStringsIntoNode.
(WebCore::Node::convertNodesOrStringsIntoNode):
* Source/WebCore/dom/Node.h:

Canonical link: https://commits.webkit.org/267565@main
@webkit-commit-queue
Copy link
Collaborator

Committed 267565@main (c177199): https://commits.webkit.org/267565@main

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

@webkit-commit-queue webkit-commit-queue merged commit c177199 into WebKit:main Sep 1, 2023
@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 Sep 1, 2023
@rniwa rniwa deleted the fix261016 branch May 21, 2024 17:20
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

Assignees

@rniwa rniwa

Labels
DOM For bugs specific to XML/HTML DOM elements (including parsing).
Projects
None yet
Milestone
No milestone
4 participants
@rniwa @webkit-early-warning-system @webkit-commit-queue @cdumez