From 589d188a3f90852f5e8db6553614c92ee27874d0 Mon Sep 17 00:00:00 2001 From: Euclid Ye Date: Sat, 9 Aug 2025 16:04:31 +0800 Subject: [PATCH] script: Change signature of `Event::dispatch` to match the spec and simplify things (#38566) - [Dispatch Event](https://dom.spec.whatwg.org/#concept-event-dispatch) should return a Boolean. This function is used frequently in spec and the change makes things easier to follow. - Remove `enum EventStatus` and related functions. - Update some dead spec link. - Update some steps. This is intended as cleanup before working on #38435 and reduces binary size by 488KB in Release profile. Testing: No behaviour change. --------- Signed-off-by: Euclid Ye --- components/script/dom/debuggerglobalscope.rs | 4 +--- .../script/dom/dedicatedworkerglobalscope.rs | 6 ++--- components/script/dom/document.rs | 24 +++++++------------ components/script/dom/event.rs | 22 ++++------------- components/script/dom/eventtarget.rs | 9 +++---- components/script/dom/globalscope.rs | 20 +++++++++------- components/script/dom/htmlscriptelement.rs | 4 ++-- components/script/dom/window.rs | 10 +++----- components/script/script_runtime.rs | 9 +++---- 9 files changed, 40 insertions(+), 68 deletions(-) diff --git a/components/script/dom/debuggerglobalscope.rs b/components/script/dom/debuggerglobalscope.rs index 06e3de7ac63..0429b7593db 100644 --- a/components/script/dom/debuggerglobalscope.rs +++ b/components/script/dom/debuggerglobalscope.rs @@ -24,7 +24,6 @@ use crate::dom::bindings::inheritance::Castable; use crate::dom::bindings::root::DomRoot; use crate::dom::bindings::trace::CustomTraceable; use crate::dom::bindings::utils::define_all_exposed_interfaces; -use crate::dom::event::EventStatus; use crate::dom::globalscope::GlobalScope; use crate::dom::types::{DebuggerEvent, Event}; #[cfg(feature = "testbinding")] @@ -124,10 +123,9 @@ impl DebuggerGlobalScope { } pub(crate) fn fire_add_debuggee(&self, can_gc: CanGc, global: &GlobalScope) { - assert_eq!( + assert!( DomRoot::upcast::(DebuggerEvent::new(self.upcast(), global, can_gc)) .fire(self.upcast(), can_gc), - EventStatus::NotCanceled, "Guaranteed by DebuggerEvent::new" ); } diff --git a/components/script/dom/dedicatedworkerglobalscope.rs b/components/script/dom/dedicatedworkerglobalscope.rs index b96e3174e46..671da3c2c48 100644 --- a/components/script/dom/dedicatedworkerglobalscope.rs +++ b/components/script/dom/dedicatedworkerglobalscope.rs @@ -46,7 +46,7 @@ use crate::dom::bindings::trace::{CustomTraceable, RootedTraceableBox}; use crate::dom::bindings::utils::define_all_exposed_interfaces; use crate::dom::csp::{Violation, parse_csp_list_from_metadata}; use crate::dom::errorevent::ErrorEvent; -use crate::dom::event::{Event, EventBubbles, EventCancelable, EventStatus}; +use crate::dom::event::{Event, EventBubbles, EventCancelable}; use crate::dom::eventtarget::EventTarget; use crate::dom::globalscope::GlobalScope; use crate::dom::messageevent::MessageEvent; @@ -737,11 +737,9 @@ impl DedicatedWorkerGlobalScope { HandleValue::null(), CanGc::note(), ); - let event_status = - event.upcast::().fire(worker.upcast::(), CanGc::note()); // Step 2. - if event_status == EventStatus::NotCanceled { + if event.upcast::().fire(worker.upcast::(), CanGc::note()) { global.report_an_error(error_info, HandleValue::null(), CanGc::note()); } })); diff --git a/components/script/dom/document.rs b/components/script/dom/document.rs index 9de12104e01..3e13a2260f4 100644 --- a/components/script/dom/document.rs +++ b/components/script/dom/document.rs @@ -151,7 +151,7 @@ use crate::dom::element::{ CustomElementCreationMode, Element, ElementCreator, ElementPerformFullscreenEnter, ElementPerformFullscreenExit, }; -use crate::dom::event::{Event, EventBubbles, EventCancelable, EventDefault, EventStatus}; +use crate::dom::event::{Event, EventBubbles, EventCancelable, EventDefault}; use crate::dom::eventtarget::EventTarget; use crate::dom::focusevent::FocusEvent; use crate::dom::fontfaceset::FontFaceSet; @@ -1678,11 +1678,12 @@ impl Document { vec![], // predicted_events can_gc, ); - let event = menu_event.upcast::(); - event.fire(target, can_gc); - // if the event was not canceled, notify the embedder to show the context menu - if event.status() == EventStatus::NotCanceled { + // Step 3. Let result = dispatch menuevent at target. + let result = menu_event.upcast::().fire(target, can_gc); + + // Step 4. If result is true, then show the UA context menu + if result { let (sender, receiver) = ipc::channel::().expect("Failed to create IPC channel."); self.send_to_embedder(EmbedderMsg::ShowContextMenu( @@ -2393,13 +2394,8 @@ impl Document { false, can_gc, ); - let event = event.upcast::(); - let result = event.fire(&target, can_gc); - match result { - EventStatus::Canceled => TouchEventResult::Processed(false), - EventStatus::NotCanceled => TouchEventResult::Processed(true), - } + TouchEventResult::Processed(event.upcast::().fire(&target, can_gc)) } // If hittest fails, we still need to update the active point information. @@ -3026,8 +3022,7 @@ impl Document { ); let event = event.upcast::(); event.set_trusted(true); - let _ = self - .window + self.window .dispatch_event_with_target_override(event, can_gc); // Step 6 Update the visibility state of oldDocument to "hidden". self.update_visibility_state(DocumentVisibilityState::Hidden, can_gc); @@ -3044,8 +3039,7 @@ impl Document { event.set_trusted(true); let event_target = self.window.upcast::(); let has_listeners = event_target.has_listeners_for(&atom!("unload")); - let _ = self - .window + self.window .dispatch_event_with_target_override(&event, can_gc); self.fired_unload.set(true); // Step 9 diff --git a/components/script/dom/event.rs b/components/script/dom/event.rs index 6153866ce95..03844d810f7 100644 --- a/components/script/dom/event.rs +++ b/components/script/dom/event.rs @@ -279,7 +279,7 @@ impl Event { legacy_target_override: bool, can_gc: CanGc, // TODO legacy_did_output_listeners_throw_flag for indexeddb - ) -> EventStatus { + ) -> bool { let mut target = DomRoot::from_ref(target); // Step 1. Set event’s dispatch flag. @@ -629,15 +629,7 @@ impl Event { } // Step 12 Return false if event’s canceled flag is set; otherwise true. - self.status() - } - - pub(crate) fn status(&self) -> EventStatus { - if self.DefaultPrevented() { - EventStatus::Canceled - } else { - EventStatus::NotCanceled - } + !self.DefaultPrevented() } #[inline] @@ -673,8 +665,8 @@ impl Event { self.composed.set(composed); } - /// - pub(crate) fn fire(&self, target: &EventTarget, can_gc: CanGc) -> EventStatus { + /// + pub(crate) fn fire(&self, target: &EventTarget, can_gc: CanGc) -> bool { self.set_trusted(true); target.dispatch_event(self, can_gc) } @@ -1105,12 +1097,6 @@ pub(crate) enum EventDefault { Handled, } -#[derive(Debug, PartialEq)] -pub(crate) enum EventStatus { - Canceled, - NotCanceled, -} - /// pub(crate) struct EventTask { pub(crate) target: Trusted, diff --git a/components/script/dom/eventtarget.rs b/components/script/dom/eventtarget.rs index 1ed2ccb4622..55e10e69a4c 100644 --- a/components/script/dom/eventtarget.rs +++ b/components/script/dom/eventtarget.rs @@ -57,7 +57,7 @@ use crate::dom::csp::{CspReporting, InlineCheckType}; use crate::dom::document::Document; use crate::dom::element::Element; use crate::dom::errorevent::ErrorEvent; -use crate::dom::event::{Event, EventBubbles, EventCancelable, EventComposed, EventStatus}; +use crate::dom::event::{Event, EventBubbles, EventCancelable, EventComposed}; use crate::dom::globalscope::GlobalScope; use crate::dom::htmlformelement::FormControlElementHelpers; use crate::dom::node::{Node, NodeTraits}; @@ -430,7 +430,7 @@ impl EventTarget { .map_or(EventListeners(vec![]), |listeners| listeners.clone()) } - pub(crate) fn dispatch_event(&self, event: &Event, can_gc: CanGc) -> EventStatus { + pub(crate) fn dispatch_event(&self, event: &Event, can_gc: CanGc) -> bool { event.dispatch(self, false, can_gc) } @@ -994,10 +994,7 @@ impl EventTargetMethods for EventTarget { return Err(Error::InvalidState); } event.set_trusted(false); - Ok(match self.dispatch_event(event, can_gc) { - EventStatus::Canceled => false, - EventStatus::NotCanceled => true, - }) + Ok(self.dispatch_event(event, can_gc)) } } diff --git a/components/script/dom/globalscope.rs b/components/script/dom/globalscope.rs index 77c11cd28a4..98a4a472f7c 100644 --- a/components/script/dom/globalscope.rs +++ b/components/script/dom/globalscope.rs @@ -100,7 +100,7 @@ use crate::dom::dedicatedworkerglobalscope::{ DedicatedWorkerControlMsg, DedicatedWorkerGlobalScope, }; use crate::dom::errorevent::ErrorEvent; -use crate::dom::event::{Event, EventBubbles, EventCancelable, EventStatus}; +use crate::dom::event::{Event, EventBubbles, EventCancelable}; use crate::dom::eventsource::EventSource; use crate::dom::eventtarget::EventTarget; use crate::dom::file::File; @@ -2645,15 +2645,18 @@ impl GlobalScope { /// pub(crate) fn report_an_error(&self, error_info: ErrorInfo, value: HandleValue, can_gc: CanGc) { - // Step 1. + // Step 6. Early return if global is in error reporting mode, if self.in_error_reporting_mode.get() { return; } - // Step 2. + // Step 6.1. Set global's in error reporting mode to true. self.in_error_reporting_mode.set(true); - // Steps 3-6. + // Step 6.2. Set notHandled to the result of firing an event named error at global, + // using ErrorEvent, with the cancelable attribute initialized to true, + // and additional attributes initialized according to errorInfo. + // FIXME(#13195): muted errors. let event = ErrorEvent::new( self, @@ -2668,16 +2671,15 @@ impl GlobalScope { can_gc, ); - // Step 7. - let event_status = event + let not_handled = event .upcast::() .fire(self.upcast::(), can_gc); - // Step 8. + // Step 6.3. Set global's in error reporting mode to false. self.in_error_reporting_mode.set(false); - // Step 9. - if event_status == EventStatus::NotCanceled { + // Step 7. + if not_handled { // https://html.spec.whatwg.org/multipage/#runtime-script-errors-2 if let Some(dedicated) = self.downcast::() { dedicated.forward_error_to_worker_object(error_info); diff --git a/components/script/dom/htmlscriptelement.rs b/components/script/dom/htmlscriptelement.rs index 677f07b4aa2..3de8bf36485 100644 --- a/components/script/dom/htmlscriptelement.rs +++ b/components/script/dom/htmlscriptelement.rs @@ -65,7 +65,7 @@ use crate::dom::element::{ referrer_policy_for_element, reflect_cross_origin_attribute, reflect_referrer_policy_attribute, set_cross_origin_attribute, }; -use crate::dom::event::{Event, EventBubbles, EventCancelable, EventStatus}; +use crate::dom::event::{Event, EventBubbles, EventCancelable}; use crate::dom::globalscope::GlobalScope; use crate::dom::htmlelement::HTMLElement; use crate::dom::node::{ChildrenMutation, CloneChildrenFlag, Node, NodeTraits}; @@ -1416,7 +1416,7 @@ impl HTMLScriptElement { bubbles: EventBubbles, cancelable: EventCancelable, can_gc: CanGc, - ) -> EventStatus { + ) -> bool { let window = self.owner_window(); let event = Event::new(window.upcast(), type_, bubbles, cancelable, can_gc); event.fire(self.upcast(), can_gc) diff --git a/components/script/dom/window.rs b/components/script/dom/window.rs index 148fb53c959..9a9bcdf9815 100644 --- a/components/script/dom/window.rs +++ b/components/script/dom/window.rs @@ -130,7 +130,7 @@ use crate::dom::cssstyledeclaration::{CSSModificationAccess, CSSStyleDeclaration use crate::dom::customelementregistry::CustomElementRegistry; use crate::dom::document::{AnimationFrameCallback, Document}; use crate::dom::element::Element; -use crate::dom::event::{Event, EventBubbles, EventCancelable, EventStatus}; +use crate::dom::event::{Event, EventBubbles, EventCancelable}; use crate::dom::eventtarget::EventTarget; use crate::dom::gamepad::{Gamepad, contains_user_gesture}; use crate::dom::gamepadevent::GamepadEventType; @@ -705,12 +705,8 @@ impl Window { } // see note at https://dom.spec.whatwg.org/#concept-event-dispatch step 2 - pub(crate) fn dispatch_event_with_target_override( - &self, - event: &Event, - can_gc: CanGc, - ) -> EventStatus { - event.dispatch(self.upcast(), true, can_gc) + pub(crate) fn dispatch_event_with_target_override(&self, event: &Event, can_gc: CanGc) { + event.dispatch(self.upcast(), true, can_gc); } pub(crate) fn font_context(&self) -> &Arc { diff --git a/components/script/script_runtime.rs b/components/script/script_runtime.rs index a40583aca09..457f02dbd6f 100644 --- a/components/script/script_runtime.rs +++ b/components/script/script_runtime.rs @@ -75,7 +75,7 @@ use crate::dom::bindings::root::trace_roots; use crate::dom::bindings::utils::DOM_CALLBACKS; use crate::dom::bindings::{principals, settings_stack}; use crate::dom::csp::CspReporting; -use crate::dom::event::{Event, EventBubbles, EventCancelable, EventStatus}; +use crate::dom::event::{Event, EventBubbles, EventCancelable}; use crate::dom::eventtarget::EventTarget; use crate::dom::globalscope::GlobalScope; use crate::dom::promise::Promise; @@ -559,10 +559,11 @@ pub(crate) fn notify_about_rejected_promises(global: &GlobalScope) { CanGc::note() ); - let event_status = event.upcast::().fire(&target, CanGc::note()); + let not_canceled = event.upcast::().fire(&target, CanGc::note()); - // Step 4-3. - if event_status == EventStatus::Canceled { + // Step 4-3. If notCanceled is true, then the user agent + // may report p.[[PromiseResult]] to a developer console. + if not_canceled { // TODO: The promise rejection is not handled; we need to add it back to the list. }