Skip to content

script: Fix panic when deleting after range.deleteContents#44748

Merged
TimvdLippe merged 1 commit into
servo:mainfrom
TimvdLippe:fix-panic-on-parent-deletion
May 5, 2026
Merged

script: Fix panic when deleting after range.deleteContents#44748
TimvdLippe merged 1 commit into
servo:mainfrom
TimvdLippe:fix-panic-on-parent-deletion

Conversation

@TimvdLippe

@TimvdLippe TimvdLippe commented May 5, 2026

Copy link
Copy Markdown
Contributor

The state of the document after range.deleteContents leads to the delete command to traverse to the root element and then crash. Instead, we should simply stop the loop and continue the algorithm and delete backwards.

Also added an additional test I created when trying to figure out what other browsers are doing
and added spec comments to deleteContents. Lastly, also incorporate the spec fix of whatwg/dom#1452

Fixes #44742

Testing: WPT

@TimvdLippe
script: Fix panic when deleting after range.deleteContents
0d06667 
The state of the document after `range.deleteContents` leads
to the delete command to traverse to the root element and
then crash. Instead, we should simply stop the loop and
continue the algorithm and delete backwards.

Fixes servo#44742

Signed-off-by: Tim van der Lippe <[email protected]>
@TimvdLippe TimvdLippe requested a review from gterzian as a code owner May 5, 2026 17:47
@TimvdLippe TimvdLippe added the T-linux-wpt Do a try run of the WPT label May 5, 2026
@servo-highfive servo-highfive added the S-awaiting-review There is new code that needs to be reviewed. label May 5, 2026
@github-actions github-actions Bot removed the T-linux-wpt Do a try run of the WPT label May 5, 2026
@github-actions

github-actions Bot commented May 5, 2026

Copy link
Copy Markdown

🔨 Triggering try run (#25392739094) for Linux (WPT)

@servo-wpt-sync servo-wpt-sync mentioned this pull request May 5, 2026
Merged
@servo-wpt-sync

servo-wpt-sync commented May 5, 2026

Copy link
Copy Markdown
Collaborator

🤖 Opened new upstream WPT pull request (web-platform-tests/wpt#59670) with upstreamable changes.

@github-actions

github-actions Bot commented May 5, 2026

Copy link
Copy Markdown

Test results for linux-wpt from try job (#25392739094):

Flaky unexpected result (28)
  • ERROR [expected OK] /_mozilla/mozilla/img-load-more-than-cache.html
  • CRASH [expected OK] /_webgl/conformance/attribs/gl-disabled-vertex-attrib-update.html
  • OK /_webgl/conformance/textures/misc/texture-upload-size.html (#21770)
    • FAIL [expected PASS] subtest: WebGL test #45

      assert_true: Texture was smaller than the expected size 2x2 expected true got false

    • FAIL [expected PASS] subtest: WebGL test #47

      assert_true: getError expected: INVALID_VALUE. Was NO_ERROR : when calling texSubImage2D with the same texture upload with offset 1, 1 expected true got false

    • FAIL [expected PASS] subtest: WebGL test #49

      assert_true: Texture was smaller than the expected size 2x2 expected true got false

    • FAIL [expected PASS] subtest: WebGL test #51

      assert_true: getError expected: INVALID_VALUE. Was NO_ERROR : when calling texSubImage2D with the same texture upload with offset 1, 1 expected true got false

    • PASS [expected FAIL] subtest: WebGL test #53
    • PASS [expected FAIL] subtest: WebGL test #55
    • PASS [expected FAIL] subtest: WebGL test #57
    • PASS [expected FAIL] subtest: WebGL test #59
    • FAIL [expected PASS] subtest: WebGL test #61

      assert_true: Texture was smaller than the expected size 2x2 expected true got false

    • FAIL [expected PASS] subtest: WebGL test #63

      assert_true: getError expected: INVALID_VALUE. Was NO_ERROR : when calling texSubImage2D with the same texture upload with offset 1, 1 expected true got false

    • And 14 more unexpected results...
  • OK /beacon/beacon-basic.https.window.html (#41723)
    • FAIL [expected PASS] subtest: Payload size restriction should be accumulated: type = blob

      assert_false: expected false got true

  • FAIL [expected PASS] /css/css-backgrounds/background-size-041.html
  • OK /css/css-fonts/generic-family-keywords-002.html (#40929)
    • FAIL [expected PASS] subtest: font-family: -webkit-serif treated as &lt;font-family&gt;, not &lt;generic-name&gt;

      assert_equals: expected 30 but got 50

    • FAIL [expected PASS] subtest: font-family: -webkit-sans-serif treated as &lt;font-family&gt;, not &lt;generic-name&gt;

      assert_equals: expected 30 but got 50

    • FAIL [expected PASS] subtest: font-family: -webkit-cursive treated as &lt;font-family&gt;, not &lt;generic-name&gt;

      assert_equals: expected 30 but got 50

    • FAIL [expected PASS] subtest: font-family: -webkit-fantasy treated as &lt;font-family&gt;, not &lt;generic-name&gt;

      assert_equals: expected 30 but got 50

    • FAIL [expected PASS] subtest: font-family: -webkit-monospace treated as &lt;font-family&gt;, not &lt;generic-name&gt;

      assert_equals: expected 30 but got 50

    • FAIL [expected PASS] subtest: font-family: -webkit-system-ui treated as &lt;font-family&gt;, not &lt;generic-name&gt;

      assert_equals: expected 30 but got 50

    • FAIL [expected PASS] subtest: font-family: -webkit-math treated as &lt;font-family&gt;, not &lt;generic-name&gt;

      assert_equals: expected 30 but got 50

    • PASS [expected FAIL] subtest: font-family: -webkit-generic(fangsong) treated as &lt;font-family&gt;, not &lt;generic-name&gt;
    • PASS [expected FAIL] subtest: font-family: -webkit-generic(kai) treated as &lt;font-family&gt;, not &lt;generic-name&gt;
    • PASS [expected FAIL] subtest: font-family: -webkit-generic(khmer-mul) treated as &lt;font-family&gt;, not &lt;generic-name&gt;
    • And 12 more unexpected results...
  • FAIL [expected PASS] /css/css-ui/appearance-button-001.html
  • CRASH [expected OK] /fetch/api/cors/cors-cookies.any.html
  • CRASH [expected OK] /fetch/api/cors/cors-redirect.any.html
  • OK /html/browsers/browsing-the-web/navigating-across-documents/navigation-unload-cross-origin.sub.window.html (#29056)
    • PASS [expected FAIL] subtest: Cross-origin navigation started from unload handler must be ignored
  • OK /html/browsers/browsing-the-web/navigating-across-documents/navigation-unload-same-origin.window.html (#29049)
    • PASS [expected FAIL] subtest: Same-origin navigation started from unload handler must be ignored
  • OK /html/browsers/browsing-the-web/navigating-across-documents/replace-before-load/a-click.html (#28697)
    • PASS [expected FAIL] subtest: aElement.click() before the load event must NOT replace
  • OK /html/browsers/history/the-history-interface/traverse_the_history_2.html (#21383)
    • PASS [expected FAIL] subtest: Multiple history traversals, last would be aborted
  • OK /html/browsers/history/the-history-interface/traverse_the_history_5.html (#21383)
    • PASS [expected FAIL] subtest: Multiple history traversals, last would be aborted
  • OK [expected TIMEOUT] /html/infrastructure/urls/base-url/document-base-url-window-initiator-is-not-opener.https.window.html (#30970)
  • TIMEOUT [expected OK] /html/interaction/focus/the-autofocus-attribute/update-the-rendering.html (#24145)
    • TIMEOUT [expected FAIL] subtest: "Flush autofocus candidates" should be happen before a scroll event and animation frame callbacks

      Test timed out

  • OK /html/semantics/document-metadata/the-meta-element/pragma-directives/attr-meta-http-equiv-refresh/allow-scripts-flag-changing-2.html (#39703)
    • FAIL [expected PASS] subtest: Meta refresh of the original iframe is not blocked if moved into a sandboxed iframe

      uncaught exception: Error: assert_unreached: The iframe into which the meta was moved must not refresh Reached unreachable code

  • OK /html/semantics/forms/form-submission-0/jsurl-form-submit.tentative.html (#36489)
    • PASS [expected FAIL] subtest: Verifies that form submissions scheduled inside javascript: urls take precedence over the javascript: url's return value.
  • OK /html/semantics/scripting-1/the-script-element/module/choice-of-error-1.html (#44058)
    • PASS [expected FAIL] subtest: Parse errors in different files should be reported depending on different roots
  • ERROR [expected OK] /intersection-observer/v2/animated-opacity.html (#40827)
  • TIMEOUT [expected OK] /pointerevents/compat/pointerevent_touch-action_two-finger_interaction.html (#44693)
    • NOTRUN [expected PASS] subtest: touch two-finger pan on 'touch-action: pan-x pan-y'
    • NOTRUN [expected FAIL] subtest: touch two-finger pan on 'touch-action: pinch-zoom'
  • OK [expected TIMEOUT] /trusted-types/trusted-types-navigation.html?01-05 (#38975)
    • PASS [expected TIMEOUT] subtest: Navigate a window via anchor with javascript:-urls in report-only mode.
    • PASS [expected NOTRUN] subtest: Navigate a window via anchor with javascript:-urls w/ default policy in report-only mode.
    • PASS [expected NOTRUN] subtest: Navigate a frame via anchor with javascript:-urls in enforcing mode.
  • TIMEOUT /trusted-types/trusted-types-navigation.html?26-30 (#38807)
    • PASS [expected TIMEOUT] subtest: Navigate a window via form-submission with javascript:-urls in report-only mode.
    • PASS [expected NOTRUN] subtest: Navigate a window via form-submission with javascript:-urls w/ default policy in report-only mode.
    • PASS [expected NOTRUN] subtest: Navigate a frame via form-submission with javascript:-urls in enforcing mode.
    • TIMEOUT [expected NOTRUN] subtest: Navigate a frame via form-submission with javascript:-urls w/ default policy in enforcing mode.

      Test timed out

  • TIMEOUT /trusted-types/trusted-types-navigation.html?31-35 (#38034)
    • TIMEOUT [expected PASS] subtest: Navigate a frame via form-submission with javascript:-urls in report-only mode.

      Test timed out

    • NOTRUN [expected TIMEOUT] subtest: Navigate a frame via form-submission with javascript:-urls w/ default policy in report-only mode.
  • OK /webdriver/tests/classic/execute_async_script/promise.py
    • FAIL [expected PASS] subtest: test_promise_resolve

      AssertionError: no such window (404): No such window

  • OK /webdriver/tests/classic/find_element_from_shadow_root/find.py
    • FAIL [expected PASS] subtest: test_null_parameter_value

      webdriver.error.NoSuchWindowException: no such window (404): No such window

  • OK /webdriver/tests/classic/get_element_attribute/get.py
    • ERROR [expected PASS] subtest: test_no_top_browsing_context

      setup error: webdriver.error.NoSuchElementException: no such element (404)

  • CRASH [expected OK] /workers/constructors/Worker/ctor-1.html
Stable unexpected results that are known to be intermittent (16)
  • TIMEOUT /FileAPI/url/url-in-tags-revoke.window.html (#19978)
    • TIMEOUT [expected PASS] subtest: Fetching a blob URL immediately before revoking it works in &lt;script&gt; tags.

      Test timed out

  • TIMEOUT [expected OK] /_mozilla/mozilla/window-resize-event.html (#36741)
    • TIMEOUT [expected PASS] subtest: Popup onresize event fires after resizeTo

      Test timed out

  • OK [expected TIMEOUT] /credential-management/credentialscontainer-frame-basics.https.html (#39430)
    • FAIL [expected TIMEOUT] subtest: navigator.credentials should be undefined in documents generated from data: URLs.

      assert_equals: expected "undefined" but got "[object CredentialsContainer]"

  • OK /css/css-fonts/generic-family-keywords-001.html (#37467)
    • PASS [expected FAIL] subtest: @font-face matching for quoted and unquoted generic(fangsong)
    • PASS [expected FAIL] subtest: @font-face matching for quoted and unquoted generic(khmer-mul)
  • OK /css/css-fonts/generic-family-keywords-003.html (#38994)
    • FAIL [expected PASS] subtest: @font-face matching for quoted and unquoted cursive (drawing text in a canvas)

      assert_equals: quoted cursive matches  @font-face rule expected 125 but got 40

    • FAIL [expected PASS] subtest: @font-face matching for quoted and unquoted fantasy (drawing text in a canvas)

      assert_equals: quoted fantasy matches  @font-face rule expected 125 but got 40

    • FAIL [expected PASS] subtest: @font-face matching for quoted and unquoted math (drawing text in a canvas)

      assert_equals: unquoted math does not match @font-face rule expected 40 but got 125

    • FAIL [expected PASS] subtest: @font-face matching for quoted and unquoted generic(nastaliq) (drawing text in a canvas)

      assert_equals: quoted generic(nastaliq) matches  @font-face rule expected 125 but got 40

  • TIMEOUT /fetch/metadata/generated/css-images.sub.tentative.html (#29047)
    • FAIL [expected PASS] subtest: content sec-fetch-site - HTTPS downgrade-upgrade

      assert_unreached: Reached unreachable code

  • TIMEOUT [expected OK] /html/browsers/browsing-the-web/navigating-across-documents/replace-before-load/form-submit-button-click.html (#44099)
    • TIMEOUT [expected FAIL] subtest: Replace before load, triggered by submitButton.click()

      Test timed out

  • TIMEOUT [expected OK] /html/browsers/browsing-the-web/navigating-across-documents/replace-before-load/form-submit.html (#44028)
  • OK [expected ERROR] /html/canvas/offscreen/text/2d.text.measure.getActualBoundingBox.tentative.html (#43710)
  • TIMEOUT /html/interaction/focus/the-autofocus-attribute/supported-elements.html (#24145)
    • TIMEOUT [expected NOTRUN] subtest: Non-HTMLElement should not support autofocus

      Test timed out

  • TIMEOUT [expected OK] /html/semantics/embedded-content/the-iframe-element/iframe_sandbox_navigate_other_frame_popup.sub.html (#39702)
    • TIMEOUT [expected FAIL] subtest: Sandboxed iframe can not navigate other frame's popup

      Test timed out

  • OK /html/semantics/scripting-1/the-script-element/module/dynamic-import/blob-url.any.worker.html (#33909)
    • PASS [expected FAIL] subtest: Revoking a blob URL immediately after calling import will not fail
  • OK /navigation-timing/test-navigation-type-reload.html (#33334)
    • FAIL [expected PASS] subtest: Reload domComplete &gt; Original domComplete

      assert_true: Reload domComplete &gt; Original domComplete expected true got false

    • FAIL [expected PASS] subtest: Reload domContentLoadedEventEnd &gt; Original domContentLoadedEventEnd

      assert_true: Reload domContentLoadedEventEnd &gt; Original domContentLoadedEventEnd expected true got false

    • FAIL [expected PASS] subtest: Reload domContentLoadedEventStart &gt; Original domContentLoadedEventStart

      assert_true: Reload domContentLoadedEventStart &gt; Original domContentLoadedEventStart expected true got false

    • FAIL [expected PASS] subtest: Reload domInteractive &gt; Original domInteractive

      assert_true: Reload domInteractive &gt; Original domInteractive expected true got false

    • FAIL [expected PASS] subtest: Reload loadEventEnd &gt; Original loadEventEnd

      assert_true: Reload loadEventEnd &gt; Original loadEventEnd expected true got false

    • FAIL [expected PASS] subtest: Reload loadEventStart &gt; Original loadEventStart

      assert_true: Reload loadEventStart &gt; Original loadEventStart expected true got false

  • FAIL [expected PASS] /png/apng/acTL-plays-two.html (#41191)
  • OK /trusted-types/trusted-types-reporting.html (#43737)
    • FAIL [expected PASS] subtest: Trusted Type violation report: creating a forbidden-but-not-reported policy.

      assert_equals: a single violation reported expected 1 but got 0

  • TIMEOUT [expected OK] /webstorage/localstorage-about-blank-3P-iframe-opens-3P-window.partitioned.html (#29053)
    • TIMEOUT [expected FAIL] subtest: StorageKey: test 3P about:blank window opened from a 3P iframe

      Test timed out

@github-actions

github-actions Bot commented May 5, 2026

Copy link
Copy Markdown

✨ Try run (#25392739094) succeeded.

Taym95
Taym95 approved these changes May 5, 2026
View reviewed changes

@Taym95 Taym95 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Nice spec alignment, deleteContents() ordering matches the spec!

@servo-highfive servo-highfive removed the S-awaiting-review There is new code that needs to be reviewed. label May 5, 2026
@TimvdLippe TimvdLippe added this pull request to the merge queue May 5, 2026
@servo-highfive servo-highfive added the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label May 5, 2026
Merged via the queue into servo:main with commit ff92c64 May 5, 2026
71 checks passed
@TimvdLippe TimvdLippe deleted the fix-panic-on-parent-deletion branch May 5, 2026 19:20
@servo-highfive servo-highfive removed the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label May 5, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Taym95 Taym95 Taym95 approved these changes
@gterzian gterzian Awaiting requested review from gterzian gterzian is a code owner

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

panic: Must always have a parent components/script/dom/execcommand/commands/delete.rs:193

4 participants

@TimvdLippe @servo-wpt-sync @Taym95 @servo-highfive