From 590316a468a0ded9f2004943b9eadc96af6b095c Mon Sep 17 00:00:00 2001 From: Anthony Ramine Date: Mon, 29 Feb 2016 10:09:45 +0100 Subject: [PATCH 1/4] Merge EventTarget::{get_listeners, get_listeners_for} --- components/script/dom/eventdispatcher.rs | 6 +++--- components/script/dom/eventtarget.rs | 12 ++++-------- 2 files changed, 7 insertions(+), 11 deletions(-) diff --git a/components/script/dom/eventdispatcher.rs b/components/script/dom/eventdispatcher.rs index f80a217658b..966314062b5 100644 --- a/components/script/dom/eventdispatcher.rs +++ b/components/script/dom/eventdispatcher.rs @@ -67,7 +67,7 @@ fn dispatch_to_listeners(event: &Event, target: &EventTarget, chain: &[&EventTar /* capturing */ event.set_phase(EventPhase::Capturing); for cur_target in chain.iter().rev() { - if let Some(listeners) = cur_target.get_listeners_for(&type_, ListenerPhase::Capturing) { + if let Some(listeners) = cur_target.get_listeners_for(&type_, Some(ListenerPhase::Capturing)) { event.set_current_target(cur_target); for listener in &listeners { handle_event(window.r(), listener, *cur_target, event); @@ -90,7 +90,7 @@ fn dispatch_to_listeners(event: &Event, target: &EventTarget, chain: &[&EventTar event.set_phase(EventPhase::AtTarget); event.set_current_target(target); - if let Some(listeners) = target.get_listeners(&type_) { + if let Some(listeners) = target.get_listeners_for(&type_, None) { for listener in listeners { handle_event(window.r(), &listener, target, event); @@ -113,7 +113,7 @@ fn dispatch_to_listeners(event: &Event, target: &EventTarget, chain: &[&EventTar event.set_phase(EventPhase::Bubbling); for cur_target in chain { - if let Some(listeners) = cur_target.get_listeners_for(&type_, ListenerPhase::Bubbling) { + if let Some(listeners) = cur_target.get_listeners_for(&type_, Some(ListenerPhase::Bubbling)) { event.set_current_target(cur_target); for listener in &listeners { handle_event(window.r(), listener, *cur_target, event); diff --git a/components/script/dom/eventtarget.rs b/components/script/dom/eventtarget.rs index aa49d9a110f..8c4df94a64d 100644 --- a/components/script/dom/eventtarget.rs +++ b/components/script/dom/eventtarget.rs @@ -301,16 +301,12 @@ impl EventTarget { } } - pub fn get_listeners(&self, type_: &Atom) -> Option> { - self.handlers.borrow_mut().get_mut(type_).map(|listeners| { - listeners.get_listeners(None, self, type_) - }) - } - - pub fn get_listeners_for(&self, type_: &Atom, desired_phase: ListenerPhase) + pub fn get_listeners_for(&self, + type_: &Atom, + desired_phase: Option) -> Option> { self.handlers.borrow_mut().get_mut(type_).map(|listeners| { - listeners.get_listeners(Some(desired_phase), self, type_) + listeners.get_listeners(desired_phase, self, type_) }) } From fc2cf31d5a61920701cb1a8f46c33f32ac2f4ace Mon Sep 17 00:00:00 2001 From: Anthony Ramine Date: Mon, 29 Feb 2016 10:15:55 +0100 Subject: [PATCH 2/4] Return a Vec from EventTarget::get_listeners_for --- components/script/dom/eventdispatcher.rs | 52 +++++++++++------------- components/script/dom/eventtarget.rs | 4 +- 2 files changed, 26 insertions(+), 30 deletions(-) diff --git a/components/script/dom/eventdispatcher.rs b/components/script/dom/eventdispatcher.rs index 966314062b5..072380281f7 100644 --- a/components/script/dom/eventdispatcher.rs +++ b/components/script/dom/eventdispatcher.rs @@ -67,20 +67,19 @@ fn dispatch_to_listeners(event: &Event, target: &EventTarget, chain: &[&EventTar /* capturing */ event.set_phase(EventPhase::Capturing); for cur_target in chain.iter().rev() { - if let Some(listeners) = cur_target.get_listeners_for(&type_, Some(ListenerPhase::Capturing)) { - event.set_current_target(cur_target); - for listener in &listeners { - handle_event(window.r(), listener, *cur_target, event); + let listeners = cur_target.get_listeners_for(&type_, Some(ListenerPhase::Capturing)); + event.set_current_target(cur_target); + for listener in &listeners { + handle_event(window.r(), listener, *cur_target, event); - if event.stop_immediate() { - return; - } - } - - if event.stop_propagation() { + if event.stop_immediate() { return; } } + + if event.stop_propagation() { + return; + } } assert!(!event.stop_propagation()); @@ -90,18 +89,16 @@ fn dispatch_to_listeners(event: &Event, target: &EventTarget, chain: &[&EventTar event.set_phase(EventPhase::AtTarget); event.set_current_target(target); - if let Some(listeners) = target.get_listeners_for(&type_, None) { - for listener in listeners { - handle_event(window.r(), &listener, target, event); + for listener in target.get_listeners_for(&type_, None) { + handle_event(window.r(), &listener, target, event); - if event.stop_immediate() { - return; - } - } - if event.stop_propagation() { + if event.stop_immediate() { return; } } + if event.stop_propagation() { + return; + } assert!(!event.stop_propagation()); assert!(!event.stop_immediate()); @@ -113,20 +110,19 @@ fn dispatch_to_listeners(event: &Event, target: &EventTarget, chain: &[&EventTar event.set_phase(EventPhase::Bubbling); for cur_target in chain { - if let Some(listeners) = cur_target.get_listeners_for(&type_, Some(ListenerPhase::Bubbling)) { - event.set_current_target(cur_target); - for listener in &listeners { - handle_event(window.r(), listener, *cur_target, event); + let listeners = cur_target.get_listeners_for(&type_, Some(ListenerPhase::Bubbling)); + event.set_current_target(cur_target); + for listener in &listeners { + handle_event(window.r(), listener, *cur_target, event); - if event.stop_immediate() { - return; - } - } - - if event.stop_propagation() { + if event.stop_immediate() { return; } } + + if event.stop_propagation() { + return; + } } } diff --git a/components/script/dom/eventtarget.rs b/components/script/dom/eventtarget.rs index 8c4df94a64d..21801e02062 100644 --- a/components/script/dom/eventtarget.rs +++ b/components/script/dom/eventtarget.rs @@ -304,8 +304,8 @@ impl EventTarget { pub fn get_listeners_for(&self, type_: &Atom, desired_phase: Option) - -> Option> { - self.handlers.borrow_mut().get_mut(type_).map(|listeners| { + -> Vec { + self.handlers.borrow_mut().get_mut(type_).map_or(vec![], |listeners| { listeners.get_listeners(desired_phase, self, type_) }) } From 4d2587d6c398198587af76e7d3fed6900f7364b0 Mon Sep 17 00:00:00 2001 From: Anthony Ramine Date: Mon, 29 Feb 2016 10:35:37 +0100 Subject: [PATCH 3/4] Rename various bindings in event dispatching --- components/script/dom/eventdispatcher.rs | 35 ++++++++++++------------ components/script/dom/eventtarget.rs | 4 +-- 2 files changed, 20 insertions(+), 19 deletions(-) diff --git a/components/script/dom/eventdispatcher.rs b/components/script/dom/eventdispatcher.rs index 072380281f7..07d92e9dad6 100644 --- a/components/script/dom/eventdispatcher.rs +++ b/components/script/dom/eventdispatcher.rs @@ -47,7 +47,7 @@ fn handle_event(window: Option<&Window>, listener: &CompiledEventListener, listener.call_or_handle_event(current_target, event, Report); } -fn dispatch_to_listeners(event: &Event, target: &EventTarget, chain: &[&EventTarget]) { +fn dispatch_to_listeners(event: &Event, target: &EventTarget, event_path: &[&EventTarget]) { assert!(!event.stop_propagation()); assert!(!event.stop_immediate()); @@ -66,11 +66,11 @@ fn dispatch_to_listeners(event: &Event, target: &EventTarget, chain: &[&EventTar /* capturing */ event.set_phase(EventPhase::Capturing); - for cur_target in chain.iter().rev() { - let listeners = cur_target.get_listeners_for(&type_, Some(ListenerPhase::Capturing)); - event.set_current_target(cur_target); + for object in event_path.iter().rev() { + let listeners = object.get_listeners_for(&type_, Some(ListenerPhase::Capturing)); + event.set_current_target(object); for listener in &listeners { - handle_event(window.r(), listener, *cur_target, event); + handle_event(window.r(), listener, *object, event); if event.stop_immediate() { return; @@ -109,11 +109,11 @@ fn dispatch_to_listeners(event: &Event, target: &EventTarget, chain: &[&EventTar } event.set_phase(EventPhase::Bubbling); - for cur_target in chain { - let listeners = cur_target.get_listeners_for(&type_, Some(ListenerPhase::Bubbling)); - event.set_current_target(cur_target); + for object in event_path { + let listeners = object.get_listeners_for(&type_, Some(ListenerPhase::Bubbling)); + event.set_current_target(object); for listener in &listeners { - handle_event(window.r(), listener, *cur_target, event); + handle_event(window.r(), listener, *object, event); if event.stop_immediate() { return; @@ -127,15 +127,16 @@ fn dispatch_to_listeners(event: &Event, target: &EventTarget, chain: &[&EventTar } // See https://dom.spec.whatwg.org/#concept-event-dispatch for the full dispatch algorithm -pub fn dispatch_event(target: &EventTarget, pseudo_target: Option<&EventTarget>, +pub fn dispatch_event(target: &EventTarget, + target_override: Option<&EventTarget>, event: &Event) -> bool { assert!(!event.dispatching()); assert!(event.initialized()); assert_eq!(event.phase(), EventPhase::None); assert!(event.GetCurrentTarget().is_none()); - event.set_target(match pseudo_target { - Some(pseudo_target) => pseudo_target, + event.set_target(match target_override { + Some(target_override) => target_override, None => target.clone(), }); @@ -145,21 +146,21 @@ pub fn dispatch_event(target: &EventTarget, pseudo_target: Option<&EventTarget>, event.set_dispatching(true); - let mut chain: RootedVec> = RootedVec::new(); + let mut event_path: RootedVec> = RootedVec::new(); if let Some(target_node) = target.downcast::() { for ancestor in target_node.ancestors() { - chain.push(JS::from_ref(ancestor.upcast())); + event_path.push(JS::from_ref(ancestor.upcast())); } let top_most_ancestor_or_target = - Root::from_ref(chain.r().last().cloned().unwrap_or(target)); + Root::from_ref(event_path.r().last().cloned().unwrap_or(target)); if let Some(document) = Root::downcast::(top_most_ancestor_or_target) { if event.type_() != atom!("load") && document.browsing_context().is_some() { - chain.push(JS::from_ref(document.window().upcast())); + event_path.push(JS::from_ref(document.window().upcast())); } } } - dispatch_to_listeners(event, target, chain.r()); + dispatch_to_listeners(event, target, event_path.r()); /* default action */ let target = event.GetTarget(); diff --git a/components/script/dom/eventtarget.rs b/components/script/dom/eventtarget.rs index 21801e02062..991fc1e9415 100644 --- a/components/script/dom/eventtarget.rs +++ b/components/script/dom/eventtarget.rs @@ -303,10 +303,10 @@ impl EventTarget { pub fn get_listeners_for(&self, type_: &Atom, - desired_phase: Option) + specific_phase: Option) -> Vec { self.handlers.borrow_mut().get_mut(type_).map_or(vec![], |listeners| { - listeners.get_listeners(desired_phase, self, type_) + listeners.get_listeners(specific_phase, self, type_) }) } From 587963c6e3f32116fc2bb05ff5b46debbd54bdfc Mon Sep 17 00:00:00 2001 From: Anthony Ramine Date: Mon, 29 Feb 2016 10:35:43 +0100 Subject: [PATCH 4/4] Refactor dispatch_event according to the spec (fixes #9178) Two new functions invoke and inner_invoke are introduced and some invariants documented. --- components/script/dom/eventdispatcher.rs | 125 +++++++++++++++-------- 1 file changed, 84 insertions(+), 41 deletions(-) diff --git a/components/script/dom/eventdispatcher.rs b/components/script/dom/eventdispatcher.rs index 07d92e9dad6..cef67d2d18c 100644 --- a/components/script/dom/eventdispatcher.rs +++ b/components/script/dom/eventdispatcher.rs @@ -47,6 +47,8 @@ fn handle_event(window: Option<&Window>, listener: &CompiledEventListener, listener.call_or_handle_event(current_target, event, Report); } +// See dispatch_event. +// https://dom.spec.whatwg.org/#concept-event-dispatch fn dispatch_to_listeners(event: &Event, target: &EventTarget, event_path: &[&EventTarget]) { assert!(!event.stop_propagation()); assert!(!event.stop_immediate()); @@ -62,71 +64,47 @@ fn dispatch_to_listeners(event: &Event, target: &EventTarget, event_path: &[&Eve _ => None, }; - let type_ = event.type_(); - - /* capturing */ + // Step 5. event.set_phase(EventPhase::Capturing); + + // Step 6. for object in event_path.iter().rev() { - let listeners = object.get_listeners_for(&type_, Some(ListenerPhase::Capturing)); - event.set_current_target(object); - for listener in &listeners { - handle_event(window.r(), listener, *object, event); - - if event.stop_immediate() { - return; - } - } - + invoke(window.r(), object, event, Some(ListenerPhase::Capturing)); if event.stop_propagation() { return; } } - assert!(!event.stop_propagation()); assert!(!event.stop_immediate()); - /* at target */ + // Step 7. event.set_phase(EventPhase::AtTarget); - event.set_current_target(target); - for listener in target.get_listeners_for(&type_, None) { - handle_event(window.r(), &listener, target, event); - - if event.stop_immediate() { - return; - } - } + // Step 8. + invoke(window.r(), target, event, None); if event.stop_propagation() { return; } - assert!(!event.stop_propagation()); assert!(!event.stop_immediate()); - /* bubbling */ if !event.bubbles() { return; } + // Step 9.1. event.set_phase(EventPhase::Bubbling); + + // Step 9.2. for object in event_path { - let listeners = object.get_listeners_for(&type_, Some(ListenerPhase::Bubbling)); - event.set_current_target(object); - for listener in &listeners { - handle_event(window.r(), listener, *object, event); - - if event.stop_immediate() { - return; - } - } - + invoke(window.r(), object, event, Some(ListenerPhase::Bubbling)); if event.stop_propagation() { return; } } } -// See https://dom.spec.whatwg.org/#concept-event-dispatch for the full dispatch algorithm +// https://dom.spec.whatwg.org/#concept-event-dispatch pub fn dispatch_event(target: &EventTarget, target_override: Option<&EventTarget>, event: &Event) -> bool { @@ -135,18 +113,24 @@ pub fn dispatch_event(target: &EventTarget, assert_eq!(event.phase(), EventPhase::None); assert!(event.GetCurrentTarget().is_none()); - event.set_target(match target_override { - Some(target_override) => target_override, - None => target.clone(), - }); + // Step 2. + event.set_target(target_override.unwrap_or(target)); if event.stop_propagation() { + // If the event's stop propagation flag is set, we can skip everything because + // it prevents the calls of the invoke algorithm in the spec and we asserted + // at the beginning that steps 10-12 don't need to be executed. return !event.DefaultPrevented(); } + // Step 1. Postponed here for the reason stated above. event.set_dispatching(true); + // Step 3. The "invoke" algorithm is only used on `target` separately, + // so we don't put it in the path. let mut event_path: RootedVec> = RootedVec::new(); + + // Step 4. if let Some(target_node) = target.downcast::() { for ancestor in target_node.ancestors() { event_path.push(JS::from_ref(ancestor.upcast())); @@ -160,9 +144,10 @@ pub fn dispatch_event(target: &EventTarget, } } + // Steps 5-9. In a separate function to short-circuit various things easily. dispatch_to_listeners(event, target, event_path.r()); - /* default action */ + // Default action. let target = event.GetTarget(); match target { Some(ref target) => { @@ -174,9 +159,67 @@ pub fn dispatch_event(target: &EventTarget, None => {} } + // Step 10. event.set_dispatching(false); + + // Step 11. event.set_phase(EventPhase::None); + + // Step 12. event.clear_current_target(); + // Step 13. !event.DefaultPrevented() } + +// https://dom.spec.whatwg.org/#concept-event-listener-invoke +fn invoke(window: Option<&Window>, + object: &EventTarget, + event: &Event, + specific_listener_phase: Option) { + // Step 1. + assert!(!event.stop_propagation()); + + // Steps 2-3. + let listeners = object.get_listeners_for(&event.type_(), specific_listener_phase); + + // Step 4. + event.set_current_target(object); + + // Step 5. + inner_invoke(window, object, event, &listeners); + + // TODO: step 6. +} + +// https://dom.spec.whatwg.org/#concept-event-listener-inner-invoke +fn inner_invoke(window: Option<&Window>, + object: &EventTarget, + event: &Event, + listeners: &[CompiledEventListener]) + -> bool { + // Step 1. + let mut found = false; + + // Step 2. + for listener in listeners { + // Steps 2.1 and 2.3-2.4 are not done because `listeners` contain only the + // relevant ones for this invoke call during the dispatch algorithm. + + // Step 2.2. + found = true; + + // TODO: step 2.5. + + // Step 2.6. + handle_event(window, listener, object, event); + if event.stop_immediate() { + return found; + } + + // TODO: step 2.7. + } + + // Step 3. + found +}