auto merge of #4959 : glennw/servo/fix-hover, r=jdm

Specifically:
 - Use inclusive_ancestors instead of ancestors, to detect hover on elements like divs.
 - Send the mousemove event after all the hover states have been set correctly.
 - Correctly handle removing hover state from elements when mouse is not over any elements.
 - Correctly detect when a reflow is required (previous code failed in several edge cases).

components/script/page.rs

@@ -436,23 +436,23 @@ impl Page {
         address
     }
 
-    pub fn get_nodes_under_mouse(&self, point: &Point2D<f32>) -> Option<Vec<UntrustedNodeAddress>> {
+    pub fn get_nodes_under_mouse(&self, point: &Point2D<f32>) -> Vec<UntrustedNodeAddress> {
+        let mut results = vec!();
         let frame = self.frame();
         let document = frame.as_ref().unwrap().document.root();
-        let root = match document.r().GetDocumentElement().root() {
+        match document.r().GetDocumentElement().root() {
-            None => return None,
+            Some(root) => {
-            Some(root) => root,
+                let root: JSRef<Node> = NodeCast::from_ref(root.r());
-        };
+                match self.layout().mouse_over(root.to_trusted_node_address(), *point) {
-        let root: JSRef<Node> = NodeCast::from_ref(root.r());
+                    Ok(MouseOverResponse(node_addresses)) => {
-        let address = match self.layout().mouse_over(root.to_trusted_node_address(), *point) {
+                        results = node_addresses;
-            Ok(MouseOverResponse(node_address)) => {
+                    }
-                Some(node_address)
+                    Err(()) => {}
+                };
             }
-            Err(()) => {
+            None => {}
-                None
+        }
-            }
+        results
-        };
-        address
     }
 }
 

components/script/script_task.rs

@@ -220,7 +220,7 @@ pub struct ScriptTask {
     /// The JSContext.
     js_context: DOMRefCell<Option<Rc<Cx>>>,
 
-    mouse_over_targets: DOMRefCell<Option<Vec<JS<Node>>>>
+    mouse_over_targets: DOMRefCell<Vec<JS<Node>>>
 }
 
 /// In the event of task failure, all data on the stack runs its destructor. However, there
@@ -383,7 +383,7 @@ impl ScriptTask {
 
             js_runtime: js_runtime,
             js_context: DOMRefCell::new(Some(js_context)),
-            mouse_over_targets: DOMRefCell::new(None)
+            mouse_over_targets: DOMRefCell::new(vec!())
         }
     }
 
@@ -1149,87 +1149,72 @@ impl ScriptTask {
 
     fn handle_mouse_move_event(&self, pipeline_id: PipelineId, point: Point2D<f32>) {
         let page = get_page(&*self.page.borrow(), pipeline_id);
-        match page.get_nodes_under_mouse(&point) {
+        let mut needs_reflow = false;
-            Some(node_address) => {
-                let mut target_list = vec!();
-                let mut target_compare = false;
 
-                let mouse_over_targets = &mut *self.mouse_over_targets.borrow_mut();
+        // Build a list of elements that are currently under the mouse.
-                match *mouse_over_targets {
+        let mouse_over_addresses = page.get_nodes_under_mouse(&point);
-                    Some(ref mut mouse_over_targets) => {
+        let mouse_over_targets: Vec<JS<Node>> = mouse_over_addresses.iter()
-                        for node in mouse_over_targets.iter_mut() {
+                                                                    .filter_map(|node_address| {
-                            let node = node.root();
+            let node = node::from_untrusted_node_address(self.js_runtime.ptr, *node_address);
-                            node.r().set_hover_state(false);
+            node.root().r().inclusive_ancestors().find(|node| node.is_element()).map(JS::from_rooted)
-                        }
+        }).collect();
-                    }
-                    None => {}
-                }
 
-                if node_address.len() > 0 {
+        // Remove hover from any elements in the previous list that are no longer
-                    let top_most_node =
+        // under the mouse.
-                        node::from_untrusted_node_address(self.js_runtime.ptr, node_address[0]).root();
+        let prev_mouse_over_targets = &mut *self.mouse_over_targets.borrow_mut();
-
+        for target in prev_mouse_over_targets.iter() {
-                    if let Some(ref frame) = *page.frame() {
+            if !mouse_over_targets.contains(target) {
-                        let window = frame.window.root();
+                target.root().r().set_hover_state(false);
-
+                needs_reflow = true;
-                        let x = point.x.to_i32().unwrap_or(0);
-                        let y = point.y.to_i32().unwrap_or(0);
-
-                        let mouse_event = MouseEvent::new(window.r(),
-                            "mousemove".to_owned(),
-                            true,
-                            true,
-                            Some(window.r()),
-                            0i32,
-                            x, y, x, y,
-                            false, false, false, false,
-                            0i16,
-                            None).root();
-
-                        let event: JSRef<Event> = EventCast::from_ref(mouse_event.r());
-                        let target: JSRef<EventTarget> = EventTargetCast::from_ref(top_most_node.r());
-                        event.fire(target);
-                    }
-                }
-
-                for node_address in node_address.iter() {
-                    let temp_node =
-                        node::from_untrusted_node_address(self.js_runtime.ptr, *node_address).root();
-
-                    let maybe_node = temp_node.r().ancestors().find(|node| node.is_element());
-                    match maybe_node {
-                        Some(node) => {
-                            node.set_hover_state(true);
-                            match *mouse_over_targets {
-                                Some(ref mouse_over_targets) if !target_compare => {
-                                    target_compare =
-                                        !mouse_over_targets.contains(&JS::from_rooted(node));
-                                }
-                                _ => {}
-                            }
-                            target_list.push(JS::from_rooted(node));
-                        }
-                        None => {}
-                    }
-                }
-                match *mouse_over_targets {
-                    Some(ref mouse_over_targets) => {
-                        if mouse_over_targets.len() != target_list.len() {
-                            target_compare = true
-                        }
-                    }
-                    None => target_compare = true,
-                }
-
-                if target_compare {
-                    if mouse_over_targets.is_some() {
-                        self.force_reflow(&*page)
-                    }
-                    *mouse_over_targets = Some(target_list);
-                }
             }
+        }
 
-            None => {}
+        // Set hover state for any elements in the current mouse over list.
+        // Check if any of them changed state to determine whether to
+        // force a reflow below.
+        for target in mouse_over_targets.iter() {
+            let target = target.root();
+            let target_ref = target.r();
+            if !target_ref.get_hover_state() {
+                target_ref.set_hover_state(true);
+                needs_reflow = true;
+            }
+        }
+
+        // Send mousemove event to topmost target
+        if mouse_over_addresses.len() > 0 {
+            let top_most_node =
+                node::from_untrusted_node_address(self.js_runtime.ptr, mouse_over_addresses[0]).root();
+
+            if let Some(ref frame) = *page.frame() {
+                let window = frame.window.root();
+
+                let x = point.x.to_i32().unwrap_or(0);
+                let y = point.y.to_i32().unwrap_or(0);
+
+                let mouse_event = MouseEvent::new(window.r(),
+                    "mousemove".to_owned(),
+                    true,
+                    true,
+                    Some(window.r()),
+                    0i32,
+                    x, y, x, y,
+                    false, false, false, false,
+                    0i16,
+                    None).root();
+
+                let event: JSRef<Event> = EventCast::from_ref(mouse_event.r());
+                let target: JSRef<EventTarget> = EventTargetCast::from_ref(top_most_node.r());
+                event.fire(target);
+            }
+        }
+
+        // Store the current mouse over targets for next frame
+        *prev_mouse_over_targets = mouse_over_targets;
+
+        // Reflow if hover state changed
+        if needs_reflow {
+            self.force_reflow(&*page);
         }
     }
 }