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 <tvanderlippe@gmail.com>

components/script/dom/eventtarget.rs

@@ -71,7 +71,7 @@ use crate::script_runtime::CanGc;
 /// <https://html.spec.whatwg.org/multipage/#event-handler-content-attributes>
 /// containing the values from
 /// <https://html.spec.whatwg.org/multipage/#globaleventhandlers>
-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",

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") => {
+            // https://html.spec.whatwg.org/multipage/#event-handler-attributes:event-handler-content-attributes-3
-                let source = &**attr.value();
+            (name, mutation)
+                if name.starts_with("on") && EventTarget::is_content_event_handler(name) =>
+            {
                 let evtarget = self.upcast::<EventTarget>();
-                let source_line = 1; // TODO(#9604) get current JS execution line
+                let event_name = &name[2..];
-                evtarget.set_event_handler_uncompiled(
+                match mutation {
-                    self.owner_window().get_url(),
+                    // https://html.spec.whatwg.org/multipage/#activate-an-event-handler
-                    source_line,
+                    AttributeMutation::Set(_) => {
-                    &name[2..],
+                        let source = &**attr.value();
-                    source,
+                        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);
             },

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);

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

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
 

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
 

tests/wpt/meta/html/webappapis/scripting/events/event-handler-non-content-document-idl-attributes.html.ini

@@ -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