Skip to content

DOM: Make moveBefore() throw for all pre-move validity checks#48642

Merged
KyleJu merged 1 commit into
masterfrom
chromium-export-cl-5935350
Oct 21, 2024
Merged

DOM: Make moveBefore() throw for all pre-move validity checks#48642
KyleJu merged 1 commit into
masterfrom
chromium-export-cl-5935350

Conversation

@chromium-wpt-export-bot
Copy link
Copy Markdown
Collaborator

@chromium-wpt-export-bot chromium-wpt-export-bot commented Oct 16, 2024

This CL makes moveBefore() match the spec PR 1, with regard to the
agreed-upon error-throwing behavior for all pre-moving validity and
hierarchy conditions. This means throwing an exception for:

  • Disconnected parent destination or move target
  • Cross-document Nodes
  • Destination parent that is not an Element node
  • Move target that is not an Element or character data

R=nrosenthal@chromium.org

Bug: 40150299
Change-Id: Iaf5243fb2762e21ede068a222600bd158859fe92
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5935350
Reviewed-by: Noam Rosenthal <nrosenthal@chromium.org>
Commit-Queue: Dominic Farolino <dom@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1369303}

This CL makes moveBefore() match the spec PR [1], with regard to the
agreed-upon error-throwing behavior for all pre-moving validity and
hierarchy conditions. This means throwing an exception for:
 - Disconnected parent destination or move target
 - Cross-document Nodes
 - Destination parent that is not an Element node
 - Move target that is not an Element or character data

[1]: whatwg/dom#1307

R=nrosenthal@chromium.org

Bug: 40150299
Change-Id: Iaf5243fb2762e21ede068a222600bd158859fe92
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5935350
Reviewed-by: Noam Rosenthal <nrosenthal@chromium.org>
Commit-Queue: Dominic Farolino <dom@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1369303}
Copy link
Copy Markdown
Collaborator

@wpt-pr-bot wpt-pr-bot left a comment

Choose a reason for hiding this comment

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

The review process for this patch is being conducted in the Chromium project.

@dev-ansung
Copy link
Copy Markdown
Contributor

WPT Command: python3 ./wpt run --channel=nightly --verify --verify-no-chaos-mode --verify-repeat-loop=0 --verify-repeat-restart=10 --github-checks-text-file=/home/test/artifacts/checkrun.md --affected base_head --log-mach-level=info --log-mach=- -y --no-pause --no-restart-on-unexpected --install-fonts --no-headless --verify-log-full --binary=/home/test/build/firefox/firefox firefox

Some affected tests had inconsistent (flaky) results:

Unstable results

Test Subtest Results Messages
/dom/nodes/moveBefore/tentative/css-transition-cross-document.html OK: 1/10, TIMEOUT: 9/10
/dom/nodes/moveBefore/tentative/css-transition-cross-document.html Moving a transition across documents should reset its state FAIL: 1/10, TIMEOUT: 9/10 Test timed out;assert_throws_dom: function "() => {\n document.querySelector("#new-parent").moveBefore(item, null);\n }" threw object "TypeError: document.querySelector(...).moveBefore is not a function" that is not a DOMException HIERARCHY_REQUEST_ERR: property "code" is equal to undefined, expected 3

These may be pre-existing or new flakes. Please try to reproduce (see the above WPT command, though some flags may not be needed when running locally) and determine if your change introduced the flake. If you are unable to reproduce the problem, please tag @web-platform-tests/wpt-core-team in a comment for help.

@dev-ansung
Copy link
Copy Markdown
Contributor

Created crbug.com/374788499
@KyleJu @jcscottiii can you help admin merge, thanks!

@KyleJu KyleJu merged commit 06b6ff6 into master Oct 21, 2024
@KyleJu KyleJu deleted the chromium-export-cl-5935350 branch October 21, 2024 18:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants