script: Clear all associated event listeners when removing an event listener content attribute. #39011

kotx · 2025-08-29T04:45:34Z

This change allows callers of remove_event_listener to specify None for the options argument to skip phase (bubble/capture) checking (and remove either type of listener).

Notably, this changes the HTMLElement attribute_mutated code to remove all event listeners rather than ones with just capture: false, which should be correct behavior.

Testing: mach try linux-wpt: https://github.com/kotx/servo/actions/runs/17313405730
Fixes: #38742

Signed-off-by: Kot <kot@kot.pink>

TimvdLippe · 2025-08-29T05:50:07Z

components/script/dom/htmlelement.rs

@@ -1135,7 +1135,7 @@ impl VirtualMethods for HTMLElement {
                        evtarget.remove_event_listener(
                            event_name.into(),
                            evtarget.get_event_handler_common(event_name, can_gc),
-                            EventListenerOptions { capture: false },
+                            None,


Can you check why the test linked in #38734 (comment) is still failing with this patch? I would expect it to be fixed, which was the original motivation for the issue you linked

I think we should be using set_event_handler_common instead of remove_event_listener here? It doesn't make sense to run EventTarget.removeEventListener on inline event listeners (onclick, etc). Which means we wouldn't need to make options an Option.

Signed-off-by: Kot <kot@kot.pink>

github-actions · 2025-08-30T01:48:29Z

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

github-actions · 2025-08-30T02:14:09Z

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

Flaky unexpected result (23)

CRASH [expected TIMEOUT] /IndexedDB/crashtests/create-index.any.html
OK /IndexedDB/idbfactory_open.any.html
- FAIL [expected PASS] subtest: Calling open() with version argument 1.5 should not throw.
```
assert_equals: version expected 1 but got 9007199254740991
```
FAIL [expected PASS] /_mozilla/gfx-rs-gecko/descriptor-ranges.html (#23258)
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-cascade/layer-font-face-override.html (#35935)
- PASS [expected FAIL] subtest: @font-face override update with appended sheet 1
- PASS [expected FAIL] subtest: @font-face override update with appended sheet 2
OK /fetch/metadata/generated/css-font-face.https.sub.tentative.html (#32732)
- PASS [expected FAIL] subtest: sec-fetch-dest
- PASS [expected FAIL] subtest: sec-fetch-storage-access - Cross-site
ERROR /fetch/metadata/generated/serviceworker.https.sub.html (#36247)
- PASS [expected FAIL] subtest: sec-fetch-site - Same origin, no options - registration
OK /html/browsers/browsing-the-web/navigating-across-documents/navigation-unload-same-origin-fragment.html (#20768)
- FAIL [expected PASS] subtest: Tests that a fragment navigation in the unload handler will not block the initial navigation
```
assert_equals: expected "" but got "#fragment"
```
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/interaction/focus/the-autofocus-attribute/autofocus-dialog.html (#29087)
- TIMEOUT [expected FAIL] subtest: <dialog>-contained autofocus element gets focused when the dialog is shown
```
Test timed out
```
ERROR [expected OK] /html/semantics/embedded-content/media-elements/event_timeupdate_noautoplay.html (#25046)
OK /html/semantics/embedded-content/media-elements/loading-the-media-resource/resource-selection-invoke-pause-networkState.html
- FAIL [expected PASS] subtest: NOT invoking resource selection with pause() when networkState is not NETWORK_EMPTY
```
assert_equals: networkState in onerror expected 3 but got 1
```
OK /html/semantics/embedded-content/media-elements/seeking/seek-to-currentTime.html (#38247)
- FAIL [expected PASS] subtest: seek to currentTime
```
assert_greater_than: seekable ranges expected a number greater than 0 but got 0
```
OK [expected CRASH] /html/semantics/embedded-content/the-iframe-element/iframe_sandbox_popups_escaping-3.html (#24057)
OK /html/semantics/forms/form-submission-0/urlencoded2.window.html (#28687)
- PASS [expected FAIL] subtest: application/x-www-form-urlencoded: Basic test (normal form)
OK /intersection-observer/callback-cross-realm-report-exception.html (#38829)
- FAIL [expected PASS] subtest: IntersectionObserver reports the exception from its callback in the callback's global object
```
assert_array_equals: lengths differ, expected array ["frame1"] length 1, got [] length 0
```
TIMEOUT /resource-timing/test_resource_timing.https.html (#25216)
- PASS [expected FAIL] subtest: PerformanceEntry has correct name, initiatorType, startTime, and duration (img)
TIMEOUT [expected OK] /service-workers/service-worker/detached-context.https.html
- TIMEOUT [expected FAIL] subtest: accessing navigator.serviceWorker on a detached iframe
```
Test timed out
```
OK /trusted-types/trusted-types-navigation.html?01-05 (#38975)
- PASS [expected FAIL] subtest: Navigate a window via anchor with javascript:-urls in enforcing mode.
OK [expected TIMEOUT] /webmessaging/without-ports/018.html (#24485)
- PASS [expected TIMEOUT] subtest: origin of the script that invoked the method, javascript:
TIMEOUT [expected OK] /webstorage/localstorage-about-blank-3P-iframe-opens-3P-window.partitioned.html (#29053)
- TIMEOUT [expected PASS] subtest: StorageKey: test 3P about:blank window opened from a 3P iframe
```
Test timed out
```
OK [expected ERROR] /workers/constructors/Worker/Worker-constructor.html (#22991)
OK /xhr/open-url-multi-window-5.htm (#23360)
- FAIL [expected PASS] subtest: XMLHttpRequest: open() resolving URLs (multi-Window; 5)
```
assert_throws_dom: function "function() {client.open("GET", "...") }" did not throw
```

Stable unexpected results that are known to be intermittent (25)

FAIL [expected PASS] /_mozilla/css/stacked_layers.html (#15988)
FAIL [expected PASS] /_mozilla/mozilla/sslfail.html (#10760)
OK /content-security-policy/frame-ancestors/frame-ancestors-path-ignored.window.html (#36468)
- FAIL [expected PASS] subtest: A 'frame-ancestors' CSP directive with a URL that includes a path should be ignored.
```
assert_unreached: The IFrame should have been blocked (or cross-origin). It wasn't. Reached unreachable code
```
OK /css/compositing/canvas-composite-modes.html (#37283)
- PASS [expected FAIL] subtest: globalCompositeOperation source-out
- PASS [expected FAIL] subtest: globalCompositeOperation destination-out
- FAIL [expected PASS] subtest: globalCompositeOperation source-atop
```
assert_approx_equals: green: g=47 a=47 source-atop g=47 a=47 expected 46.11702127659574 +/- 6.212765957446809 but got 53
```
- FAIL [expected PASS] subtest: globalCompositeOperation destination-atop
```
assert_approx_equals: green: g=47 a=47 destination-atop g=47 a=47 expected 46.11702127659574 +/- 6.212765957446809 but got 53
```
PASS [expected FAIL] /css/css-fonts/downloadable-font-scoped-to-document.html (#38691)
TIMEOUT [expected FAIL] /dom/xslt/large-cdata.html (#38029)
OK /fetch/metadata/generated/css-font-face.sub.tentative.html (#34624)
- PASS [expected FAIL] subtest: sec-fetch-storage-access - Not sent to non-trustworthy same-site destination
- PASS [expected FAIL] subtest: sec-fetch-storage-access - Not sent to non-trustworthy cross-site destination
TIMEOUT /fetch/metadata/generated/css-images.sub.tentative.html (#29047)
- TIMEOUT [expected PASS] subtest: background-image sec-fetch-site - Not sent to non-trustworthy same-origin destination
```
Test timed out
```
OK /html/browsers/browsing-the-web/navigating-across-documents/initial-empty-document/iframe-src-aboutblank-navigate-immediately.html (#29048)
- FAIL [expected PASS] subtest: Navigating to a different document with form submission
```
assert_equals: expected "http://web-platform.test:8000/common/blank.html?1=" but got "about:blank"
```
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_5.html (#21383)
- FAIL [expected PASS] subtest: Multiple history traversals, last would be aborted
```
assert_array_equals: Pages opened during history navigation expected property 1 to be 5 but got 3 (expected array [6, 5] got [6, 3])
```
OK /html/browsers/windows/embedded-opener-remove-frame.html (#23867)
- PASS [expected FAIL] subtest: opener of discarded auxiliary browsing context
OK [expected TIMEOUT] /html/interaction/focus/the-autofocus-attribute/document-with-fragment-empty.html (#28259)
- FAIL [expected TIMEOUT] subtest: Autofocus elements in top-level browsing context's documents with empty fragments should work.
```
assert_not_equals: got disallowed value Element node &lt;body&gt;&lt;/body&gt;
```
TIMEOUT /html/interaction/focus/the-autofocus-attribute/supported-elements.html (#24145)
- FAIL [expected NOTRUN] subtest: Host element with delegatesFocus should support autofocus
```
assert_equals: expected Element node &lt;div autofocus=""&gt;&lt;/div&gt; but got Element node &lt;body&gt;&lt;div autofocus=""&gt;&lt;/div&gt;&lt;/body&gt;
```
- TIMEOUT [expected NOTRUN] subtest: Host element with delegatesFocus including no focusable descendants should be skipped
```
Test timed out
```
CRASH [expected OK] /html/semantics/embedded-content/the-iframe-element/iframe_sandbox_popups_nonescaping-1.html (#24066)
OK [expected TIMEOUT] /html/semantics/embedded-content/the-iframe-element/iframe_sandbox_popups_nonescaping-3.html (#24066)
OK /html/semantics/forms/form-submission-0/multipart-formdata.window.html (#28725)
- PASS [expected FAIL] subtest: multipart/form-data: Basic test (normal form)
OK [expected CRASH] /html/semantics/forms/the-fieldset-element/disabled-003.html (#31730)
OK /html/semantics/scripting-1/the-script-element/execution-timing/077.html (#22139)
- PASS [expected FAIL] subtest: adding several types of scripts through the DOM and removing some of them confuses scheduler
OK /navigation-timing/test-navigation-type-reload.html (#33334)
- FAIL [expected PASS] subtest: Reload domComplete > Original domComplete
```
assert_true: Reload domComplete &gt; Original domComplete expected true got false
```
- FAIL [expected PASS] subtest: Reload loadEventEnd > Original loadEventEnd
```
assert_true: Reload loadEventEnd &gt; Original loadEventEnd expected true got false
```
- FAIL [expected PASS] subtest: Reload loadEventStart > Original loadEventStart
```
assert_true: Reload loadEventStart &gt; Original loadEventStart expected true got false
```

OK /preload/preload-error.sub.html (#37177)

FAIL [expected PASS] subtest: CORS (xhr): main

assert_greater_than: http://not-web-platform.test:8000/preload/resources/dummy.xml?pipe=header%28Access-Control-Allow-Origin%2C*%29&amp;label=xhr should be loaded expected a number greater than 0 but got 0

PASS [expected FAIL] subtest: Decode-error (style): main

TIMEOUT [expected CRASH] /trusted-types/trusted-types-navigation.html?06-10 (#37920)
- PASS [expected FAIL] subtest: Navigate a frame via anchor with javascript:-urls w/ default policy in enforcing mode.
- TIMEOUT [expected FAIL] subtest: Navigate a frame via anchor with javascript:-urls w/ default policy in report-only mode.
```
Test timed out
```
- NOTRUN [expected FAIL] subtest: Navigate a window via anchor with javascript:-urls w/ a default policy throwing an exception in enforcing mode.
- NOTRUN [expected FAIL] subtest: Navigate a window via anchor with javascript:-urls w/ a default policy throwing an exception in report-only mode.
OK /trusted-types/trusted-types-navigation.html?21-25 (#38997)
- PASS [expected FAIL] subtest: Navigate a window via form-submission with javascript:-urls in enforcing mode.
OK /trusted-types/trusted-types-navigation.html?26-30 (#38807)
- PASS [expected FAIL] subtest: Navigate a window via form-submission with javascript:-urls w/ default policy in enforcing mode.
- PASS [expected FAIL] subtest: Navigate a window via form-submission with javascript:-urls in report-only mode.
- PASS [expected FAIL] subtest: Navigate a window via form-submission with javascript:-urls w/ default policy in report-only mode.
- PASS [expected FAIL] subtest: Navigate a frame via form-submission with javascript:-urls in enforcing mode.
- PASS [expected FAIL] subtest: Navigate a frame via form-submission with javascript:-urls w/ default policy in enforcing mode.
TIMEOUT [expected CRASH] /trusted-types/trusted-types-navigation.html?31-35 (#38034)
- PASS [expected FAIL] subtest: Navigate a frame via form-submission with javascript:-urls in report-only mode.

Stable unexpected results (1)

OK /html/webappapis/scripting/events/event-handler-removal.window.html
- PASS [expected FAIL] subtest: Event handler set through content attribute should be deactivated when the content attribute is removed.

github-actions · 2025-08-30T02:14:16Z

⚠️ Try run (#17337838219) failed.

kotx · 2025-08-30T02:20:26Z

Looks like it passes! Fixing the lint (and WPT) now.

Signed-off-by: Kot <kot@kot.pink>

TimvdLippe

Nice!

jdm

Thanks for digging into this!

kotx requested a review from gterzian as a code owner August 29, 2025 04:45

Improve remove_event_listener matching

27657f9

Signed-off-by: Kot <kot@kot.pink>

kotx force-pushed the remove-event-listener-matches branch from 81ac758 to 27657f9 Compare August 29, 2025 05:04

TimvdLippe reviewed Aug 29, 2025

View reviewed changes

yezhizhen changed the title ~~Improve remove_event_listener matching (#38742)~~ script: Improve remove_event_listener matching Aug 29, 2025

Fix content attribute removal for event handlers

2bc0b28

Signed-off-by: Kot <kot@kot.pink>

jdm added the T-linux-wpt Do a try run of the WPT label Aug 30, 2025

github-actions bot removed the T-linux-wpt Do a try run of the WPT label Aug 30, 2025

Fix lints, WPT pass

8b93c2e

Signed-off-by: Kot <kot@kot.pink>

TimvdLippe approved these changes Aug 30, 2025

View reviewed changes

jdm approved these changes Aug 30, 2025

View reviewed changes

jdm changed the title ~~script: Improve remove_event_listener matching~~ script: Clear all associated event listeners when removing an event listener content attribute. Aug 30, 2025

jdm enabled auto-merge August 30, 2025 05:03

jdm added this pull request to the merge queue Aug 30, 2025

Merged via the queue into servo:main with commit 2e1b2e7 Aug 30, 2025
23 checks passed

kotx deleted the remove-event-listener-matches branch August 30, 2025 12:52

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

script: Clear all associated event listeners when removing an event listener content attribute. #39011

script: Clear all associated event listeners when removing an event listener content attribute. #39011

Uh oh!

kotx commented Aug 29, 2025 •

edited

Loading

Uh oh!

TimvdLippe Aug 29, 2025

Uh oh!

kotx Aug 30, 2025 •

edited

Loading

Uh oh!

github-actions bot commented Aug 30, 2025

Uh oh!

github-actions bot commented Aug 30, 2025

Uh oh!

github-actions bot commented Aug 30, 2025

Uh oh!

kotx commented Aug 30, 2025 •

edited

Loading

Uh oh!

TimvdLippe left a comment

Uh oh!

jdm left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

script: Clear all associated event listeners when removing an event listener content attribute. #39011

script: Clear all associated event listeners when removing an event listener content attribute. #39011

Uh oh!

Conversation

kotx commented Aug 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

TimvdLippe Aug 29, 2025

Choose a reason for hiding this comment

Uh oh!

kotx Aug 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Aug 30, 2025

Uh oh!

github-actions bot commented Aug 30, 2025

Uh oh!

github-actions bot commented Aug 30, 2025

Uh oh!

kotx commented Aug 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

TimvdLippe left a comment

Choose a reason for hiding this comment

Uh oh!

jdm left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

kotx commented Aug 29, 2025 •

edited

Loading

kotx Aug 30, 2025 •

edited

Loading

kotx commented Aug 30, 2025 •

edited

Loading