diff options
author | Martin Robinson <mrobinson@igalia.com> | 2023-09-15 12:57:54 +0200 |
---|---|---|
committer | GitHub <noreply@github.com> | 2023-09-15 10:57:54 +0000 |
commit | abca586e0aa6111d9acd77d4976d52da7671214f (patch) | |
tree | 66d53361ec9c06246a5caf1ac5eec9ee35f8027c | |
parent | 0b86d6579823d0786b37cee86eaaf3ce6bd8aa7d (diff) | |
download | servo-abca586e0aa6111d9acd77d4976d52da7671214f.tar.gz servo-abca586e0aa6111d9acd77d4976d52da7671214f.zip |
Refactor scrolls on the window object (#29680)
Refactor the scrolling and scrollable area calculation on the window
object, to make it better match the specification. This has some mild
changes to behavior, but in general things work the same as they did
before. This is mainly preparation for properly handling viewport
propagation of the `overflow` property but seems to fix a few issues as
well.
There is one new failure in Layout 2020 regarding `position: sticky`,
but this isn't a big deal because there is no support for `position:
sticky` in Layout 2020 yet.
Co-authored-by: Rakhi Sharma <atbrakhi@igalia.com>
-rw-r--r-- | components/layout/query.rs | 21 | ||||
-rw-r--r-- | components/layout_2020/fragment_tree/fragment.rs | 9 | ||||
-rw-r--r-- | components/layout_2020/fragment_tree/fragment_tree.rs | 46 | ||||
-rw-r--r-- | components/layout_2020/query.rs | 23 | ||||
-rw-r--r-- | components/layout_thread/lib.rs | 19 | ||||
-rw-r--r-- | components/layout_thread_2020/lib.rs | 10 | ||||
-rw-r--r-- | components/script/dom/element.rs | 12 | ||||
-rw-r--r-- | components/script/dom/node.rs | 54 | ||||
-rw-r--r-- | components/script/dom/window.rs | 38 | ||||
-rw-r--r-- | components/script_layout_interface/message.rs | 6 | ||||
-rw-r--r-- | components/script_layout_interface/rpc.rs | 2 | ||||
-rw-r--r-- | tests/wpt/meta/css/css-position/sticky/position-sticky-hyperlink.html.ini | 2 | ||||
-rw-r--r-- | tests/wpt/mozilla/meta/MANIFEST.json | 2 | ||||
-rw-r--r-- | tests/wpt/mozilla/tests/mozilla/scrollTo.html | 2 |
14 files changed, 128 insertions, 118 deletions
diff --git a/components/layout/query.rs b/components/layout/query.rs index a4eba68b947..61dd1c2ba33 100644 --- a/components/layout/query.rs +++ b/components/layout/query.rs @@ -76,7 +76,7 @@ pub struct LayoutThreadData { pub scroll_id_response: Option<ExternalScrollId>, /// A queued response for the scroll {top, left, width, height} of a node in pixels. - pub scroll_area_response: Rect<i32>, + pub scrolling_area_response: Rect<i32>, /// A queued response for the resolved style property of an element. pub resolved_style_response: String, @@ -158,9 +158,9 @@ impl LayoutRPC for LayoutRPCImpl { } } - fn node_scroll_area(&self) -> NodeGeometryResponse { + fn scrolling_area(&self) -> NodeGeometryResponse { NodeGeometryResponse { - client_rect: self.0.lock().unwrap().scroll_area_response, + client_rect: self.0.lock().unwrap().scrolling_area_response, } } @@ -730,10 +730,21 @@ pub fn process_node_scroll_id_request<'dom>( } /// https://drafts.csswg.org/cssom-view/#scrolling-area -pub fn process_node_scroll_area_request( - requested_node: OpaqueNode, +pub fn process_scrolling_area_request( + requested_node: Option<OpaqueNode>, layout_root: &mut dyn Flow, ) -> Rect<i32> { + let requested_node = match requested_node { + Some(node) => node, + None => { + let rect = layout_root.base().overflow.scroll; + return Rect::new( + Point2D::new(rect.origin.x.to_nearest_px(), rect.origin.y.to_nearest_px()), + Size2D::new(rect.width().ceil_to_px(), rect.height().ceil_to_px()), + ); + }, + }; + let mut iterator = UnioningFragmentScrollAreaIterator::new(requested_node); sequential::iterate_through_flow_tree_fragment_border_boxes(layout_root, &mut iterator); match iterator.overflow_direction { diff --git a/components/layout_2020/fragment_tree/fragment.rs b/components/layout_2020/fragment_tree/fragment.rs index 77b575a9d42..fda4636e9e7 100644 --- a/components/layout_2020/fragment_tree/fragment.rs +++ b/components/layout_2020/fragment_tree/fragment.rs @@ -167,6 +167,15 @@ impl Fragment { } } + pub fn scrolling_area(&self, containing_block: &PhysicalRect<Length>) -> PhysicalRect<Length> { + match self { + Fragment::Box(fragment) | Fragment::Float(fragment) => fragment + .scrollable_overflow(containing_block) + .translate(containing_block.origin.to_vector()), + _ => self.scrollable_overflow(containing_block), + } + } + pub fn scrollable_overflow( &self, containing_block: &PhysicalRect<Length>, diff --git a/components/layout_2020/fragment_tree/fragment_tree.rs b/components/layout_2020/fragment_tree/fragment_tree.rs index 023344a1b6c..f797f5644c6 100644 --- a/components/layout_2020/fragment_tree/fragment_tree.rs +++ b/components/layout_2020/fragment_tree/fragment_tree.rs @@ -172,36 +172,26 @@ impl FragmentTree { .unwrap_or_else(Rect::zero) } - pub fn get_scroll_area_for_node(&self, requested_node: OpaqueNode) -> Rect<i32> { - let mut scroll_area: PhysicalRect<Length> = PhysicalRect::zero(); + pub fn get_scrolling_area_for_viewport(&self) -> PhysicalRect<Length> { + let mut scroll_area = self.initial_containing_block; + for fragment in self.root_fragments.iter() { + scroll_area = fragment + .borrow() + .scrolling_area(&self.initial_containing_block) + .union(&scroll_area); + } + scroll_area + } + + pub fn get_scrolling_area_for_node(&self, requested_node: OpaqueNode) -> PhysicalRect<Length> { let tag_to_find = Tag::new(requested_node); - self.find(|fragment, _, containing_block| { - if fragment.tag() != Some(tag_to_find) { - return None::<()>; + let scroll_area = self.find(|fragment, _, containing_block| { + if fragment.tag() == Some(tag_to_find) { + Some(fragment.scrolling_area(&containing_block)) + } else { + None } - - scroll_area = match fragment { - Fragment::Box(fragment) | Fragment::Float(fragment) => fragment - .scrollable_overflow(&containing_block) - .translate(containing_block.origin.to_vector()), - Fragment::Text(_) | - Fragment::AbsoluteOrFixedPositioned(_) | - Fragment::Image(_) | - Fragment::IFrame(_) | - Fragment::Anonymous(_) => return None, - }; - None::<()> }); - - Rect::new( - Point2D::new( - scroll_area.origin.x.px() as i32, - scroll_area.origin.y.px() as i32, - ), - Size2D::new( - scroll_area.size.width.px() as i32, - scroll_area.size.height.px() as i32, - ), - ) + scroll_area.unwrap_or_else(PhysicalRect::<Length>::zero) } } diff --git a/components/layout_2020/query.rs b/components/layout_2020/query.rs index e71b5df20ee..3ea4003cd0a 100644 --- a/components/layout_2020/query.rs +++ b/components/layout_2020/query.rs @@ -56,7 +56,7 @@ pub struct LayoutThreadData { pub scroll_id_response: Option<ExternalScrollId>, /// A queued response for the scroll {top, left, width, height} of a node in pixels. - pub scroll_area_response: Rect<i32>, + pub scrolling_area_response: Rect<i32>, /// A queued response for the resolved style property of an element. pub resolved_style_response: String, @@ -115,9 +115,9 @@ impl LayoutRPC for LayoutRPCImpl { } } - fn node_scroll_area(&self) -> NodeGeometryResponse { + fn scrolling_area(&self) -> NodeGeometryResponse { NodeGeometryResponse { - client_rect: self.0.lock().unwrap().scroll_area_response, + client_rect: self.0.lock().unwrap().scrolling_area_response, } } @@ -201,14 +201,19 @@ pub fn process_node_scroll_id_request<'dom>( /// https://drafts.csswg.org/cssom-view/#scrolling-area pub fn process_node_scroll_area_request( - requested_node: OpaqueNode, + requested_node: Option<OpaqueNode>, fragment_tree: Option<Arc<FragmentTree>>, ) -> Rect<i32> { - if let Some(fragment_tree) = fragment_tree { - fragment_tree.get_scroll_area_for_node(requested_node) - } else { - Rect::zero() - } + let rect = match (fragment_tree, requested_node) { + (Some(tree), Some(node)) => tree.get_scrolling_area_for_node(node), + (Some(tree), None) => tree.get_scrolling_area_for_viewport(), + _ => return Rect::zero(), + }; + + Rect::new( + Point2D::new(rect.origin.x.px() as i32, rect.origin.y.px() as i32), + Size2D::new(rect.size.width.px() as i32, rect.size.height.px() as i32), + ) } /// Return the resolved value of property for a given (pseudo)element. diff --git a/components/layout_thread/lib.rs b/components/layout_thread/lib.rs index 515648e21e3..aa08b341ee6 100644 --- a/components/layout_thread/lib.rs +++ b/components/layout_thread/lib.rs @@ -41,10 +41,9 @@ use layout::flow_ref::FlowRef; use layout::incremental::{RelayoutMode, SpecialRestyleDamage}; use layout::query::{ process_client_rect_query, process_content_box_request, process_content_boxes_request, - process_element_inner_text_query, process_node_scroll_area_request, - process_node_scroll_id_request, process_offset_parent_query, - process_resolved_font_style_request, process_resolved_style_request, LayoutRPCImpl, - LayoutThreadData, + process_element_inner_text_query, process_node_scroll_id_request, process_offset_parent_query, + process_resolved_font_style_request, process_resolved_style_request, + process_scrolling_area_request, LayoutRPCImpl, LayoutThreadData, }; use layout::traversal::{ construct_flows_at_ancestors, ComputeStackingRelativePositions, PreorderFlowTraversal, @@ -492,7 +491,7 @@ impl LayoutThread { content_boxes_response: Vec::new(), client_rect_response: Rect::zero(), scroll_id_response: None, - scroll_area_response: Rect::zero(), + scrolling_area_response: Rect::zero(), resolved_style_response: String::new(), resolved_font_style_response: None, offset_parent_response: OffsetParentResponse::empty(), @@ -1121,8 +1120,8 @@ impl LayoutThread { &QueryMsg::ClientRectQuery(_) => { rw_data.client_rect_response = Rect::zero(); }, - &QueryMsg::NodeScrollGeometryQuery(_) => { - rw_data.scroll_area_response = Rect::zero(); + &QueryMsg::ScrollingAreaQuery(_) => { + rw_data.scrolling_area_response = Rect::zero(); }, &QueryMsg::NodeScrollIdQuery(_) => { rw_data.scroll_id_response = None; @@ -1431,9 +1430,9 @@ impl LayoutThread { &QueryMsg::ClientRectQuery(node) => { rw_data.client_rect_response = process_client_rect_query(node, root_flow); }, - &QueryMsg::NodeScrollGeometryQuery(node) => { - rw_data.scroll_area_response = - process_node_scroll_area_request(node, root_flow); + &QueryMsg::ScrollingAreaQuery(node) => { + rw_data.scrolling_area_response = + process_scrolling_area_request(node, root_flow); }, &QueryMsg::NodeScrollIdQuery(node) => { let node: ServoLayoutNode<LayoutData> = unsafe { ServoLayoutNode::new(&node) }; diff --git a/components/layout_thread_2020/lib.rs b/components/layout_thread_2020/lib.rs index 56bbd204e78..793e3a2f665 100644 --- a/components/layout_thread_2020/lib.rs +++ b/components/layout_thread_2020/lib.rs @@ -469,7 +469,7 @@ impl LayoutThread { content_boxes_response: Vec::new(), client_rect_response: Rect::zero(), scroll_id_response: None, - scroll_area_response: Rect::zero(), + scrolling_area_response: Rect::zero(), resolved_style_response: String::new(), resolved_font_style_response: None, offset_parent_response: OffsetParentResponse::empty(), @@ -817,8 +817,8 @@ impl LayoutThread { &QueryMsg::ClientRectQuery(_) => { rw_data.client_rect_response = Rect::zero(); }, - &QueryMsg::NodeScrollGeometryQuery(_) => { - rw_data.scroll_area_response = Rect::zero(); + &QueryMsg::ScrollingAreaQuery(_) => { + rw_data.scrolling_area_response = Rect::zero(); }, &QueryMsg::NodeScrollIdQuery(_) => { rw_data.scroll_id_response = None; @@ -1096,8 +1096,8 @@ impl LayoutThread { rw_data.client_rect_response = process_node_geometry_request(node, self.fragment_tree.borrow().clone()); }, - &QueryMsg::NodeScrollGeometryQuery(node) => { - rw_data.scroll_area_response = + &QueryMsg::ScrollingAreaQuery(node) => { + rw_data.scrolling_area_response = process_node_scroll_area_request(node, self.fragment_tree.borrow().clone()); }, &QueryMsg::NodeScrollIdQuery(node) => { diff --git a/components/script/dom/element.rs b/components/script/dom/element.rs index 31ea2701c82..f204b95f3b4 100644 --- a/components/script/dom/element.rs +++ b/components/script/dom/element.rs @@ -584,6 +584,13 @@ impl Element { node.parent_directionality() }) } + + pub(crate) fn is_root(&self) -> bool { + match self.node.GetParentNode() { + None => false, + Some(node) => node.is::<Document>(), + } + } } #[inline] @@ -3194,10 +3201,7 @@ impl<'a> SelectorsElement for DomRoot<Element> { } fn is_root(&self) -> bool { - match self.node.GetParentNode() { - None => false, - Some(node) => node.is::<Document>(), - } + Element::is_root(self) } fn is_empty(&self) -> bool { diff --git a/components/script/dom/node.rs b/components/script/dom/node.rs index 4758c28717d..d360ee3694f 100644 --- a/components/script/dom/node.rs +++ b/components/script/dom/node.rs @@ -16,7 +16,7 @@ use app_units::Au; use bitflags::bitflags; use devtools_traits::NodeInfo; use dom_struct::dom_struct; -use euclid::default::{Point2D, Rect, Size2D, Vector2D}; +use euclid::default::{Rect, Size2D, Vector2D}; use html5ever::{namespace_url, ns, Namespace, Prefix, QualName}; use js::jsapi::JSObject; use js::rust::HandleObject; @@ -808,38 +808,42 @@ impl Node { // https://drafts.csswg.org/cssom-view/#dom-element-scrollwidth // https://drafts.csswg.org/cssom-view/#dom-element-scrollheight pub fn scroll_area(&self) -> Rect<i32> { - // Step 1 + // "1. Let document be the element’s node document."" let document = self.owner_doc(); - // Step 3 - let window = document.window(); - let html_element = document.GetDocumentElement(); + // "2. If document is not the active document, return zero and terminate these steps."" + if !document.is_active() { + return Rect::zero(); + } + + // "3. Let viewport width/height be the width of the viewport excluding the width/height of the + // scroll bar, if any, or zero if there is no viewport." + let window = document.window(); + let viewport = Size2D::new(window.InnerWidth(), window.InnerHeight()); + let in_quirks_mode = document.quirks_mode() == QuirksMode::Quirks; + let is_root = self.downcast::<Element>().map_or(false, |e| e.is_root()); let is_body_element = self .downcast::<HTMLBodyElement>() .map_or(false, |e| e.is_the_html_body_element()); - let scroll_area = window.scroll_area_query(self); - - match ( - document != window.Document(), - is_body_element, - document.quirks_mode(), - html_element.as_deref() == self.downcast::<Element>(), - ) { - // Step 2 && Step 5 - (true, _, _, _) | (_, false, QuirksMode::Quirks, true) => Rect::zero(), - // Step 6 && Step 7 - (false, false, _, true) | (false, true, QuirksMode::Quirks, _) => Rect::new( - Point2D::new(window.ScrollX(), window.ScrollY()), - Size2D::new( - cmp::max(window.InnerWidth(), scroll_area.size.width), - cmp::max(window.InnerHeight(), scroll_area.size.height), - ), - ), - // Step 9 - _ => scroll_area, + // "4. If the element is the root element and document is not in quirks mode + // return max(viewport scrolling area width/height, viewport width/height)." + // "5. If the element is the body element, document is in quirks mode and the + // element is not potentially scrollable, return max(viewport scrolling area + // width, viewport width)." + if (is_root && !in_quirks_mode) || (is_body_element && in_quirks_mode) { + let viewport_scrolling_area = window.scrolling_area_query(None); + return Rect::new( + viewport_scrolling_area.origin, + viewport_scrolling_area.size.max(viewport), + ); } + + // "6. If the element does not have any associated box return zero and terminate + // these steps." + // "7. Return the width of the element’s scrolling area." + window.scrolling_area_query(Some(self)) } pub fn scroll_offset(&self) -> Vector2D<f32> { diff --git a/components/script/dom/window.rs b/components/script/dom/window.rs index ce7306e8e1b..a0af8e2537d 100644 --- a/components/script/dom/window.rs +++ b/components/script/dom/window.rs @@ -1721,26 +1721,13 @@ impl Window { // Step 7 & 8 // TODO: Consider `block-end` and `inline-end` overflow direction. - let body = self.Document().GetBody(); - let (x, y) = match body { - Some(e) => { - // This doesn't properly take into account the overflow set on <body> - // and the root element, which might affect how much the root can - // scroll. That requires properly handling propagating those values - // according to the rules defined in in the specification at: - // https://w3c.github.io/csswg-drafts/css-overflow/#overflow-propagation - let scroll_area = e.upcast::<Node>().bounding_content_box_or_zero(); - ( - xfinite - .min(scroll_area.width().to_f64_px() - viewport.width as f64) - .max(0.0f64), - yfinite - .min(scroll_area.height().to_f64_px() - viewport.height as f64) - .max(0.0f64), - ) - }, - None => (xfinite.max(0.0f64), yfinite.max(0.0f64)), - }; + let scrolling_area = self.scrolling_area_query(None); + let x = xfinite + .min(scrolling_area.width() as f64 - viewport.width as f64) + .max(0.0f64); + let y = yfinite + .min(scrolling_area.height() as f64 - viewport.height as f64) + .max(0.0f64); // Step 10 //TODO handling ongoing smooth scrolling @@ -2113,11 +2100,14 @@ impl Window { self.layout_rpc.node_geometry().client_rect } - pub fn scroll_area_query(&self, node: &Node) -> UntypedRect<i32> { - if !self.layout_reflow(QueryMsg::NodeScrollGeometryQuery(node.to_opaque())) { + /// Find the scroll area of the given node, if it is not None. If the node + /// is None, find the scroll area of the viewport. + pub fn scrolling_area_query(&self, node: Option<&Node>) -> UntypedRect<i32> { + let opaque = node.map(|node| node.to_opaque()); + if !self.layout_reflow(QueryMsg::ScrollingAreaQuery(opaque)) { return Rect::zero(); } - self.layout_rpc.node_scroll_area().client_rect + self.layout_rpc.scrolling_area().client_rect } pub fn scroll_offset_query(&self, node: &Node) -> Vector2D<f32, LayoutPixel> { @@ -2748,7 +2738,7 @@ fn debug_reflow_events(id: PipelineId, reflow_goal: &ReflowGoal, reason: &Reflow &QueryMsg::ContentBoxesQuery(_n) => "\tContentBoxesQuery", &QueryMsg::NodesFromPointQuery(..) => "\tNodesFromPointQuery", &QueryMsg::ClientRectQuery(_n) => "\tClientRectQuery", - &QueryMsg::NodeScrollGeometryQuery(_n) => "\tNodeScrollGeometryQuery", + &QueryMsg::ScrollingAreaQuery(_n) => "\tNodeScrollGeometryQuery", &QueryMsg::NodeScrollIdQuery(_n) => "\tNodeScrollIdQuery", &QueryMsg::ResolvedStyleQuery(_, _, _) => "\tResolvedStyleQuery", &QueryMsg::ResolvedFontStyleQuery(..) => "\nResolvedFontStyleQuery", diff --git a/components/script_layout_interface/message.rs b/components/script_layout_interface/message.rs index 5aeadef6894..01c5d58e659 100644 --- a/components/script_layout_interface/message.rs +++ b/components/script_layout_interface/message.rs @@ -101,7 +101,7 @@ pub enum QueryMsg { ContentBoxQuery(OpaqueNode), ContentBoxesQuery(OpaqueNode), ClientRectQuery(OpaqueNode), - NodeScrollGeometryQuery(OpaqueNode), + ScrollingAreaQuery(Option<OpaqueNode>), OffsetParentQuery(OpaqueNode), TextIndexQuery(OpaqueNode, Point2D<f32>), NodesFromPointQuery(Point2D<f32>, NodesFromPointQueryType), @@ -143,7 +143,7 @@ impl ReflowGoal { QueryMsg::ContentBoxQuery(_) | QueryMsg::ContentBoxesQuery(_) | QueryMsg::ClientRectQuery(_) | - QueryMsg::NodeScrollGeometryQuery(_) | + QueryMsg::ScrollingAreaQuery(_) | QueryMsg::NodeScrollIdQuery(_) | QueryMsg::ResolvedStyleQuery(..) | QueryMsg::ResolvedFontStyleQuery(..) | @@ -165,7 +165,7 @@ impl ReflowGoal { QueryMsg::ContentBoxQuery(_) | QueryMsg::ContentBoxesQuery(_) | QueryMsg::ClientRectQuery(_) | - QueryMsg::NodeScrollGeometryQuery(_) | + QueryMsg::ScrollingAreaQuery(_) | QueryMsg::NodeScrollIdQuery(_) | QueryMsg::ResolvedStyleQuery(..) | QueryMsg::ResolvedFontStyleQuery(..) | diff --git a/components/script_layout_interface/rpc.rs b/components/script_layout_interface/rpc.rs index 232fecabe6b..3778d8f975a 100644 --- a/components/script_layout_interface/rpc.rs +++ b/components/script_layout_interface/rpc.rs @@ -27,7 +27,7 @@ pub trait LayoutRPC { /// Requests the geometry of this node. Used by APIs such as `clientTop`. fn node_geometry(&self) -> NodeGeometryResponse; /// Requests the scroll geometry of this node. Used by APIs such as `scrollTop`. - fn node_scroll_area(&self) -> NodeGeometryResponse; + fn scrolling_area(&self) -> NodeGeometryResponse; /// Requests the scroll id of this node. Used by APIs such as `scrollTop` fn node_scroll_id(&self) -> NodeScrollIdResponse; /// Query layout for the resolved value of a given CSS property diff --git a/tests/wpt/meta/css/css-position/sticky/position-sticky-hyperlink.html.ini b/tests/wpt/meta/css/css-position/sticky/position-sticky-hyperlink.html.ini deleted file mode 100644 index 8b52a2be825..00000000000 --- a/tests/wpt/meta/css/css-position/sticky/position-sticky-hyperlink.html.ini +++ /dev/null @@ -1,2 +0,0 @@ -[position-sticky-hyperlink.html] - expected: FAIL diff --git a/tests/wpt/mozilla/meta/MANIFEST.json b/tests/wpt/mozilla/meta/MANIFEST.json index d6e7863afe8..c509870b10a 100644 --- a/tests/wpt/mozilla/meta/MANIFEST.json +++ b/tests/wpt/mozilla/meta/MANIFEST.json @@ -13794,7 +13794,7 @@ ] ], "scrollTo.html": [ - "f1b4384e63bfc12c45c3eca5edcd98ad32a85502", + "b90c4e69344216372c9f118d70f4989e82a6fce7", [ null, {} diff --git a/tests/wpt/mozilla/tests/mozilla/scrollTo.html b/tests/wpt/mozilla/tests/mozilla/scrollTo.html index f1b4384e63b..b90c4e69344 100644 --- a/tests/wpt/mozilla/tests/mozilla/scrollTo.html +++ b/tests/wpt/mozilla/tests/mozilla/scrollTo.html @@ -3,6 +3,7 @@ <title></title> <script src="/resources/testharness.js"></script> <script src="/resources/testharnessreport.js"></script> +<div style="position: absolute; top: 0; left: 0; width: 10000px; height: 10000px; background: purple"></div> <script> test(function() { scrollTo(0, 5000); @@ -18,4 +19,3 @@ test(function() { assert_equals(pageYOffset, 0); }); </script> -<div style="position: absolute; top: 0; left: 0; width: 10000px; height: 10000px; background: purple"></div> |