From 5fe54b800334d0963cbf52496e2cbce42d839c46 Mon Sep 17 00:00:00 2001 From: Utsav Oza Date: Sun, 26 Jul 2020 18:36:30 +0530 Subject: [PATCH] Fix ordering of mousemove event --- components/compositing/compositor.rs | 2 +- components/script/dom/document.rs | 160 +++++++++++++-------------- components/script_traits/lib.rs | 2 +- 3 files changed, 80 insertions(+), 84 deletions(-) diff --git a/components/compositing/compositor.rs b/components/compositing/compositor.rs index ba5002a83dd..49b33f1d7de 100644 --- a/components/compositing/compositor.rs +++ b/components/compositing/compositor.rs @@ -940,7 +940,7 @@ impl IOCompositor { let results = self.hit_test_at_point(cursor); if let Some(item) = results.items.first() { let node_address = Some(UntrustedNodeAddress(item.tag.0 as *const c_void)); - let event = MouseMoveEvent(Some(item.point_in_viewport.to_untyped()), node_address, 0); + let event = MouseMoveEvent(item.point_in_viewport.to_untyped(), node_address, 0); let pipeline_id = PipelineId::from_webrender(item.pipeline); let msg = ConstellationMsg::ForwardEvent(pipeline_id, event); if let Err(e) = self.constellation_chan.send(msg) { diff --git a/components/script/dom/document.rs b/components/script/dom/document.rs index 8244f4af31f..cb4d9160cac 100644 --- a/components/script/dom/document.rs +++ b/components/script/dom/document.rs @@ -1376,21 +1376,11 @@ impl Document { pub fn handle_mouse_move_event( &self, js_runtime: *mut JSRuntime, - client_point: Option>, + client_point: Point2D, prev_mouse_over_target: &MutNullableDom, node_address: Option, pressed_mouse_buttons: u16, ) { - let client_point = match client_point { - None => { - // If there's no point, there's no target under the mouse - // FIXME: dispatch mouseout here. We have no point. - prev_mouse_over_target.set(None); - return; - }, - Some(client_point) => client_point, - }; - let maybe_new_target = node_address.and_then(|address| { let node = unsafe { node::from_untrusted_node_address(js_runtime, address) }; node.inclusive_ancestors(ShadowIncluding::No) @@ -1398,13 +1388,82 @@ impl Document { .next() }); - // Send mousemove event to topmost target, unless it's an iframe, in which case the - // compositor should have also sent an event to the inner document. let new_target = match maybe_new_target { Some(ref target) => target, None => return, }; + let target_has_changed = prev_mouse_over_target + .get() + .as_ref() + .map_or(true, |old_target| old_target != new_target); + + // Here we know the target has changed, so we must update the state, + // dispatch mouseout to the previous one, mouseover to the new one, + if target_has_changed { + // Dispatch mouseout to previous target + if let Some(old_target) = prev_mouse_over_target.get() { + let old_target_is_ancestor_of_new_target = prev_mouse_over_target + .get() + .as_ref() + .map_or(false, |old_target| { + old_target + .upcast::() + .is_ancestor_of(new_target.upcast::()) + }); + + // If the old target is an ancestor of the new target, this can be skipped + // completely, since the node's hover state will be reseted below. + if !old_target_is_ancestor_of_new_target { + for element in old_target + .upcast::() + .inclusive_ancestors(ShadowIncluding::No) + .filter_map(DomRoot::downcast::) + { + element.set_hover_state(false); + element.set_active_state(false); + } + } + + // Remove hover state to old target and its parents + self.fire_mouse_event( + client_point, + old_target.upcast(), + FireMouseEventType::Out, + pressed_mouse_buttons, + ); + + // TODO: Fire mouseleave here only if the old target is + // not an ancestor of the new target. + } + + // Dispatch mouseover to new target - TODO: Redundant check? + if let Some(new_target) = maybe_new_target.as_ref() { + for element in new_target + .upcast::() + .inclusive_ancestors(ShadowIncluding::No) + .filter_map(DomRoot::downcast::) + { + if element.hover_state() { + break; + } + + element.set_hover_state(true); + } + + self.fire_mouse_event( + client_point, + &new_target.upcast(), + FireMouseEventType::Over, + pressed_mouse_buttons, + ); + + // TODO: Fire mouseenter here. + } + } + + // Send mousemove event to topmost target, unless it's an iframe, in which case the + // compositor should have also sent an event to the inner document. self.fire_mouse_event( client_point, new_target.upcast(), @@ -1412,76 +1471,13 @@ impl Document { pressed_mouse_buttons, ); - // Nothing more to do here, mousemove is sent, - // and the element under the mouse hasn't changed. - if maybe_new_target == prev_mouse_over_target.get() { - return; + if target_has_changed { + // Store the current mouse over target for next frame. + prev_mouse_over_target.set(maybe_new_target.as_deref()); + + self.window + .reflow(ReflowGoal::Full, ReflowReason::MouseEvent); } - - let old_target_is_ancestor_of_new_target = - match (prev_mouse_over_target.get(), maybe_new_target.as_ref()) { - (Some(old_target), Some(new_target)) => old_target - .upcast::() - .is_ancestor_of(new_target.upcast::()), - _ => false, - }; - - // Here we know the target has changed, so we must update the state, - // dispatch mouseout to the previous one, mouseover to the new one, - if let Some(old_target) = prev_mouse_over_target.get() { - // If the old target is an ancestor of the new target, this can be skipped - // completely, since the node's hover state will be reseted below. - if !old_target_is_ancestor_of_new_target { - for element in old_target - .upcast::() - .inclusive_ancestors(ShadowIncluding::No) - .filter_map(DomRoot::downcast::) - { - element.set_hover_state(false); - element.set_active_state(false); - } - } - - // Remove hover state to old target and its parents - self.fire_mouse_event( - client_point, - old_target.upcast(), - FireMouseEventType::Out, - pressed_mouse_buttons, - ); - - // TODO: Fire mouseleave here only if the old target is - // not an ancestor of the new target. - } - - if let Some(ref new_target) = maybe_new_target { - for element in new_target - .upcast::() - .inclusive_ancestors(ShadowIncluding::No) - .filter_map(DomRoot::downcast::) - { - if element.hover_state() { - break; - } - - element.set_hover_state(true); - } - - self.fire_mouse_event( - client_point, - &new_target.upcast(), - FireMouseEventType::Over, - pressed_mouse_buttons, - ); - - // TODO: Fire mouseenter here. - } - - // Store the current mouse over target for next frame. - prev_mouse_over_target.set(maybe_new_target.as_deref()); - - self.window - .reflow(ReflowGoal::Full, ReflowReason::MouseEvent); } #[allow(unsafe_code)] diff --git a/components/script_traits/lib.rs b/components/script_traits/lib.rs index 13dc9c32fa3..1087e37a9fc 100644 --- a/components/script_traits/lib.rs +++ b/components/script_traits/lib.rs @@ -552,7 +552,7 @@ pub enum CompositorEvent { ), /// The mouse was moved over a point (or was moved out of the recognizable region). MouseMoveEvent( - Option>, + Point2D, Option, // Bitmask of MouseButton values representing the currently pressed buttons u16,