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>

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 {

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>,

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)
     }
 }

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.

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) };

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) => {

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 {

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
+
+        // "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 html_element = document.GetDocumentElement();
-
+        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> {

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",

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(..) |

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

tests/wpt/meta/css/css-position/sticky/position-sticky-hyperlink.html.ini

@@ -1,2 +0,0 @@
-[position-sticky-hyperlink.html]
-  expected: FAIL

tests/wpt/mozilla/meta/MANIFEST.json

@@ -13794,7 +13794,7 @@
      ]
     ],
     "scrollTo.html": [
-     "f1b4384e63bfc12c45c3eca5edcd98ad32a85502",
+     "b90c4e69344216372c9f118d70f4989e82a6fce7",
      [
       null,
       {}

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>