From 2c7866eb24ae347f04b045ae4031a9bf502ac20d Mon Sep 17 00:00:00 2001 From: Martin Robinson Date: Wed, 3 Sep 2025 11:52:15 -0700 Subject: [PATCH] script/layout: Implement `HTMLElement.scrollParent` (#39110) This new API allows getting the element which establishes an element's scroll container. This will be used to properly implement `scrollIntoView`. There is still work to do for this API and `offsetParent` to properly handle ancestors which are closed-shadow-hidden from the original query element. In addition, fix an issue where inline boxes were establishing scrolling containers (they shouldn't do that). Testing: There are tests for this change. Fixes: #39096. Signed-off-by: Martin Robinson Co-authored-by: Oriol Brufau --- components/layout/layout_impl.rs | 13 ++- components/layout/query.rs | 94 ++++++++++++++++++- components/layout/style_ext.rs | 8 ++ components/script/dom/html/htmlelement.rs | 5 + components/script/dom/window.rs | 21 ++++- .../webidls/HTMLElement.webidl | 1 + components/shared/layout/lib.rs | 8 ++ tests/wpt/meta/MANIFEST.json | 15 ++- .../meta/css/cssom-view/idlharness.html.ini | 9 -- .../scrollParent-shadow-tree.html.ini | 6 -- .../meta/css/cssom-view/scrollParent.html.ini | 27 ------ ...overflow-does-not-apply-to-inline-box.html | 12 +++ .../tests/css/cssom-view/scrollParent.html | 3 +- 13 files changed, 173 insertions(+), 49 deletions(-) delete mode 100644 tests/wpt/meta/css/cssom-view/scrollParent.html.ini create mode 100644 tests/wpt/tests/css/css-overflow/overflow-does-not-apply-to-inline-box.html diff --git a/components/layout/layout_impl.rs b/components/layout/layout_impl.rs index 92bf50cb39c..9d631766ee5 100644 --- a/components/layout/layout_impl.rs +++ b/components/layout/layout_impl.rs @@ -29,7 +29,8 @@ use layout_api::wrapper_traits::LayoutNode; use layout_api::{ BoxAreaType, IFrameSizes, Layout, LayoutConfig, LayoutDamage, LayoutFactory, OffsetParentResponse, PropertyRegistration, QueryMsg, ReflowGoal, ReflowPhasesRun, - ReflowRequest, ReflowRequestRestyle, ReflowResult, RegisterPropertyError, TrustedNodeAddress, + ReflowRequest, ReflowRequestRestyle, ReflowResult, RegisterPropertyError, ScrollParentResponse, + TrustedNodeAddress, }; use log::{debug, error, warn}; use malloc_size_of::{MallocConditionalSizeOf, MallocSizeOf, MallocSizeOfOps}; @@ -91,7 +92,8 @@ use crate::display_list::{DisplayListBuilder, HitTest, StackingContextTree}; use crate::query::{ get_the_text_steps, process_box_area_request, process_box_areas_request, process_client_rect_request, process_node_scroll_area_request, process_offset_parent_query, - process_resolved_font_style_query, process_resolved_style_request, process_text_index_request, + process_resolved_font_style_query, process_resolved_style_request, process_scroll_parent_query, + process_text_index_request, }; use crate::traversal::{RecalcStyle, compute_damage_and_repair_style}; use crate::{BoxTree, FragmentTree}; @@ -323,6 +325,12 @@ impl Layout for LayoutThread { process_offset_parent_query(node).unwrap_or_default() } + #[servo_tracing::instrument(skip_all)] + fn query_scroll_parent(&self, node: TrustedNodeAddress) -> Option { + let node = unsafe { ServoLayoutNode::new(&node) }; + process_scroll_parent_query(node) + } + #[servo_tracing::instrument(skip_all)] fn query_resolved_style( &self, @@ -1608,6 +1616,7 @@ impl ReflowPhases { QueryMsg::ElementInnerOuterTextQuery | QueryMsg::InnerWindowDimensionsQuery | QueryMsg::OffsetParentQuery | + QueryMsg::ScrollParentQuery | QueryMsg::ResolvedFontStyleQuery | QueryMsg::TextIndexQuery | QueryMsg::StyleQuery => Self::empty(), diff --git a/components/layout/query.rs b/components/layout/query.rs index aef2691177b..c8b0348ef79 100644 --- a/components/layout/query.rs +++ b/components/layout/query.rs @@ -11,7 +11,9 @@ use euclid::default::{Point2D, Rect}; use euclid::{SideOffsets2D, Size2D}; use itertools::Itertools; use layout_api::wrapper_traits::{LayoutNode, ThreadSafeLayoutElement, ThreadSafeLayoutNode}; -use layout_api::{BoxAreaType, LayoutElementType, LayoutNodeType, OffsetParentResponse}; +use layout_api::{ + BoxAreaType, LayoutElementType, LayoutNodeType, OffsetParentResponse, ScrollParentResponse, +}; use script::layout_dom::{ServoLayoutNode, ServoThreadSafeLayoutNode}; use servo_arc::Arc as ServoArc; use servo_geometry::{FastLayoutTransform, au_rect_to_f32_rect, f32_rect_to_au_rect}; @@ -47,6 +49,7 @@ use crate::flow::inline::construct::{TextTransformation, WhitespaceCollapse, cap use crate::fragment_tree::{ BoxFragment, Fragment, FragmentFlags, FragmentTree, SpecificLayoutInfo, }; +use crate::style_ext::ComputedValuesExt; use crate::taffy::SpecificTaffyGridInfo; /// Get a scroll node that would represents this [`ServoLayoutNode`]'s transform and @@ -638,6 +641,95 @@ pub fn process_offset_parent_query(node: ServoLayoutNode<'_>) -> Option. +#[inline] +pub(crate) fn process_scroll_parent_query( + node: ServoLayoutNode<'_>, +) -> Option { + let layout_data = node.to_threadsafe().inner_layout_data()?; + + // 1. If any of the following holds true, return null and terminate this algorithm: + // - The element does not have an associated box. + let layout_box = layout_data.self_box.borrow(); + let layout_box = layout_box.as_ref()?; + + let (mut current_position_value, flags) = layout_box + .with_base_flat(|base| vec![(base.style.clone_position(), base.base_fragment_info.flags)]) + .first() + .cloned()?; + + // - The element is the root element. + // - The element is the body element. + // - The element’s computed value of the position property is fixed and no ancestor + // establishes a fixed position containing block. + if flags.intersects( + FragmentFlags::IS_ROOT_ELEMENT | FragmentFlags::IS_BODY_ELEMENT_OF_HTML_ELEMENT_ROOT, + ) { + return None; + } + + // 2. Let ancestor be the containing block of the element in the flat tree and repeat these substeps: + // - If ancestor is the initial containing block, return the scrollingElement for the + // element’s document if it is not closed-shadow-hidden from the element, otherwise + // return null. + // - If ancestor is not closed-shadow-hidden from the element, and is a scroll + // container, terminate this algorithm and return ancestor. + // - If the computed value of the position property of ancestor is fixed, and no + // ancestor establishes a fixed position containing block, terminate this algorithm + // and return null. + // - Let ancestor be the containing block of ancestor in the flat tree. + // + // Notes: We don't follow the specification exactly below, but we follow the spirit. + // + // TODO: Handle the situation where the ancestor is "closed-shadow-hidden" from the element. + let mut current_ancestor = node.as_element()?; + while let Some(ancestor) = current_ancestor.traversal_parent() { + current_ancestor = ancestor; + + let Some(layout_data) = ancestor.as_node().to_threadsafe().inner_layout_data() else { + continue; + }; + let ancestor_layout_box = layout_data.self_box.borrow(); + let Some(ancestor_layout_box) = ancestor_layout_box.as_ref() else { + continue; + }; + + let (ancestor_style, ancestor_flags) = ancestor_layout_box + .with_base_flat(|base| vec![(base.style.clone(), base.base_fragment_info.flags)]) + .first() + .cloned()?; + + let is_containing_block = match current_position_value { + Position::Static | Position::Relative | Position::Sticky => { + !ancestor_style.is_inline_box(ancestor_flags) + }, + Position::Absolute => { + ancestor_style.establishes_containing_block_for_absolute_descendants(ancestor_flags) + }, + Position::Fixed => { + ancestor_style.establishes_containing_block_for_all_descendants(ancestor_flags) + }, + }; + if !is_containing_block { + continue; + } + + if ancestor_style.establishes_scroll_container(ancestor_flags) { + return Some(ScrollParentResponse::Element( + ancestor.as_node().opaque().into(), + )); + } + + current_position_value = ancestor_style.clone_position(); + } + + match current_position_value { + Position::Fixed => None, + _ => Some(ScrollParentResponse::DocumentScrollingElement), + } +} + /// pub fn get_the_text_steps(node: ServoLayoutNode<'_>) -> String { // Step 1: If element is not being rendered or if the user agent is a non-CSS user agent, then diff --git a/components/layout/style_ext.rs b/components/layout/style_ext.rs index 38a0b5424a1..8849d3d9f09 100644 --- a/components/layout/style_ext.rs +++ b/components/layout/style_ext.rs @@ -598,6 +598,14 @@ impl ComputedValuesExt for ComputedValues { let mut overflow_x = style_box.overflow_x; let mut overflow_y = style_box.overflow_y; + // Inline boxes should never establish scroll containers. + if self.is_inline_box(fragment_flags) { + return AxesOverflow { + x: Overflow::Visible, + y: Overflow::Visible, + }; + } + // https://www.w3.org/TR/css-overflow-3/#overflow-propagation // The element from which the value is propagated must then have a used overflow value of visible. if fragment_flags.contains(FragmentFlags::PROPAGATED_OVERFLOW_TO_VIEWPORT) { diff --git a/components/script/dom/html/htmlelement.rs b/components/script/dom/html/htmlelement.rs index 35691bec57e..227e9ec5731 100644 --- a/components/script/dom/html/htmlelement.rs +++ b/components/script/dom/html/htmlelement.rs @@ -444,6 +444,11 @@ impl HTMLElementMethods for HTMLElement { document.request_focus(None, FocusInitiator::Local, can_gc); } + /// + fn GetScrollParent(&self) -> Option> { + self.owner_window().scroll_parent_query(self.upcast()) + } + /// fn GetOffsetParent(&self) -> Option> { if self.is_body_element() || self.is::() { diff --git a/components/script/dom/window.rs b/components/script/dom/window.rs index 84d3f3eb15b..b09f024352d 100644 --- a/components/script/dom/window.rs +++ b/components/script/dom/window.rs @@ -56,8 +56,8 @@ use js::rust::{ use layout_api::{ BoxAreaType, ElementsFromPointFlags, ElementsFromPointResult, FragmentType, Layout, PendingImage, PendingImageState, PendingRasterizationImage, QueryMsg, ReflowGoal, - ReflowPhasesRun, ReflowRequest, ReflowRequestRestyle, RestyleReason, TrustedNodeAddress, - combine_id_with_fragment_type, + ReflowPhasesRun, ReflowRequest, ReflowRequestRestyle, RestyleReason, ScrollParentResponse, + TrustedNodeAddress, combine_id_with_fragment_type, }; use malloc_size_of::MallocSizeOf; use media::WindowGLContext; @@ -2599,6 +2599,23 @@ impl Window { (element, response.rect) } + #[allow(unsafe_code)] + pub(crate) fn scroll_parent_query(&self, node: &Node) -> Option> { + self.layout_reflow(QueryMsg::ScrollParentQuery); + self.layout + .borrow() + .query_scroll_parent(node.to_trusted_node_address()) + .and_then(|response| match response { + ScrollParentResponse::DocumentScrollingElement => { + self.Document().GetScrollingElement() + }, + ScrollParentResponse::Element(parent_node_address) => { + let node = unsafe { from_untrusted_node_address(parent_node_address) }; + DomRoot::downcast(node) + }, + }) + } + pub(crate) fn text_index_query( &self, node: &Node, diff --git a/components/script_bindings/webidls/HTMLElement.webidl b/components/script_bindings/webidls/HTMLElement.webidl index 19ee9619d44..1b2a0c3c578 100644 --- a/components/script_bindings/webidls/HTMLElement.webidl +++ b/components/script_bindings/webidls/HTMLElement.webidl @@ -64,6 +64,7 @@ interface HTMLElement : Element { // http://dev.w3.org/csswg/cssom-view/#extensions-to-the-htmlelement-interface partial interface HTMLElement { // CSSOM things are not [Pure] because they can flush + readonly attribute Element? scrollParent; readonly attribute Element? offsetParent; readonly attribute long offsetTop; readonly attribute long offsetLeft; diff --git a/components/shared/layout/lib.rs b/components/shared/layout/lib.rs index eb91c1657ce..4d1c4b661f9 100644 --- a/components/shared/layout/lib.rs +++ b/components/shared/layout/lib.rs @@ -295,6 +295,7 @@ pub trait Layout { fn query_client_rect(&self, node: TrustedNodeAddress) -> Rect; fn query_element_inner_outer_text(&self, node: TrustedNodeAddress) -> String; fn query_offset_parent(&self, node: TrustedNodeAddress) -> OffsetParentResponse; + fn query_scroll_parent(&self, node: TrustedNodeAddress) -> Option; fn query_resolved_style( &self, node: TrustedNodeAddress, @@ -351,6 +352,12 @@ pub struct OffsetParentResponse { pub rect: Rect, } +#[derive(Clone)] +pub enum ScrollParentResponse { + DocumentScrollingElement, + Element(UntrustedNodeAddress), +} + #[derive(Debug, PartialEq)] pub enum QueryMsg { BoxArea, @@ -361,6 +368,7 @@ pub enum QueryMsg { InnerWindowDimensionsQuery, NodesFromPointQuery, OffsetParentQuery, + ScrollParentQuery, ResolvedFontStyleQuery, ResolvedStyleQuery, ScrollingAreaOrOffsetQuery, diff --git a/tests/wpt/meta/MANIFEST.json b/tests/wpt/meta/MANIFEST.json index 88d67ba6bff..a1a29c03ac7 100644 --- a/tests/wpt/meta/MANIFEST.json +++ b/tests/wpt/meta/MANIFEST.json @@ -232173,6 +232173,19 @@ {} ] ], + "overflow-does-not-apply-to-inline-box.html": [ + "f4c8a1816399ed368fb219bd1a4e49f61f18d083", + [ + null, + [ + [ + "/css/reference/ref-filled-green-100px-square.xht", + "==" + ] + ], + {} + ] + ], "overflow-ellipsis-dynamic-001.html": [ "2a9edba9308bf06009d7b9a27f21f1e0f1a231c7", [ @@ -630023,7 +630036,7 @@ ] ], "scrollParent.html": [ - "9faa0c9a3e92e38662ac77d25b08731fe00a7db3", + "344ee522ef2664e0b963e79812074f08f343f5bf", [ null, {} diff --git a/tests/wpt/meta/css/cssom-view/idlharness.html.ini b/tests/wpt/meta/css/cssom-view/idlharness.html.ini index 382b2929670..5aa865ec699 100644 --- a/tests/wpt/meta/css/cssom-view/idlharness.html.ini +++ b/tests/wpt/meta/css/cssom-view/idlharness.html.ini @@ -574,12 +574,3 @@ [Document interface: calling caretPositionFromPoint(double, double, optional CaretPositionFromPointOptions) on document with too few arguments must throw TypeError] expected: FAIL - - [HTMLElement interface: attribute scrollParent] - expected: FAIL - - [HTMLElement interface: document.createElement("div") must inherit property "scrollParent" with the proper type] - expected: FAIL - - [HTMLElement interface: document.createElement("img") must inherit property "scrollParent" with the proper type] - expected: FAIL diff --git a/tests/wpt/meta/css/cssom-view/scrollParent-shadow-tree.html.ini b/tests/wpt/meta/css/cssom-view/scrollParent-shadow-tree.html.ini index c528070783b..35228c8b055 100644 --- a/tests/wpt/meta/css/cssom-view/scrollParent-shadow-tree.html.ini +++ b/tests/wpt/meta/css/cssom-view/scrollParent-shadow-tree.html.ini @@ -4,9 +4,3 @@ [scrollParent skips intermediate open shadow tree nodes] expected: FAIL - - [scrollParent from inside closed shadow tree] - expected: FAIL - - [scrollParent from inside open shadow tree] - expected: FAIL diff --git a/tests/wpt/meta/css/cssom-view/scrollParent.html.ini b/tests/wpt/meta/css/cssom-view/scrollParent.html.ini deleted file mode 100644 index 9c1cf262f94..00000000000 --- a/tests/wpt/meta/css/cssom-view/scrollParent.html.ini +++ /dev/null @@ -1,27 +0,0 @@ -[scrollParent.html] - [scrollParent returns the nearest scroll container.] - expected: FAIL - - [hidden element is a scroll container.] - expected: FAIL - - [Element with no box has null scrollParent.] - expected: FAIL - - [scrollParent follows absolute positioned containing block chain.] - expected: FAIL - - [scrollParent follows fixed positioned containing block chain.] - expected: FAIL - - [scrollParent of element fixed to root is null.] - expected: FAIL - - [scrollParent of child in root viewport returns document scrolling element.] - expected: FAIL - - [scrollParent of fixed element contained within root is document scrolling element.] - expected: FAIL - - [scrollParent of body is null.] - expected: FAIL diff --git a/tests/wpt/tests/css/css-overflow/overflow-does-not-apply-to-inline-box.html b/tests/wpt/tests/css/css-overflow/overflow-does-not-apply-to-inline-box.html new file mode 100644 index 00000000000..f4c8a181639 --- /dev/null +++ b/tests/wpt/tests/css/css-overflow/overflow-does-not-apply-to-inline-box.html @@ -0,0 +1,12 @@ + + + + + + + +

Test passes if there is a filled green square and no red.

+
+ X +
+ diff --git a/tests/wpt/tests/css/cssom-view/scrollParent.html b/tests/wpt/tests/css/cssom-view/scrollParent.html index 9faa0c9a3e9..344ee522ef2 100644 --- a/tests/wpt/tests/css/cssom-view/scrollParent.html +++ b/tests/wpt/tests/css/cssom-view/scrollParent.html @@ -29,6 +29,7 @@ display: none; } .contains-fixed { + transform: scale(1); contain: paint; } @@ -83,4 +84,4 @@ test(() => { assert_equals(document.body.scrollParent, null); }, "scrollParent of body is null."); - \ No newline at end of file +