From 3774ef00d48fb9a6fdcb87fcea4750ba3c5fe147 Mon Sep 17 00:00:00 2001 From: Martin Robinson Date: Fri, 20 Jun 2025 11:39:12 +0200 Subject: [PATCH] script: Get scroll offsets from layout (#37509) No longer store scroll offsets for elements in the DOM. Instead consistently get and set these in layout's `ScrollTree`. This more consistently requires layout to run when querying scroll offsets, which ensures that they are up-to-date and properly bounded by scrollable overflow area. Testing: This causes several WPT tests to start passing, and one to start failing. In the case of `/shadow-dom/scroll-to-the-fragment-in-shadow-tree.html`, I believe the issue is that we don't properly handle scrolling and shadow DOM elements. Before, the faulty scrolling was hiding this issue. Signed-off-by: Martin Robinson --- components/layout/layout_impl.rs | 10 +- components/script/dom/element.rs | 4 +- components/script/dom/mouseevent.rs | 9 +- components/script/dom/node.rs | 8 +- components/script/dom/window.rs | 115 +++++++++--------- components/script/script_thread.rs | 17 +-- components/shared/compositing/display_list.rs | 9 ++ components/shared/script_layout/lib.rs | 10 +- ...lpos-percentage-top-in-scrollable.html.ini | 3 - .../incremental-scroll-002.html.ini | 2 - .../css/cssom-view/elementScroll.html.ini | 3 - .../css/cssom-view/scrollLeftTop.html.ini | 7 -- .../scrolling-quirks-vs-nonquirks.html.ini | 9 -- ...ll-to-the-fragment-in-shadow-tree.html.ini | 27 ++++ 14 files changed, 116 insertions(+), 117 deletions(-) delete mode 100644 tests/wpt/meta/css/CSS2/positioning/relpos-percentage-top-in-scrollable.html.ini delete mode 100644 tests/wpt/meta/css/css-overflow/incremental-scroll-002.html.ini delete mode 100644 tests/wpt/meta/css/cssom-view/elementScroll.html.ini create mode 100644 tests/wpt/meta/shadow-dom/scroll-to-the-fragment-in-shadow-tree.html.ini diff --git a/components/layout/layout_impl.rs b/components/layout/layout_impl.rs index 4188da443c4..5cfd23351c7 100644 --- a/components/layout/layout_impl.rs +++ b/components/layout/layout_impl.rs @@ -472,6 +472,14 @@ impl Layout for LayoutThread { .scroll_tree .set_all_scroll_offsets(scroll_states); } + + fn scroll_offset(&self, id: ExternalScrollId) -> Option { + self.stacking_context_tree + .borrow_mut() + .as_mut() + .and_then(|tree| tree.compositor_info.scroll_tree.scroll_offset(id)) + .map(|scroll_offset| -scroll_offset) + } } impl LayoutThread { @@ -996,7 +1004,7 @@ impl LayoutThread { .scroll_tree .set_scroll_offset_for_node_with_external_scroll_id( external_scroll_id, - offset, + -offset, ScrollType::Script, ) { diff --git a/components/script/dom/element.rs b/components/script/dom/element.rs index d0e4892b4de..58129470860 100644 --- a/components/script/dom/element.rs +++ b/components/script/dom/element.rs @@ -3080,7 +3080,7 @@ impl ElementMethods for Element { } // Step 9 - let point = node.scroll_offset(); + let point = win.scroll_offset_query(node, can_gc); point.y.abs() as f64 } @@ -3179,7 +3179,7 @@ impl ElementMethods for Element { } // Step 9 - let point = node.scroll_offset(); + let point = win.scroll_offset_query(node, can_gc); point.x.abs() as f64 } diff --git a/components/script/dom/mouseevent.rs b/components/script/dom/mouseevent.rs index 63752e6eb0a..657101b6681 100644 --- a/components/script/dom/mouseevent.rs +++ b/components/script/dom/mouseevent.rs @@ -9,6 +9,7 @@ use dom_struct::dom_struct; use embedder_traits::CompositorHitTestResult; use euclid::default::Point2D; use js::rust::HandleObject; +use script_bindings::codegen::GenericBindings::WindowBinding::WindowMethods; use servo_config::pref; use crate::dom::bindings::codegen::Bindings::EventBinding::Event_Binding::EventMethods; @@ -373,9 +374,7 @@ impl MouseEventMethods for MouseEvent { if self.upcast::().dispatching() { self.page_x.get() } else { - let global = self.global(); - let window = global.as_window(); - window.current_viewport().origin.x.to_px() + self.client_x.get() + self.global().as_window().ScrollX() + self.client_x.get() } } @@ -384,9 +383,7 @@ impl MouseEventMethods for MouseEvent { if self.upcast::().dispatching() { self.page_y.get() } else { - let global = self.global(); - let window = global.as_window(); - window.current_viewport().origin.y.to_px() + self.client_y.get() + self.global().as_window().ScrollY() + self.client_y.get() } } diff --git a/components/script/dom/node.rs b/components/script/dom/node.rs index 798d7963e5d..bb16082d310 100644 --- a/components/script/dom/node.rs +++ b/components/script/dom/node.rs @@ -18,7 +18,7 @@ use bitflags::bitflags; use devtools_traits::NodeInfo; use dom_struct::dom_struct; use embedder_traits::UntrustedNodeAddress; -use euclid::default::{Rect, Size2D, Vector2D}; +use euclid::default::{Rect, Size2D}; use html5ever::serialize::HtmlSerializer; use html5ever::{Namespace, Prefix, QualName, ns, serialize as html_serialize}; use js::jsapi::JSObject; @@ -980,12 +980,6 @@ impl Node { window.scrolling_area_query(Some(self), can_gc) } - pub(crate) fn scroll_offset(&self) -> Vector2D { - let document = self.owner_doc(); - let window = document.window(); - window.scroll_offset_query(self).to_untyped() - } - /// pub(crate) fn before(&self, nodes: Vec, can_gc: CanGc) -> ErrorResult { // Step 1. diff --git a/components/script/dom/window.rs b/components/script/dom/window.rs index cc19982cd10..c4c4cfe71c0 100644 --- a/components/script/dom/window.rs +++ b/components/script/dom/window.rs @@ -36,8 +36,8 @@ use embedder_traits::{ GamepadUpdateType, PromptResponse, SimpleDialog, Theme, ViewportDetails, WebDriverJSError, WebDriverJSResult, }; -use euclid::default::{Point2D as UntypedPoint2D, Rect as UntypedRect}; -use euclid::{Point2D, Rect, Scale, Size2D, Vector2D}; +use euclid::default::{Point2D as UntypedPoint2D, Rect as UntypedRect, Size2D as UntypedSize2D}; +use euclid::{Point2D, Scale, Size2D, Vector2D}; use fonts::FontContext; use ipc_channel::ipc::{self, IpcSender}; use js::conversions::ToJSValConvertible; @@ -77,7 +77,6 @@ use servo_arc::Arc as ServoArc; use servo_config::{opts, pref}; use servo_geometry::{DeviceIndependentIntRect, f32_rect_to_au_rect}; use servo_url::{ImmutableOrigin, MutableOrigin, ServoUrl}; -use style::dom::OpaqueNode; use style::error_reporting::{ContextualParseError, ParseErrorReporter}; use style::properties::PropertyId; use style::properties::style_structs::Font; @@ -311,15 +310,13 @@ pub(crate) struct Window { /// The current state of the window object current_state: Cell, + /// The current size of the viewport. This might change if the `WebView` or containing `