Skip to content

script: Implement removeFormat command - #44710

Merged
TimvdLippe merged 1 commit into
servo:mainfrom
TimvdLippe:implement-remove-format-command
May 5, 2026
Merged

script: Implement removeFormat command#44710
TimvdLippe merged 1 commit into
servo:mainfrom
TimvdLippe:implement-remove-format-command

Conversation

@TimvdLippe

@TimvdLippe TimvdLippe commented May 4, 2026

Copy link
Copy Markdown
Contributor

This is the last inline formatting command. It also fixes an issue with effectively contained nodes not including the parents of a fully selected text node.

The result is that now more correct HTML is generated, but several query results now are wrong as the selection is incorrectly updated. I need to investigate further why that is happening.

Part of #25005

Testing: WPT

@TimvdLippe TimvdLippe added the T-linux-wpt Do a try run of the WPT label May 4, 2026
@github-actions github-actions Bot removed the T-linux-wpt Do a try run of the WPT label May 4, 2026
@github-actions

github-actions Bot commented May 4, 2026

Copy link
Copy Markdown

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

@github-actions

github-actions Bot commented May 4, 2026

Copy link
Copy Markdown

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

Flaky unexpected result (27)
  • CRASH [expected OK] /_webgl/conformance/extensions/angle-instanced-arrays-out-of-bounds.html
  • OK /beacon/beacon-basic.https.window.html (#41723)
    • FAIL [expected PASS] subtest: Payload size restriction should be accumulated: type = arraybuffer

      assert_false: expected false got true

  • CRASH [expected OK] /content-security-policy/meta/sandbox-iframe.html (#43478)
  • FAIL [expected PASS] /css/css-backgrounds/background-size-041.html
  • OK /css/css-fonts/variations/font-weight-matching.html (#38577)
    • FAIL [expected PASS] subtest: Test @font-face matching for weight 420

      assert_approx_equals: @font-face should be mapped to CSSTest Weights 600. expected 90 +/- 2 but got 180

  • PASS [expected FAIL] /css/css-ui/appearance-menulist-button-002.tentative.html
  • TIMEOUT [expected OK] /fetch/api/redirect/redirect-keepalive.https.any.html (#32153)
    • TIMEOUT [expected PASS] subtest: [keepalive][iframe][load] mixed content redirect; setting up

      Test timed out

  • CRASH [expected OK] /fetch/api/request/request-cache-no-cache.any.worker.html
  • OK /fetch/content-length/api-and-duplicate-headers.any.html (#35873)
    • FAIL [expected PASS] subtest: fetch() and duplicate Content-Length/Content-Type headers

      promise_test: Unhandled rejection with value: object "TypeError: Network error: HTTP failure: client error (SendRequest)"

  • OK [expected ERROR] /fetch/fetch-later/quota/same-origin-iframe/multiple-iframes.https.window.html (#35176)
  • OK [expected ERROR] /fetch/fetch-later/quota/same-origin-iframe/sandboxed-iframe.https.window.html (#41704)
  • OK [expected TIMEOUT] /html/browsers/browsing-the-web/navigating-across-documents/replace-before-load/form-requestsubmit.html (#44098)
    • FAIL [expected TIMEOUT] subtest: Replace before load, triggered by formElement.requestSubmit()

      assert_equals: expected "http://web-platform.test:8000/common/blank.html?thereplacement=" but got "http://web-platform.test:8000/html/browsers/browsing-the-web/navigating-across-documents/replace-before-load/resources/code-injector.html?pipe=sub(none)&code=%0A%20%20%20%20const%20form%20%3D%20document.createElement(%22form%22)%3B%0A%20%20%20%20form.action%20%3D%20%22%2Fcommon%2Fblank.html%22%3B%0A%0A%20%20%20%20const%20input%20%3D%20document.createElement(%22input%22)%3B%0A%20%20%20%20input.type%20%3D%20%22hidden%22%3B%0A%20%20%20%20input.name%20%3D%20%22thereplacement%22%3B%0A%20%20%20%20form.append(input)%3B%0A%0A%20%20%20%20document.currentScript.before(form)%3B%0A%20%20%20%20form.requestSubmit()%3B%0A%20%20"

  • CRASH [expected TIMEOUT] /html/browsers/history/the-location-interface/location_replace_session_history.html (#41896)
  • 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/embedded-content/the-iframe-element/iframe-loading-lazy-nav-location-replace-set-src.html (#32697)
    • PASS [expected FAIL] subtest: Navigating iframe loading='lazy' and then setting src: location.replace
  • 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/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/dynamic-import/blob-url.any.html (#33948)
    • FAIL [expected PASS] subtest: Revoking a blob URL immediately after calling import will not fail

      promise_test: Unhandled rejection with value: object "TypeError: Module fetching failed"

  • OK /html/semantics/scripting-1/the-script-element/module/dynamic-import/blob-url.any.worker-module.html (#43510)
    • FAIL [expected PASS] subtest: Revoking a blob URL immediately after calling import will not fail

      promise_test: Unhandled rejection with value: object "TypeError: Module fetching failed"

  • TIMEOUT [expected OK] /html/user-activation/navigation-state-reset-sameorigin.html
    • TIMEOUT [expected PASS] subtest: Post-navigation state reset.

      Test timed out

  • FAIL [expected PASS] /png/apng/fcTL-blend-source-solid.html (#41560)
  • 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 /touch-events/single-tap-when-touchend-listener-use-sync-xhr.html (#41175)
    • PASS [expected FAIL] subtest: Click event should be fired when touchend opens synchronous XHR
  • OK [expected TIMEOUT] /trusted-types/trusted-types-navigation.html?06-10 (#37920)
    • PASS [expected FAIL] subtest: Navigate a frame via anchor with javascript:-urls in report-only mode.
    • PASS [expected TIMEOUT] subtest: Navigate a frame via anchor with javascript:-urls w/ default policy in report-only mode.
    • FAIL [expected NOTRUN] subtest: Navigate a window via anchor with javascript:-urls w/ a default policy throwing an exception in enforcing mode.

      promise_test: Unhandled rejection with value: "Unexpected message received: \"No securitypolicyviolation reported!\""

    • FAIL [expected NOTRUN] subtest: Navigate a window via anchor with javascript:-urls w/ a default policy throwing an exception in report-only mode.

      promise_test: Unhandled rejection with value: "Unexpected message received: \"No securitypolicyviolation reported!\""

  • 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.
  • CRASH [expected OK] /webaudio/the-audio-api/the-mediastreamaudiodestinationnode-interface/closed-audiocontext-construction.html
  • OK [expected ERROR] /webxr/render_state_update.https.html (#27535)
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 <script> 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 /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 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(khmer-mul) (drawing text in a canvas)

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

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

      assert_equals: unquoted generic(nastaliq) does not match @font-face rule expected 40 but got 125

  • OK /fetch/metadata/generated/css-font-face.https.sub.tentative.html (#32732)
    • PASS [expected FAIL] subtest: sec-fetch-storage-access - Cross-site
  • OK /html/browsers/browsing-the-web/navigating-across-documents/refresh/same-document-refresh.html (#34597)
    • PASS [expected FAIL] subtest: Same-Document Referrer from Refresh
  • 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

  • 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/interaction/focus/the-autofocus-attribute/supported-elements.html (#24145)
    • FAIL [expected NOTRUN] subtest: Non-HTMLElement should not support autofocus

      promise_test: Unhandled rejection with value: object "TypeError: can't access property "tagName", w.document.activeElement is null"

    • FAIL [expected NOTRUN] subtest: Host element with delegatesFocus should support autofocus

      assert_equals: expected Element node <div autofocus=""></div> but got Element node <body><div autofocus=""></div></body>

    • FAIL [expected NOTRUN] subtest: Host element with delegatesFocus including no focusable descendants should be skipped

      assert_equals: expected Element node <input autofocus=""></input> but got Element node <body><div autofocus=""></div><input autofocus=""></body>

    • FAIL [expected NOTRUN] subtest: Area element should support autofocus

      promise_test: Unhandled rejection with value: object "TypeError: can't access property "appendChild", w.document.querySelector(...) is null"

  • TIMEOUT /html/semantics/embedded-content/media-elements/preserves-pitch.html (#40352)
    • PASS [expected TIMEOUT] subtest: Speed-ups should not change the pitch when preservesPitch=true
    • PASS [expected NOTRUN] subtest: Slow-downs should not change the pitch when preservesPitch=true
    • TIMEOUT [expected NOTRUN] subtest: Speed-ups should change the pitch when preservesPitch=false

      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 > Original domComplete

      assert_true: Reload domComplete > Original domComplete expected true got false

    • FAIL [expected PASS] subtest: Reload domContentLoadedEventEnd > Original domContentLoadedEventEnd

      assert_true: Reload domContentLoadedEventEnd > Original domContentLoadedEventEnd expected true got false

    • FAIL [expected PASS] subtest: Reload domContentLoadedEventStart > Original domContentLoadedEventStart

      assert_true: Reload domContentLoadedEventStart > Original domContentLoadedEventStart expected true got false

    • FAIL [expected PASS] subtest: Reload domInteractive > Original domInteractive

      assert_true: Reload domInteractive > Original domInteractive expected true got false

    • FAIL [expected PASS] subtest: Reload loadEventEnd > Original loadEventEnd

      assert_true: Reload loadEventEnd > Original loadEventEnd expected true got false

    • FAIL [expected PASS] subtest: Reload loadEventStart > Original loadEventStart

      assert_true: Reload loadEventStart > Original loadEventStart expected true got false

  • FAIL [expected PASS] /png/apng/acTL-plays-two.html (#41191)
  • OK /resource-timing/buffer-full-then-increased.html (#44408)
    • FAIL [expected PASS] subtest: Test that overflowing the buffer and immediately increasing its limit does not trigger the resourcetimingbufferfull event

      assert_equals: Number of entries does not match the expected value. expected 3 but got 1

  • OK /resource-timing/test_resource_timing.html (#25720)
    • PASS [expected FAIL] subtest: PerformanceEntry has correct name, initiatorType, startTime, and duration (script)
  • OK /trusted-types/trusted-types-reporting.html (#43737)
    • PASS [expected FAIL] subtest: Trusted Type violation report: sample for SVGScriptElement href assignment by setAttribute
Stable unexpected results (1)
  • OK /editing/other/removeformat_background_color.html
    • PASS [expected FAIL] subtest: removeFormat removes background-color from bold element
    • PASS [expected FAIL] subtest: removeFormat removes background-color from span element
    • PASS [expected FAIL] subtest: removeFormat removes multiple styles including background-color
    • PASS [expected FAIL] subtest: removeFormat removes background-color from nested elements
    • PASS [expected FAIL] subtest: removeFormat handles complex background-color values
    • PASS [expected FAIL] subtest: removeFormat preserves text when only background-color is present

@github-actions

github-actions Bot commented May 4, 2026

Copy link
Copy Markdown

⚠️ Try run (#25309727310) failed!

@TimvdLippe
TimvdLippe force-pushed the implement-remove-format-command branch from 1cce8a4 to 2cc2af6 Compare May 5, 2026 06:31
@TimvdLippe
TimvdLippe marked this pull request as ready for review May 5, 2026 06:31
@TimvdLippe
TimvdLippe requested a review from gterzian as a code owner May 5, 2026 06:31
@servo-highfive servo-highfive added the S-awaiting-review There is new code that needs to be reviewed. label May 5, 2026
@TimvdLippe

TimvdLippe commented May 5, 2026

Copy link
Copy Markdown
Contributor Author

Decided to defer the other fix to a separate PR, as to not muck with this one. Investigating now what's going on with that.

jdm
jdm approved these changes May 5, 2026
View reviewed changes
Comment thread components/script/dom/execcommand/commands/removeformat.rs

/// <https://w3c.github.io/editing/docs/execCommand/#removeformat-candidate>
fn is_remove_format_candidate(element: &Element) -> bool {
// > A removeFormat candidate is an editable HTML element with local name

@jdm jdm May 5, 2026

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.

Are we checking the editable condition anywhere?

@TimvdLippe TimvdLippe May 5, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It's implied by for_each_effectively_contained_child. I am actually going to update that logic in a future PR to change these to asserts, since they are implied by all the logic surrounding. That should speed up things, as we are currently checking the same invariants over and over, which isn't necessary for runtime.

@servo-highfive servo-highfive removed the S-awaiting-review There is new code that needs to be reviewed. label May 5, 2026
@TimvdLippe
script: Implement removeFormat command
4cb67b7 
This is the last inline formatting command. It also fixes an
issue with effectively contained nodes not including the
parents of a fully selected text node.

The result is that now more correct HTML is generated, but
several query results now are wrong as the selection is
incorrectly updated. I need to investigate further why
that is happening.

Part of servo#25005

Signed-off-by: Tim van der Lippe <tvanderlippe@gmail.com>
@TimvdLippe
TimvdLippe force-pushed the implement-remove-format-command branch from 56e917e to 4cb67b7 Compare May 5, 2026 14:41
@servo-highfive servo-highfive added the S-awaiting-review There is new code that needs to be reviewed. label May 5, 2026
@TimvdLippe
TimvdLippe enabled auto-merge May 5, 2026 14:41
@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
@yezhizhen
yezhizhen removed this pull request from the merge queue due to a manual request May 5, 2026
@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
@yezhizhen

yezhizhen commented May 5, 2026

Copy link
Copy Markdown
Member

https://github.com/servo/servo/actions/runs/25383651088/job/74443298659

@TimvdLippe

TimvdLippe commented May 5, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author 
The job was not acquired by Runner of type hosted even after multiple attempts

https://github.com/servo/servo/actions/runs/25383651088/job/74440354414

@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 fcda21f May 5, 2026
36 checks passed
@TimvdLippe
TimvdLippe deleted the implement-remove-format-command branch May 5, 2026 16:45
@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

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

Assignees

No one assigned

Labels

S-awaiting-review There is new code that needs to be reviewed.

Milestone

No milestone