From 2c3d580ef1c5a188b373eb9c4c0cd2b292b5734e Mon Sep 17 00:00:00 2001 From: Oriol Brufau Date: Fri, 19 Sep 2025 16:20:15 +0200 Subject: [PATCH] layout: Take sticky offsets into account for offset queries (#39385) `offsetLeft` and `offsetTop` were ignoring that sticky positioned boxes can be shifted out of their normal position. Testing: Various test improvements. Signed-off-by: Oriol Brufau Co-authored-by: Martin Robinson --- .../layout/fragment_tree/box_fragment.rs | 4 +++ components/layout/layout_impl.rs | 25 ++++++++------ components/layout/query.rs | 31 ++++++++++++++--- components/shared/compositing/display_list.rs | 33 +++++++++++++------ .../sticky/position-sticky-bottom.html.ini | 6 ---- .../position-sticky-change-top.html.ini | 2 -- .../sticky/position-sticky-flexbox.html.ini | 2 -- .../position-sticky-inflow-position.html.ini | 3 -- .../sticky/position-sticky-left.html.ini | 6 ---- .../sticky/position-sticky-margins.html.ini | 3 -- .../position-sticky-nested-bottom.html.ini | 9 ----- .../position-sticky-nested-left.html.ini | 12 ------- .../position-sticky-nested-right.html.ini | 9 ----- .../position-sticky-nested-top.html.ini | 12 ------- .../position-sticky-offset-overflow.html.ini | 3 -- .../sticky/position-sticky-rendering.html.ini | 2 -- .../sticky/position-sticky-right.html.ini | 6 ---- .../position-sticky-root-scroller.html.ini | 3 -- .../position-sticky-top-and-bottom.html.ini | 6 ---- .../sticky/position-sticky-top.html.ini | 6 ---- ...oot-scroller-with-scroll-behavior.html.ini | 4 --- 21 files changed, 69 insertions(+), 118 deletions(-) delete mode 100644 tests/wpt/meta/css/css-position/sticky/position-sticky-bottom.html.ini delete mode 100644 tests/wpt/meta/css/css-position/sticky/position-sticky-change-top.html.ini delete mode 100644 tests/wpt/meta/css/css-position/sticky/position-sticky-flexbox.html.ini delete mode 100644 tests/wpt/meta/css/css-position/sticky/position-sticky-inflow-position.html.ini delete mode 100644 tests/wpt/meta/css/css-position/sticky/position-sticky-left.html.ini delete mode 100644 tests/wpt/meta/css/css-position/sticky/position-sticky-nested-bottom.html.ini delete mode 100644 tests/wpt/meta/css/css-position/sticky/position-sticky-nested-left.html.ini delete mode 100644 tests/wpt/meta/css/css-position/sticky/position-sticky-nested-right.html.ini delete mode 100644 tests/wpt/meta/css/css-position/sticky/position-sticky-nested-top.html.ini delete mode 100644 tests/wpt/meta/css/css-position/sticky/position-sticky-offset-overflow.html.ini delete mode 100644 tests/wpt/meta/css/css-position/sticky/position-sticky-rendering.html.ini delete mode 100644 tests/wpt/meta/css/css-position/sticky/position-sticky-right.html.ini delete mode 100644 tests/wpt/meta/css/css-position/sticky/position-sticky-root-scroller.html.ini delete mode 100644 tests/wpt/meta/css/css-position/sticky/position-sticky-top-and-bottom.html.ini delete mode 100644 tests/wpt/meta/css/css-position/sticky/position-sticky-top.html.ini delete mode 100644 tests/wpt/meta/css/cssom-view/position-sticky-root-scroller-with-scroll-behavior.html.ini diff --git a/components/layout/fragment_tree/box_fragment.rs b/components/layout/fragment_tree/box_fragment.rs index 62a8bf220fb..55ec947fe67 100644 --- a/components/layout/fragment_tree/box_fragment.rs +++ b/components/layout/fragment_tree/box_fragment.rs @@ -569,4 +569,8 @@ impl BoxFragment { _ => false, } } + + pub(crate) fn spatial_tree_node(&self) -> Option { + *self.spatial_tree_node.borrow() + } } diff --git a/components/layout/layout_impl.rs b/components/layout/layout_impl.rs index b9f8ffd0015..6c7c2bac3ec 100644 --- a/components/layout/layout_impl.rs +++ b/components/layout/layout_impl.rs @@ -322,7 +322,12 @@ impl Layout for LayoutThread { #[servo_tracing::instrument(skip_all)] fn query_offset_parent(&self, node: TrustedNodeAddress) -> OffsetParentResponse { let node = unsafe { ServoLayoutNode::new(&node) }; - process_offset_parent_query(node).unwrap_or_default() + let stacking_context_tree = self.stacking_context_tree.borrow(); + let stacking_context_tree = stacking_context_tree + .as_ref() + .expect("Should always have a StackingContextTree for offset parent queries"); + process_offset_parent_query(&stacking_context_tree.compositor_info.scroll_tree, node) + .unwrap_or_default() } #[servo_tracing::instrument(skip_all)] @@ -1608,26 +1613,26 @@ impl ReflowPhases { /// so [`ReflowPhases::empty()`] implies that. fn necessary(reflow_goal: &ReflowGoal) -> Self { match reflow_goal { - ReflowGoal::UpdateTheRendering | ReflowGoal::UpdateScrollNode(..) => { - Self::StackingContextTreeConstruction | Self::DisplayListConstruction - }, ReflowGoal::LayoutQuery(query) => match query { QueryMsg::NodesFromPointQuery => { Self::StackingContextTreeConstruction | Self::DisplayListConstruction }, QueryMsg::BoxArea | QueryMsg::BoxAreas | + QueryMsg::ElementsFromPoint | + QueryMsg::OffsetParentQuery | QueryMsg::ResolvedStyleQuery | - QueryMsg::ScrollingAreaOrOffsetQuery | - QueryMsg::ElementsFromPoint => Self::StackingContextTreeConstruction, + QueryMsg::ScrollingAreaOrOffsetQuery => Self::StackingContextTreeConstruction, QueryMsg::ClientRectQuery | QueryMsg::ElementInnerOuterTextQuery | QueryMsg::InnerWindowDimensionsQuery | - QueryMsg::OffsetParentQuery | - QueryMsg::ScrollParentQuery | QueryMsg::ResolvedFontStyleQuery | - QueryMsg::TextIndexQuery | - QueryMsg::StyleQuery => Self::empty(), + QueryMsg::ScrollParentQuery | + QueryMsg::StyleQuery | + QueryMsg::TextIndexQuery => Self::empty(), + }, + ReflowGoal::UpdateScrollNode(..) | ReflowGoal::UpdateTheRendering => { + Self::StackingContextTreeConstruction | Self::DisplayListConstruction }, } } diff --git a/components/layout/query.rs b/components/layout/query.rs index 4fbadb75458..4776e5f0fbe 100644 --- a/components/layout/query.rs +++ b/components/layout/query.rs @@ -64,8 +64,7 @@ fn root_transform_for_layout_node( .first() .and_then(Fragment::retrieve_box_fragment)? .borrow(); - let scroll_tree_node_id = box_fragment.spatial_tree_node.borrow(); - let scroll_tree_node_id = (*scroll_tree_node_id)?; + let scroll_tree_node_id = box_fragment.spatial_tree_node()?; Some(scroll_tree.cumulative_node_to_root_transform(scroll_tree_node_id)) } @@ -565,7 +564,10 @@ fn offset_parent_fragments(node: ServoLayoutNode<'_>) -> Option) -> Option { +pub fn process_offset_parent_query( + scroll_tree: &ScrollTree, + node: ServoLayoutNode<'_>, +) -> Option { // Only consider the first fragment of the node found as per a // possible interpretation of the specification: "[...] return the // y-coordinate of the top border edge of the first CSS layout box @@ -588,6 +590,16 @@ pub fn process_offset_parent_query(node: ServoLayoutNode<'_>) -> Option) -> Option, + cumulative_sticky_offsets: LayoutVector2D, } #[derive(Default)] struct AncestorStickyInfo { nearest_scrolling_ancestor_offset: LayoutVector2D, nearest_scrolling_ancestor_viewport: LayoutRect, + cumulative_sticky_offsets: LayoutVector2D, } #[derive(Debug, Deserialize, MallocSizeOf, Serialize)] @@ -610,14 +612,8 @@ impl ScrollTree { &self, node_id: ScrollTreeNodeId, ) -> FastLayoutTransform { - let node = self.get_node(node_id); - if let Some(cached_transforms) = node.transformation_cache.get() { - return cached_transforms.node_to_root_transform; - } - - let (transforms, _) = self.cumulative_node_transform_inner(node); - node.transformation_cache.set(Some(transforms)); - transforms.node_to_root_transform + self.cumulative_node_transform(node_id) + .node_to_root_transform } /// Find a transformation that can convert a point in the root coordinate system to a @@ -627,14 +623,29 @@ impl ScrollTree { &self, node_id: ScrollTreeNodeId, ) -> Option { + self.cumulative_node_transform(node_id) + .root_to_node_transform + } + + /// Find the cumulative offsets of sticky positioned boxes from the given node up to + /// the root. + pub fn cumulative_sticky_offsets(&self, node_id: ScrollTreeNodeId) -> LayoutVector2D { + self.cumulative_node_transform(node_id) + .cumulative_sticky_offsets + } + + fn cumulative_node_transform( + &self, + node_id: ScrollTreeNodeId, + ) -> ScrollTreeNodeTransformationCache { let node = self.get_node(node_id); if let Some(cached_transforms) = node.transformation_cache.get() { - return cached_transforms.root_to_node_transform; + return cached_transforms; } let (transforms, _) = self.cumulative_node_transform_inner(node); node.transformation_cache.set(Some(transforms)); - transforms.root_to_node_transform + transforms } /// Traverse a scroll node to its root to calculate the transform. @@ -678,6 +689,7 @@ impl ScrollTree { SpatialTreeNodeInfo::Sticky(info) => { let offset = info.calculate_sticky_offset(&sticky_info); sticky_info.nearest_scrolling_ancestor_offset += offset; + sticky_info.cumulative_sticky_offsets += offset; let offset_transform = FastLayoutTransform::Offset(offset); (offset_transform, offset_transform.inverse()) }, @@ -696,6 +708,7 @@ impl ScrollTree { let transforms = ScrollTreeNodeTransformationCache { node_to_root_transform, root_to_node_transform, + cumulative_sticky_offsets: sticky_info.cumulative_sticky_offsets, }; (transforms, sticky_info) } diff --git a/tests/wpt/meta/css/css-position/sticky/position-sticky-bottom.html.ini b/tests/wpt/meta/css/css-position/sticky/position-sticky-bottom.html.ini deleted file mode 100644 index d80c54bf5de..00000000000 --- a/tests/wpt/meta/css/css-position/sticky/position-sticky-bottom.html.ini +++ /dev/null @@ -1,6 +0,0 @@ -[position-sticky-bottom.html] - [after reaching the sticking point the sticky box should be offset] - expected: FAIL - - [the sticky box should not be pushed outside its containing block] - expected: FAIL diff --git a/tests/wpt/meta/css/css-position/sticky/position-sticky-change-top.html.ini b/tests/wpt/meta/css/css-position/sticky/position-sticky-change-top.html.ini deleted file mode 100644 index 27054021148..00000000000 --- a/tests/wpt/meta/css/css-position/sticky/position-sticky-change-top.html.ini +++ /dev/null @@ -1,2 +0,0 @@ -[position-sticky-change-top.html] - expected: FAIL diff --git a/tests/wpt/meta/css/css-position/sticky/position-sticky-flexbox.html.ini b/tests/wpt/meta/css/css-position/sticky/position-sticky-flexbox.html.ini deleted file mode 100644 index 888c40c46df..00000000000 --- a/tests/wpt/meta/css/css-position/sticky/position-sticky-flexbox.html.ini +++ /dev/null @@ -1,2 +0,0 @@ -[position-sticky-flexbox.html] - expected: FAIL diff --git a/tests/wpt/meta/css/css-position/sticky/position-sticky-inflow-position.html.ini b/tests/wpt/meta/css/css-position/sticky/position-sticky-inflow-position.html.ini deleted file mode 100644 index d8d6445bc9c..00000000000 --- a/tests/wpt/meta/css/css-position/sticky/position-sticky-inflow-position.html.ini +++ /dev/null @@ -1,3 +0,0 @@ -[position-sticky-inflow-position.html] - [sticky offset should not affect the position of other elements.] - expected: FAIL diff --git a/tests/wpt/meta/css/css-position/sticky/position-sticky-left.html.ini b/tests/wpt/meta/css/css-position/sticky/position-sticky-left.html.ini deleted file mode 100644 index 7cdf5ae2408..00000000000 --- a/tests/wpt/meta/css/css-position/sticky/position-sticky-left.html.ini +++ /dev/null @@ -1,6 +0,0 @@ -[position-sticky-left.html] - [after reaching the sticking point the sticky box should be offset] - expected: FAIL - - [the sticky box should not be pushed outside its containing block] - expected: FAIL diff --git a/tests/wpt/meta/css/css-position/sticky/position-sticky-margins.html.ini b/tests/wpt/meta/css/css-position/sticky/position-sticky-margins.html.ini index a642314b0b8..5d9b72f3895 100644 --- a/tests/wpt/meta/css/css-position/sticky/position-sticky-margins.html.ini +++ b/tests/wpt/meta/css/css-position/sticky/position-sticky-margins.html.ini @@ -1,6 +1,3 @@ [position-sticky-margins.html] - [Whilst stuck, the margin is irrelevant.] - expected: FAIL - [The margin is taken into account when making sure the sticky element does not escape its container] expected: FAIL diff --git a/tests/wpt/meta/css/css-position/sticky/position-sticky-nested-bottom.html.ini b/tests/wpt/meta/css/css-position/sticky/position-sticky-nested-bottom.html.ini deleted file mode 100644 index 869b92e5808..00000000000 --- a/tests/wpt/meta/css/css-position/sticky/position-sticky-nested-bottom.html.ini +++ /dev/null @@ -1,9 +0,0 @@ -[position-sticky-nested-bottom.html] - [both sticky boxes can be stuck at the same time] - expected: FAIL - - [neither sticky can escape their containing block] - expected: FAIL - - [the inner sticky cannot be pushed outside the outer sticky] - expected: FAIL diff --git a/tests/wpt/meta/css/css-position/sticky/position-sticky-nested-left.html.ini b/tests/wpt/meta/css/css-position/sticky/position-sticky-nested-left.html.ini deleted file mode 100644 index 8b18534983a..00000000000 --- a/tests/wpt/meta/css/css-position/sticky/position-sticky-nested-left.html.ini +++ /dev/null @@ -1,12 +0,0 @@ -[position-sticky-nested-left.html] - [the inner sticky can stick before the outer one if necessary] - expected: FAIL - - [both sticky boxes can be stuck at the same time] - expected: FAIL - - [neither sticky can escape their containing block] - expected: FAIL - - [the inner sticky cannot be pushed outside the outer sticky] - expected: FAIL diff --git a/tests/wpt/meta/css/css-position/sticky/position-sticky-nested-right.html.ini b/tests/wpt/meta/css/css-position/sticky/position-sticky-nested-right.html.ini deleted file mode 100644 index 4f13d023abc..00000000000 --- a/tests/wpt/meta/css/css-position/sticky/position-sticky-nested-right.html.ini +++ /dev/null @@ -1,9 +0,0 @@ -[position-sticky-nested-right.html] - [both sticky boxes can be stuck at the same time] - expected: FAIL - - [neither sticky can escape their containing block] - expected: FAIL - - [the inner sticky cannot be pushed outside the outer sticky] - expected: FAIL diff --git a/tests/wpt/meta/css/css-position/sticky/position-sticky-nested-top.html.ini b/tests/wpt/meta/css/css-position/sticky/position-sticky-nested-top.html.ini deleted file mode 100644 index e8831a24861..00000000000 --- a/tests/wpt/meta/css/css-position/sticky/position-sticky-nested-top.html.ini +++ /dev/null @@ -1,12 +0,0 @@ -[position-sticky-nested-top.html] - [the inner sticky can stick before the outer one if necessary] - expected: FAIL - - [both sticky boxes can be stuck at the same time] - expected: FAIL - - [neither sticky can escape their containing block] - expected: FAIL - - [the inner sticky cannot be pushed outside the outer sticky] - expected: FAIL diff --git a/tests/wpt/meta/css/css-position/sticky/position-sticky-offset-overflow.html.ini b/tests/wpt/meta/css/css-position/sticky/position-sticky-offset-overflow.html.ini deleted file mode 100644 index 71f99234e29..00000000000 --- a/tests/wpt/meta/css/css-position/sticky/position-sticky-offset-overflow.html.ini +++ /dev/null @@ -1,3 +0,0 @@ -[position-sticky-offset-overflow.html] - [sticky position offset should be contained by scrolling box] - expected: FAIL diff --git a/tests/wpt/meta/css/css-position/sticky/position-sticky-rendering.html.ini b/tests/wpt/meta/css/css-position/sticky/position-sticky-rendering.html.ini deleted file mode 100644 index 2831b2eabe7..00000000000 --- a/tests/wpt/meta/css/css-position/sticky/position-sticky-rendering.html.ini +++ /dev/null @@ -1,2 +0,0 @@ -[position-sticky-rendering.html] - expected: FAIL diff --git a/tests/wpt/meta/css/css-position/sticky/position-sticky-right.html.ini b/tests/wpt/meta/css/css-position/sticky/position-sticky-right.html.ini deleted file mode 100644 index 656dd23bb2c..00000000000 --- a/tests/wpt/meta/css/css-position/sticky/position-sticky-right.html.ini +++ /dev/null @@ -1,6 +0,0 @@ -[position-sticky-right.html] - [after reaching the sticking point the sticky box should be offset] - expected: FAIL - - [the sticky box should not be pushed outside its containing block] - expected: FAIL diff --git a/tests/wpt/meta/css/css-position/sticky/position-sticky-root-scroller.html.ini b/tests/wpt/meta/css/css-position/sticky/position-sticky-root-scroller.html.ini deleted file mode 100644 index ff105eea0d5..00000000000 --- a/tests/wpt/meta/css/css-position/sticky/position-sticky-root-scroller.html.ini +++ /dev/null @@ -1,3 +0,0 @@ -[position-sticky-root-scroller.html] - [Sticky elements work with the root (document) scroller] - expected: FAIL diff --git a/tests/wpt/meta/css/css-position/sticky/position-sticky-top-and-bottom.html.ini b/tests/wpt/meta/css/css-position/sticky/position-sticky-top-and-bottom.html.ini deleted file mode 100644 index faf1aaa3e0d..00000000000 --- a/tests/wpt/meta/css/css-position/sticky/position-sticky-top-and-bottom.html.ini +++ /dev/null @@ -1,6 +0,0 @@ -[position-sticky-top-and-bottom.html] - [initially the sticky box should be pushed to the top of the container] - expected: FAIL - - [when we scroll past the flow position the top constraint pushes it down] - expected: FAIL diff --git a/tests/wpt/meta/css/css-position/sticky/position-sticky-top.html.ini b/tests/wpt/meta/css/css-position/sticky/position-sticky-top.html.ini deleted file mode 100644 index 6941dd7a0b2..00000000000 --- a/tests/wpt/meta/css/css-position/sticky/position-sticky-top.html.ini +++ /dev/null @@ -1,6 +0,0 @@ -[position-sticky-top.html] - [after reaching the sticking point the sticky box should be offset] - expected: FAIL - - [the sticky box should not be pushed outside its containing block] - expected: FAIL diff --git a/tests/wpt/meta/css/cssom-view/position-sticky-root-scroller-with-scroll-behavior.html.ini b/tests/wpt/meta/css/cssom-view/position-sticky-root-scroller-with-scroll-behavior.html.ini deleted file mode 100644 index ee2e58e0f89..00000000000 --- a/tests/wpt/meta/css/cssom-view/position-sticky-root-scroller-with-scroll-behavior.html.ini +++ /dev/null @@ -1,4 +0,0 @@ -[position-sticky-root-scroller-with-scroll-behavior.html] - [Sticky elements work with the root (document) scroller] - expected: FAIL -