Fix ordering of mousemove event

components/compositing/compositor.rs

@@ -940,7 +940,7 @@ impl<Window: WindowMethods + ?Sized> IOCompositor<Window> {
         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) {

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<Point2D<f32>>,
+        client_point: Point2D<f32>,
         prev_mouse_over_target: &MutNullableDom<Element>,
         node_address: Option<UntrustedNodeAddress>,
         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,37 +1388,30 @@ 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,
         };
 
-        self.fire_mouse_event(
+        let target_has_changed = prev_mouse_over_target
-            client_point,
+            .get()
-            new_target.upcast(),
+            .as_ref()
-            FireMouseEventType::Move,
+            .map_or(true, |old_target| old_target != new_target);
-            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;
-        }
-
-        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::<Node>()
-                    .is_ancestor_of(new_target.upcast::<Node>()),
-                _ => 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 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::<Node>()
+                            .is_ancestor_of(new_target.upcast::<Node>())
+                    });
+
                 // 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 {
@@ -1454,7 +1437,8 @@ impl Document {
                 // not an ancestor of the new target.
             }
 
-        if let Some(ref new_target) = maybe_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::<Node>()
                     .inclusive_ancestors(ShadowIncluding::No)
@@ -1476,13 +1460,25 @@ impl Document {
 
                 // 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(),
+            FireMouseEventType::Move,
+            pressed_mouse_buttons,
+        );
+
+        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);
         }
+    }
 
     #[allow(unsafe_code)]
     pub fn handle_wheel_event(

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<f32>>,
+        Point2D<f32>,
         Option<UntrustedNodeAddress>,
         // Bitmask of MouseButton values representing the currently pressed buttons
         u16,