From f334a56b0755f31866dc2cafd60934d07e7e8baf Mon Sep 17 00:00:00 2001 From: Tim van der Lippe Date: Fri, 22 Aug 2025 21:43:04 +0200 Subject: [PATCH] Remove event handlers when attribute is removed (#38734) We wouldn't handle the AttributeMutation::Removed for attribute event listeners and wouldn't remove the corresponding event listener. Added the necessary logic (using the newly EventTarget::is_content_event_handler to correctly only do this for known event handlers) and added links to the relevant parts of the spec. Signed-off-by: Tim van der Lippe --- components/script/dom/eventtarget.rs | 6 ++- components/script/dom/htmlelement.rs | 37 ++++++++++++++----- components/script/dom/macros.rs | 4 +- .../webidls/EventHandler.webidl | 4 +- .../css/css-animations/idlharness.html.ini | 18 --------- .../webkit-animation-start-event.html.ini | 6 --- ...n-content-document-idl-attributes.html.ini | 18 --------- 7 files changed, 38 insertions(+), 55 deletions(-) delete mode 100644 tests/wpt/meta/html/webappapis/scripting/events/event-handler-non-content-document-idl-attributes.html.ini diff --git a/components/script/dom/eventtarget.rs b/components/script/dom/eventtarget.rs index 8a614c269a3..f0c0b7d5fee 100644 --- a/components/script/dom/eventtarget.rs +++ b/components/script/dom/eventtarget.rs @@ -71,7 +71,7 @@ use crate::script_runtime::CanGc; /// /// containing the values from /// -static CONTENT_EVENT_HANDLER_NAMES: [&str; 83] = [ +static CONTENT_EVENT_HANDLER_NAMES: [&str; 85] = [ "onabort", "onauxclick", "onbeforeinput", @@ -149,8 +149,10 @@ static CONTENT_EVENT_HANDLER_NAMES: [&str; 83] = [ "onwebkittransitionend", "onwheel", // https://drafts.csswg.org/css-animations/#interface-globaleventhandlers-idl - "onanimationend", + "onanimationstart", "onanimationiteration", + "onanimationend", + "onanimationcancel", // https://drafts.csswg.org/css-transitions/#interface-globaleventhandlers-idl "ontransitionrun", "ontransitionend", diff --git a/components/script/dom/htmlelement.rs b/components/script/dom/htmlelement.rs index 2af4b278814..233e62fe53a 100644 --- a/components/script/dom/htmlelement.rs +++ b/components/script/dom/htmlelement.rs @@ -20,6 +20,7 @@ use crate::dom::bindings::codegen::Bindings::CharacterDataBinding::CharacterData use crate::dom::bindings::codegen::Bindings::EventHandlerBinding::{ EventHandlerNonNull, OnErrorEventHandlerNonNull, }; +use crate::dom::bindings::codegen::Bindings::EventTargetBinding::EventListenerOptions; use crate::dom::bindings::codegen::Bindings::HTMLElementBinding::HTMLElementMethods; use crate::dom::bindings::codegen::Bindings::HTMLLabelElementBinding::HTMLLabelElementMethods; use crate::dom::bindings::codegen::Bindings::HTMLOrSVGElementBinding::FocusOptions; @@ -1110,17 +1111,35 @@ impl VirtualMethods for HTMLElement { .attribute_mutated(attr, mutation, can_gc); let element = self.as_element(); match (attr.local_name(), mutation) { - (name, AttributeMutation::Set(_)) if name.starts_with("on") => { - let source = &**attr.value(); + // https://html.spec.whatwg.org/multipage/#event-handler-attributes:event-handler-content-attributes-3 + (name, mutation) + if name.starts_with("on") && EventTarget::is_content_event_handler(name) => + { let evtarget = self.upcast::(); - let source_line = 1; // TODO(#9604) get current JS execution line - evtarget.set_event_handler_uncompiled( - self.owner_window().get_url(), - source_line, - &name[2..], - source, - ); + let event_name = &name[2..]; + match mutation { + // https://html.spec.whatwg.org/multipage/#activate-an-event-handler + AttributeMutation::Set(_) => { + let source = &**attr.value(); + let source_line = 1; // TODO(#9604) get current JS execution line + evtarget.set_event_handler_uncompiled( + self.owner_window().get_url(), + source_line, + event_name, + source, + ); + }, + // https://html.spec.whatwg.org/multipage/#deactivate-an-event-handler + AttributeMutation::Removed => { + evtarget.remove_event_listener( + event_name.into(), + evtarget.get_event_handler_common(event_name, can_gc), + EventListenerOptions { capture: false }, + ); + }, + } }, + (&local_name!("form"), mutation) if self.is_form_associated_custom_element() => { self.form_attribute_mutated(mutation, can_gc); }, diff --git a/components/script/dom/macros.rs b/components/script/dom/macros.rs index 564fe810db0..d9e2ef62824 100644 --- a/components/script/dom/macros.rs +++ b/components/script/dom/macros.rs @@ -524,8 +524,10 @@ macro_rules! global_event_handlers( (NoOnload) => ( event_handler!(abort, GetOnabort, SetOnabort); event_handler!(auxclick, GetOnauxclick, SetOnauxclick); - event_handler!(animationend, GetOnanimationend, SetOnanimationend); + event_handler!(animationstart, GetOnanimationstart, SetOnanimationstart); event_handler!(animationiteration, GetOnanimationiteration, SetOnanimationiteration); + event_handler!(animationend, GetOnanimationend, SetOnanimationend); + event_handler!(animationcancel, GetOnanimationcancel, SetOnanimationcancel); event_handler!(beforeinput, GetOnbeforeinput, SetOnbeforeinput); event_handler!(beforematch, GetOnbeforematch, SetOnbeforematch); event_handler!(beforetoggle, GetOnbeforetoggle, SetOnbeforetoggle); diff --git a/components/script_bindings/webidls/EventHandler.webidl b/components/script_bindings/webidls/EventHandler.webidl index d32302f4b37..b49a29fa40f 100644 --- a/components/script_bindings/webidls/EventHandler.webidl +++ b/components/script_bindings/webidls/EventHandler.webidl @@ -107,8 +107,10 @@ interface mixin GlobalEventHandlers { // https://drafts.csswg.org/css-animations/#interface-globaleventhandlers-idl partial interface mixin GlobalEventHandlers { - attribute EventHandler onanimationend; + attribute EventHandler onanimationstart; attribute EventHandler onanimationiteration; + attribute EventHandler onanimationend; + attribute EventHandler onanimationcancel; }; // https://drafts.csswg.org/css-transitions/#interface-globaleventhandlers-idl diff --git a/tests/wpt/meta/css/css-animations/idlharness.html.ini b/tests/wpt/meta/css/css-animations/idlharness.html.ini index 9f40b8cca86..05cf7afd20a 100644 --- a/tests/wpt/meta/css/css-animations/idlharness.html.ini +++ b/tests/wpt/meta/css/css-animations/idlharness.html.ini @@ -1,31 +1,13 @@ [idlharness.html] - [HTMLElement interface: attribute onanimationstart] - expected: FAIL - [CSSKeyframeRule interface: keyframes.cssRules[0\] must inherit property "keyText" with the proper type] expected: FAIL - [Window interface: attribute onanimationcancel] - expected: FAIL - - [Document interface: attribute onanimationstart] - expected: FAIL - [CSSKeyframeRule interface: attribute keyText] expected: FAIL - [Window interface: attribute onanimationstart] - expected: FAIL - - [Document interface: attribute onanimationcancel] - expected: FAIL - [CSSKeyframeRule interface: attribute style] expected: FAIL - [HTMLElement interface: attribute onanimationcancel] - expected: FAIL - [CSSKeyframesRule interface: attribute length] expected: FAIL diff --git a/tests/wpt/meta/dom/events/webkit-animation-start-event.html.ini b/tests/wpt/meta/dom/events/webkit-animation-start-event.html.ini index 6e3e5febb82..cf357739b57 100644 --- a/tests/wpt/meta/dom/events/webkit-animation-start-event.html.ini +++ b/tests/wpt/meta/dom/events/webkit-animation-start-event.html.ini @@ -1,14 +1,8 @@ [webkit-animation-start-event.html] expected: TIMEOUT - [onanimationstart and onwebkitanimationstart are not aliases] - expected: FAIL - [dispatchEvent of a webkitAnimationStart event does trigger a prefixed event handler or listener] expected: FAIL - [dispatchEvent of a webkitAnimationStart event does not trigger an unprefixed event handler or listener] - expected: FAIL - [onwebkitanimationstart event handler should trigger for an animation] expected: TIMEOUT diff --git a/tests/wpt/meta/html/webappapis/scripting/events/event-handler-non-content-document-idl-attributes.html.ini b/tests/wpt/meta/html/webappapis/scripting/events/event-handler-non-content-document-idl-attributes.html.ini deleted file mode 100644 index 8f82752f095..00000000000 --- a/tests/wpt/meta/html/webappapis/scripting/events/event-handler-non-content-document-idl-attributes.html.ini +++ /dev/null @@ -1,18 +0,0 @@ -[event-handler-non-content-document-idl-attributes.html] - [div.onreadystatechange is not an event handler content attribute] - expected: FAIL - - [div.onvisibilitychange is not an event handler content attribute] - expected: FAIL - - [body.onreadystatechange is not an event handler content attribute] - expected: FAIL - - [body.onvisibilitychange is not an event handler content attribute] - expected: FAIL - - [frameset.onreadystatechange is not an event handler content attribute] - expected: FAIL - - [frameset.onvisibilitychange is not an event handler content attribute] - expected: FAIL