From b1f05816373f5e68ce242e0b1a45a50851f868bb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Sat, 20 Feb 2016 20:19:13 +0100 Subject: [PATCH 1/5] script: Fix MouseOver handling Now we only query for the topmost node, and apply the hover state to all of the parent elements. This fixes things like #9705, where the hover state was applied only to the children. This also makes us more conformant with other browsers in the case of taking in account margins and paddings. For example, prior to this PR, when your mouse was over the inner element, in the bottom part, `hover` styles didn't apply to the parent. ```html
``` Fixes #9705 --- components/layout/query.rs | 7 ++---- components/script/dom/document.rs | 33 +++++++++++++++------------ components/script/layout_interface.rs | 3 ++- 3 files changed, 22 insertions(+), 21 deletions(-) diff --git a/components/layout/query.rs b/components/layout/query.rs index 348a3ad556d..54acfe92e90 100644 --- a/components/layout/query.rs +++ b/components/layout/query.rs @@ -107,11 +107,8 @@ impl LayoutRPC for LayoutRPCImpl { if mouse_over_list.is_empty() { Err(()) } else { - let response_list = - mouse_over_list.iter() - .map(|metadata| metadata.node.to_untrusted_node_address()) - .collect(); - Ok(MouseOverResponse(response_list)) + let response = mouse_over_list[0].node.to_untrusted_node_address(); + Ok(MouseOverResponse(response)) } } diff --git a/components/script/dom/document.rs b/components/script/dom/document.rs index 501ad242305..4062fb7415f 100644 --- a/components/script/dom/document.rs +++ b/components/script/dom/document.rs @@ -574,11 +574,11 @@ impl Document { } } - pub fn get_nodes_under_mouse(&self, page_point: &Point2D) -> Vec { + pub fn get_node_under_mouse(&self, page_point: &Point2D) -> Option { assert!(self.GetDocumentElement().is_some()); match self.window.layout().mouse_over(*page_point) { - Ok(MouseOverResponse(node_address)) => node_address, - Err(()) => vec![], + Ok(MouseOverResponse(node_address)) => Some(node_address), + Err(()) => None, } } @@ -808,18 +808,21 @@ impl Document { client_point: Option>, prev_mouse_over_targets: &mut RootedVec>) { // Build a list of elements that are currently under the mouse. - let mouse_over_addresses = client_point.as_ref().map(|client_point| { + let mouse_over_address = client_point.as_ref().map(|client_point| { let page_point = Point2D::new(client_point.x + self.window.PageXOffset() as f32, client_point.y + self.window.PageYOffset() as f32); - self.get_nodes_under_mouse(&page_point) - }).unwrap_or(vec![]); - let mut mouse_over_targets = mouse_over_addresses.iter().map(|node_address| { - node::from_untrusted_node_address(js_runtime, *node_address) - .inclusive_ancestors() - .filter_map(Root::downcast::) - .next() - .unwrap() - }).collect::>>(); + self.get_node_under_mouse(&page_point) + }).unwrap_or(None); + + let mut mouse_over_targets = RootedVec::>::new(); + + if let Some(address) = mouse_over_address { + let node = node::from_untrusted_node_address(js_runtime, address); + for node in node.inclusive_ancestors() + .filter_map(Root::downcast::) { + mouse_over_targets.push(JS::from_rooted(&node)); + } + } // Remove hover from any elements in the previous list that are no longer // under the mouse. @@ -856,9 +859,9 @@ impl Document { } // Send mousemove event to topmost target - if mouse_over_addresses.len() > 0 { + if let Some(address) = mouse_over_address { let top_most_node = node::from_untrusted_node_address(js_runtime, - mouse_over_addresses[0]); + address); let client_point = client_point.unwrap(); // Must succeed because hit test succeeded. // If the target is an iframe, forward the event to the child document. diff --git a/components/script/layout_interface.rs b/components/script/layout_interface.rs index 17f553d43e8..4ad7c241f14 100644 --- a/components/script/layout_interface.rs +++ b/components/script/layout_interface.rs @@ -106,6 +106,7 @@ pub trait LayoutRPC { fn node_geometry(&self) -> NodeGeometryResponse; /// Requests the node containing the point of interest fn hit_test(&self, point: Point2D) -> Result; + /// Query layout for the topmost node under the mouse. fn mouse_over(&self, point: Point2D) -> Result; /// Query layout for the resolved value of a given CSS property fn resolved_style(&self) -> ResolvedStyleResponse; @@ -139,7 +140,7 @@ pub struct NodeGeometryResponse { pub client_rect: Rect, } pub struct HitTestResponse(pub UntrustedNodeAddress); -pub struct MouseOverResponse(pub Vec); +pub struct MouseOverResponse(pub UntrustedNodeAddress); pub struct ResolvedStyleResponse(pub Option); #[derive(Clone)] From c3786437a39faf2c52b66f7dcc3b8a9430a62dbc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Sat, 20 Feb 2016 21:18:46 +0100 Subject: [PATCH 2/5] script: Fix `mouseover`/`mouseout` dispatching. --- components/script/dom/document.rs | 45 ++++++++++++++++++++----------- 1 file changed, 30 insertions(+), 15 deletions(-) diff --git a/components/script/dom/document.rs b/components/script/dom/document.rs index 4062fb7415f..966d245628b 100644 --- a/components/script/dom/document.rs +++ b/components/script/dom/document.rs @@ -824,21 +824,28 @@ impl Document { } } + // Werther the topmost element we are hovering now is diffrent than the previous + let is_different_topmost_target = mouse_over_targets.is_empty() || + prev_mouse_over_targets.is_empty() || + prev_mouse_over_targets[0].upcast::().to_trusted_node_address() != + mouse_over_targets[0].upcast::().to_trusted_node_address(); + // Remove hover from any elements in the previous list that are no longer // under the mouse. - for target in prev_mouse_over_targets.iter() { - if !mouse_over_targets.contains(target) { - let target_ref = &**target; - if target_ref.get_hover_state() { - target_ref.set_hover_state(false); + for (index, target) in prev_mouse_over_targets.iter().enumerate() { + // Hover state is reset as appropiate later + target.set_hover_state(false); - let target = target_ref.upcast(); + // https://www.w3.org/TR/uievents/#event-type-mouseout + // + // mouseout must be dispatched when the mouse moves off an element or when pointer + // mouse moves from an element onto the boundaries of one of its descendent elements. + let has_to_dispatch_mouse_out = index == 0 && is_different_topmost_target; - // FIXME: we should be dispatching this event but we lack an actual - // point to pass to it. - if let Some(client_point) = client_point { - self.fire_mouse_event(client_point, &target, "mouseout".to_owned()); - } + if has_to_dispatch_mouse_out { + let target = target.upcast(); + if let Some(client_point) = client_point { + self.fire_mouse_event(client_point, &target, "mouseout".to_owned()); } } } @@ -846,12 +853,20 @@ impl Document { // 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.r() { - if !target.get_hover_state() { - target.set_hover_state(true); + for (index, target) in mouse_over_targets.r().iter().enumerate() { + target.set_hover_state(true); + // https://www.w3.org/TR/uievents/#event-type-mouseover + // + // Mouseover must be fired when a pointing device is moved onto the boundaries of an + // element (we only fire it in the first because it bubbles), or when the pointer has + // moved from our children to ours. + // + // The below condition adresses both situations. + let has_to_dispatch_mouse_over = index == 0 && is_different_topmost_target; + + if has_to_dispatch_mouse_over { let target = target.upcast(); - if let Some(client_point) = client_point { self.fire_mouse_event(client_point, target, "mouseover".to_owned()); } From 48dee6413dae56e3a41832a8dc415d5cc6741fa8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Sat, 20 Feb 2016 21:36:24 +0100 Subject: [PATCH 3/5] script/layout: Refactor mouse_over since it now basically uses hit_test --- components/layout/query.rs | 37 +++++++-------------------- components/script/dom/document.rs | 22 +++++----------- components/script/layout_interface.rs | 5 +--- 3 files changed, 17 insertions(+), 47 deletions(-) diff --git a/components/layout/query.rs b/components/layout/query.rs index 54acfe92e90..df45301106a 100644 --- a/components/layout/query.rs +++ b/components/layout/query.rs @@ -16,7 +16,7 @@ use layout_thread::LayoutThreadData; use msg::constellation_msg::ConstellationChan; use opaque_node::OpaqueNodeMethods; use script::layout_interface::{ContentBoxResponse, ContentBoxesResponse, NodeGeometryResponse}; -use script::layout_interface::{HitTestResponse, LayoutRPC, MouseOverResponse, OffsetParentResponse}; +use script::layout_interface::{HitTestResponse, LayoutRPC, OffsetParentResponse}; use script::layout_interface::{ResolvedStyleResponse, ScriptLayoutChan, MarginStyleResponse}; use script_traits::LayoutMsg as ConstellationMsg; use sequential; @@ -67,48 +67,29 @@ impl LayoutRPC for LayoutRPCImpl { } /// Requests the node containing the point of interest. - fn hit_test(&self, point: Point2D) -> Result { + fn hit_test(&self, point: Point2D, update_cursor: bool) -> Result { let point = Point2D::new(Au::from_f32_px(point.x), Au::from_f32_px(point.y)); let &LayoutRPCImpl(ref rw_data) = self; let rw_data = rw_data.lock().unwrap(); - let result = match rw_data.display_list { - None => panic!("Tried to hit test without a DisplayList"), - Some(ref display_list) => display_list.hit_test(point), - }; + let display_list = rw_data.display_list.as_ref().expect("Tried to hit test without a DisplayList!"); - if result.is_empty() { - return Err(()); - } + let result = display_list.hit_test(point); - Ok(HitTestResponse(result[0].node.to_untrusted_node_address())) - } - - fn mouse_over(&self, point: Point2D) -> Result { - let point = Point2D::new(Au::from_f32_px(point.x), Au::from_f32_px(point.y)); - let mouse_over_list = { - let &LayoutRPCImpl(ref rw_data) = self; - let rw_data = rw_data.lock().unwrap(); - let result = match rw_data.display_list { - None => panic!("Tried to hit test without a DisplayList"), - Some(ref display_list) => display_list.hit_test(point), - }; - - // Compute the new cursor. + if update_cursor { let cursor = if !result.is_empty() { result[0].pointing.unwrap() } else { Cursor::DefaultCursor }; + let ConstellationChan(ref constellation_chan) = rw_data.constellation_chan; constellation_chan.send(ConstellationMsg::SetCursor(cursor)).unwrap(); - result }; - if mouse_over_list.is_empty() { - Err(()) + if !result.is_empty() { + Ok(HitTestResponse(result[0].node.to_untrusted_node_address())) } else { - let response = mouse_over_list[0].node.to_untrusted_node_address(); - Ok(MouseOverResponse(response)) + Err(()) } } diff --git a/components/script/dom/document.rs b/components/script/dom/document.rs index 966d245628b..d3bbfded3b4 100644 --- a/components/script/dom/document.rs +++ b/components/script/dom/document.rs @@ -80,7 +80,7 @@ use html5ever::tree_builder::{LimitedQuirks, NoQuirks, Quirks, QuirksMode}; use ipc_channel::ipc::{self, IpcSender}; use js::jsapi::JS_GetRuntime; use js::jsapi::{JSContext, JSObject, JSRuntime}; -use layout_interface::{HitTestResponse, MouseOverResponse}; +use layout_interface::HitTestResponse; use layout_interface::{LayoutChan, Msg, ReflowQueryType}; use msg::constellation_msg::{ALT, CONTROL, SHIFT, SUPER}; use msg::constellation_msg::{ConstellationChan, Key, KeyModifiers, KeyState}; @@ -563,9 +563,9 @@ impl Document { .map(Root::upcast) } - pub fn hit_test(&self, page_point: &Point2D) -> Option { + pub fn hit_test(&self, page_point: &Point2D, update_cursor: bool) -> Option { assert!(self.GetDocumentElement().is_some()); - match self.window.layout().hit_test(*page_point) { + match self.window.layout().hit_test(*page_point, update_cursor) { Ok(HitTestResponse(node_address)) => Some(node_address), Err(()) => { debug!("layout query error"); @@ -574,14 +574,6 @@ impl Document { } } - pub fn get_node_under_mouse(&self, page_point: &Point2D) -> Option { - assert!(self.GetDocumentElement().is_some()); - match self.window.layout().mouse_over(*page_point) { - Ok(MouseOverResponse(node_address)) => Some(node_address), - Err(()) => None, - } - } - // https://html.spec.whatwg.org/multipage/#current-document-readiness pub fn set_ready_state(&self, state: DocumentReadyState) { match state { @@ -693,7 +685,7 @@ impl Document { let page_point = Point2D::new(client_point.x + self.window.PageXOffset() as f32, client_point.y + self.window.PageYOffset() as f32); - let node = match self.hit_test(&page_point) { + let node = match self.hit_test(&page_point, false) { Some(node_address) => { debug!("node address is {:?}", node_address); node::from_untrusted_node_address(js_runtime, node_address) @@ -811,7 +803,7 @@ impl Document { let mouse_over_address = client_point.as_ref().map(|client_point| { let page_point = Point2D::new(client_point.x + self.window.PageXOffset() as f32, client_point.y + self.window.PageYOffset() as f32); - self.get_node_under_mouse(&page_point) + self.hit_test(&page_point, true) }).unwrap_or(None); let mut mouse_over_targets = RootedVec::>::new(); @@ -918,7 +910,7 @@ impl Document { TouchEventType::Cancel => "touchcancel", }; - let node = match self.hit_test(&point) { + let node = match self.hit_test(&point, false) { Some(node_address) => node::from_untrusted_node_address(js_runtime, node_address), None => return false, }; @@ -2585,7 +2577,7 @@ impl DocumentMethods for Document { let js_runtime = unsafe { JS_GetRuntime(window.get_cx()) }; - match self.hit_test(point) { + match self.hit_test(point, false) { Some(untrusted_node_address) => { let node = node::from_untrusted_node_address(js_runtime, untrusted_node_address); let parent_node = node.GetParentNode().unwrap(); diff --git a/components/script/layout_interface.rs b/components/script/layout_interface.rs index 4ad7c241f14..3976f0a8d20 100644 --- a/components/script/layout_interface.rs +++ b/components/script/layout_interface.rs @@ -105,9 +105,7 @@ pub trait LayoutRPC { /// Requests the geometry of this node. Used by APIs such as `clientTop`. fn node_geometry(&self) -> NodeGeometryResponse; /// Requests the node containing the point of interest - fn hit_test(&self, point: Point2D) -> Result; - /// Query layout for the topmost node under the mouse. - fn mouse_over(&self, point: Point2D) -> Result; + fn hit_test(&self, point: Point2D, update_cursor: bool) -> Result; /// Query layout for the resolved value of a given CSS property fn resolved_style(&self) -> ResolvedStyleResponse; fn offset_parent(&self) -> OffsetParentResponse; @@ -140,7 +138,6 @@ pub struct NodeGeometryResponse { pub client_rect: Rect, } pub struct HitTestResponse(pub UntrustedNodeAddress); -pub struct MouseOverResponse(pub UntrustedNodeAddress); pub struct ResolvedStyleResponse(pub Option); #[derive(Clone)] From 73eff81e5d37e847c0e4365fd265a83d5d3468c0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Mon, 22 Feb 2016 00:31:36 +0100 Subject: [PATCH 4/5] Add manual tests for mouseover/out handling --- tests/wpt/metadata/MANIFEST.json | 8 ++ .../mouse-events/mouseover-out-manual.html | 125 ++++++++++++++++++ 2 files changed, 133 insertions(+) create mode 100644 tests/wpt/web-platform-tests/uievents/order-of-events/mouse-events/mouseover-out-manual.html diff --git a/tests/wpt/metadata/MANIFEST.json b/tests/wpt/metadata/MANIFEST.json index 98aaa6d001e..9885853d2ec 100644 --- a/tests/wpt/metadata/MANIFEST.json +++ b/tests/wpt/metadata/MANIFEST.json @@ -34513,6 +34513,14 @@ "local_changes": { "deleted": [], "items": { + "manual": { + "uievents/order-of-events/mouse-events/mouseover-out-manual.html": [ + { + "path": "uievents/order-of-events/mouse-events/mouseover-out-manual.html", + "url": "/uievents/order-of-events/mouse-events/mouseover-out-manual.html" + } + ] + }, "testharness": { "dom/lists/DOMTokenList-value.html": [ { diff --git a/tests/wpt/web-platform-tests/uievents/order-of-events/mouse-events/mouseover-out-manual.html b/tests/wpt/web-platform-tests/uievents/order-of-events/mouse-events/mouseover-out-manual.html new file mode 100644 index 00000000000..1d96fd303ad --- /dev/null +++ b/tests/wpt/web-platform-tests/uievents/order-of-events/mouse-events/mouseover-out-manual.html @@ -0,0 +1,125 @@ + + +Mouseover/mouseout handling + + + +

+ Steps: + +

    +
  1. Move your mouse over the blue <div> element, later + over the green one, later over the yellow one. +
  2. Move the mouse from the yellow element to the green one, later to the + blue one, and later over this paragraph. +
+

+ + +
+
+
+
+ +
+ + From 3662491d8f8c135eba360d0d6bae771c42e09a26 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Wed, 24 Feb 2016 17:37:11 +0100 Subject: [PATCH 5/5] Refactor mouseover code to be more performant This increases mouseover/out performance drastically on my machine. --- components/script/dom/document.rs | 148 ++++++++++++++--------------- components/script/dom/node.rs | 6 +- components/script/script_thread.rs | 77 ++++++++------- 3 files changed, 121 insertions(+), 110 deletions(-) diff --git a/components/script/dom/document.rs b/components/script/dom/document.rs index d3bbfded3b4..ec0e78fafc2 100644 --- a/components/script/dom/document.rs +++ b/components/script/dom/document.rs @@ -798,81 +798,33 @@ impl Document { pub fn handle_mouse_move_event(&self, js_runtime: *mut JSRuntime, client_point: Option>, - prev_mouse_over_targets: &mut RootedVec>) { - // Build a list of elements that are currently under the mouse. - let mouse_over_address = client_point.as_ref().map(|client_point| { - let page_point = Point2D::new(client_point.x + self.window.PageXOffset() as f32, - client_point.y + self.window.PageYOffset() as f32); - self.hit_test(&page_point, true) - }).unwrap_or(None); + prev_mouse_over_target: &MutNullableHeap>) { + let page_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(ref client_point) => { + Point2D::new(client_point.x + self.window.PageXOffset() as f32, + client_point.y + self.window.PageYOffset() as f32) + } + }; - let mut mouse_over_targets = RootedVec::>::new(); + let client_point = client_point.unwrap(); - if let Some(address) = mouse_over_address { + let maybe_new_target = self.hit_test(&page_point, true).and_then(|address| { let node = node::from_untrusted_node_address(js_runtime, address); - for node in node.inclusive_ancestors() - .filter_map(Root::downcast::) { - mouse_over_targets.push(JS::from_rooted(&node)); - } - } - - // Werther the topmost element we are hovering now is diffrent than the previous - let is_different_topmost_target = mouse_over_targets.is_empty() || - prev_mouse_over_targets.is_empty() || - prev_mouse_over_targets[0].upcast::().to_trusted_node_address() != - mouse_over_targets[0].upcast::().to_trusted_node_address(); - - // Remove hover from any elements in the previous list that are no longer - // under the mouse. - for (index, target) in prev_mouse_over_targets.iter().enumerate() { - // Hover state is reset as appropiate later - target.set_hover_state(false); - - // https://www.w3.org/TR/uievents/#event-type-mouseout - // - // mouseout must be dispatched when the mouse moves off an element or when pointer - // mouse moves from an element onto the boundaries of one of its descendent elements. - let has_to_dispatch_mouse_out = index == 0 && is_different_topmost_target; - - if has_to_dispatch_mouse_out { - let target = target.upcast(); - if let Some(client_point) = client_point { - self.fire_mouse_event(client_point, &target, "mouseout".to_owned()); - } - } - } - - // 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 (index, target) in mouse_over_targets.r().iter().enumerate() { - target.set_hover_state(true); - - // https://www.w3.org/TR/uievents/#event-type-mouseover - // - // Mouseover must be fired when a pointing device is moved onto the boundaries of an - // element (we only fire it in the first because it bubbles), or when the pointer has - // moved from our children to ours. - // - // The below condition adresses both situations. - let has_to_dispatch_mouse_over = index == 0 && is_different_topmost_target; - - if has_to_dispatch_mouse_over { - let target = target.upcast(); - if let Some(client_point) = client_point { - self.fire_mouse_event(client_point, target, "mouseover".to_owned()); - } - } - } - - // Send mousemove event to topmost target - if let Some(address) = mouse_over_address { - let top_most_node = node::from_untrusted_node_address(js_runtime, - address); - let client_point = client_point.unwrap(); // Must succeed because hit test succeeded. + node.inclusive_ancestors() + .filter_map(Root::downcast::) + .next() + }); + // Send mousemove event to topmost target, and forward it if it's an iframe + if let Some(ref new_target) = maybe_new_target { // If the target is an iframe, forward the event to the child document. - if let Some(iframe) = top_most_node.downcast::() { + if let Some(iframe) = new_target.downcast::() { if let Some(pipeline_id) = iframe.pipeline_id() { let rect = iframe.upcast::().GetBoundingClientRect(); let child_origin = Point2D::new(rect.X() as f32, rect.Y() as f32); @@ -884,13 +836,59 @@ impl Document { return; } - let target = top_most_node.upcast(); - self.fire_mouse_event(client_point, target, "mousemove".to_owned()); + self.fire_mouse_event(client_point, new_target.upcast(), "mousemove".to_owned()); } - // Store the current mouse over targets for next frame - prev_mouse_over_targets.clear(); - prev_mouse_over_targets.append(&mut *mouse_over_targets); + // 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::().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() + .filter_map(Root::downcast::) { + element.set_hover_state(false); + } + } + + // Remove hover state to old target and its parents + self.fire_mouse_event(client_point, old_target.upcast(), "mouseout".to_owned()); + + // 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() + .filter_map(Root::downcast::) { + if element.get_hover_state() { + break; + } + + element.set_hover_state(true); + } + + self.fire_mouse_event(client_point, &new_target.upcast(), "mouseover".to_owned()); + + // TODO: Fire mouseenter here. + } + + // Store the current mouse over target for next frame. + prev_mouse_over_target.set(maybe_new_target.as_ref().map(|target| target.r())); self.window.reflow(ReflowGoal::ForDisplay, ReflowQueryType::NoQuery, diff --git a/components/script/dom/node.rs b/components/script/dom/node.rs index ec72b6eed2f..717be0331d5 100644 --- a/components/script/dom/node.rs +++ b/components/script/dom/node.rs @@ -518,7 +518,11 @@ impl Node { } pub fn is_inclusive_ancestor_of(&self, parent: &Node) -> bool { - self == parent || parent.ancestors().any(|ancestor| ancestor.r() == self) + self == parent || self.is_ancestor_of(parent) + } + + pub fn is_ancestor_of(&self, parent: &Node) -> bool { + parent.ancestors().any(|ancestor| ancestor.r() == self) } pub fn following_siblings(&self) -> NodeSiblingIterator { diff --git a/components/script/script_thread.rs b/components/script/script_thread.rs index ddf32f85406..a735603431a 100644 --- a/components/script/script_thread.rs +++ b/components/script/script_thread.rs @@ -27,10 +27,10 @@ use dom::bindings::codegen::Bindings::DocumentBinding::{DocumentMethods, Documen use dom::bindings::conversions::{FromJSValConvertible, StringificationBehavior}; use dom::bindings::global::GlobalRef; use dom::bindings::inheritance::Castable; -use dom::bindings::js::{JS, RootCollection, trace_roots}; +use dom::bindings::js::{JS, MutNullableHeap, Root, RootCollection, trace_roots}; use dom::bindings::js::{RootCollectionPtr, RootedReference}; use dom::bindings::refcounted::{LiveDOMReferences, Trusted, TrustedReference, trace_refcounted_objects}; -use dom::bindings::trace::{JSTraceable, RootedVec, trace_traceables}; +use dom::bindings::trace::{JSTraceable, trace_traceables}; use dom::bindings::utils::{DOM_CALLBACKS, WRAP_CALLBACKS}; use dom::browsingcontext::BrowsingContext; use dom::document::{Document, DocumentProgressHandler, DocumentSource, FocusType, IsHTMLDocument}; @@ -91,7 +91,6 @@ use std::cell::{Cell, RefCell}; use std::collections::HashSet; use std::io::{Write, stdout}; use std::marker::PhantomData; -use std::mem as std_mem; use std::option::Option; use std::ptr; use std::rc::Rc; @@ -543,7 +542,8 @@ pub struct ScriptThread { /// The JavaScript runtime. js_runtime: Rc, - mouse_over_targets: DOMRefCell>>, + /// The topmost element over the mouse. + topmost_mouse_over_target: MutNullableHeap>, /// List of pipelines that have been owned and closed by this script thread. closed_pipelines: DOMRefCell>, @@ -789,7 +789,7 @@ impl ScriptThread { devtools_sender: ipc_devtools_sender, js_runtime: Rc::new(runtime), - mouse_over_targets: DOMRefCell::new(vec!()), + topmost_mouse_over_target: MutNullableHeap::new(Default::default()), closed_pipelines: DOMRefCell::new(HashSet::new()), scheduler_chan: state.scheduler_chan, @@ -1979,46 +1979,55 @@ impl ScriptThread { let page = get_page(&self.root_page(), pipeline_id); let document = page.document(); - let mut prev_mouse_over_targets: RootedVec> = RootedVec::new(); - for target in &*self.mouse_over_targets.borrow_mut() { - prev_mouse_over_targets.push(target.clone()); + // Get the previous target temporarily + let prev_mouse_over_target = self.topmost_mouse_over_target.get(); + + document.handle_mouse_move_event(self.js_runtime.rt(), point, + &self.topmost_mouse_over_target); + + // Short-circuit if nothing changed + if self.topmost_mouse_over_target.get() == prev_mouse_over_target { + return; } - // We temporarily steal the list of targets over which the mouse is to pass it to - // handle_mouse_move_event() in a safe RootedVec container. - let mut mouse_over_targets = RootedVec::new(); - std_mem::swap(&mut *self.mouse_over_targets.borrow_mut(), &mut *mouse_over_targets); - document.handle_mouse_move_event(self.js_runtime.rt(), point, &mut mouse_over_targets); - - // Notify Constellation about anchors that are no longer mouse over targets. - for target in &*prev_mouse_over_targets { - if !mouse_over_targets.contains(target) && target.is::() { - let event = ConstellationMsg::NodeStatus(None); - let ConstellationChan(ref chan) = self.constellation_chan; - chan.send(event).unwrap(); - break; - } - } + let mut state_already_changed = false; // Notify Constellation about the topmost anchor mouse over target. - for target in &*mouse_over_targets { - if target.is::() { - let status = target.get_attribute(&ns!(), &atom!("href")) - .and_then(|href| { - let value = href.value(); - let url = document.url(); - url.join(&value).map(|url| url.serialize()).ok() - }); + if let Some(target) = self.topmost_mouse_over_target.get() { + if let Some(anchor) = target.upcast::() + .inclusive_ancestors() + .filter_map(Root::downcast::) + .next() { + let status = anchor.upcast::() + .get_attribute(&ns!(), &atom!("href")) + .and_then(|href| { + let value = href.value(); + let url = document.url(); + url.join(&value).map(|url| url.serialize()).ok() + }); + let event = ConstellationMsg::NodeStatus(status); let ConstellationChan(ref chan) = self.constellation_chan; chan.send(event).unwrap(); - break; + + state_already_changed = true; } } - std_mem::swap(&mut *self.mouse_over_targets.borrow_mut(), &mut *mouse_over_targets); + // We might have to reset the anchor state + if !state_already_changed { + if let Some(target) = prev_mouse_over_target { + if let Some(_) = target.upcast::() + .inclusive_ancestors() + .filter_map(Root::downcast::) + .next() { + let event = ConstellationMsg::NodeStatus(None); + let ConstellationChan(ref chan) = self.constellation_chan; + chan.send(event).unwrap(); + } + } + } } - TouchEvent(event_type, identifier, point) => { let handled = self.handle_touch_event(pipeline_id, event_type, identifier, point); match event_type {