script: Prevent "scroll to fragment" from scrolling offscreen (#32129)

Previously, the "scroll to fragment" operation could scroll past the end
of the screen, because the scroll position was not clamped to viewport
boundaries. Correct this by using the `Window::scroll()` method which
handles this case.

In addition, ensure that `Window`'s `current_viewport` member is
initialized properly when it is created.

components/script/dom/document.rs

@@ -980,21 +980,20 @@ impl Document {
         let point = target
             .as_ref()
             .map(|element| {
-                // FIXME(#8275, pcwalton): This is pretty bogus when multiple layers are involved.
+                // TODO: This strategy is completely wrong if the element we are scrolling to in
-                // Really what needs to happen is that this needs to go through layout to ask which
+                // inside other scrollable containers. Ideally this should use an implementation of
-                // layer the element belongs to, and have it send the scroll message to the
+                // `scrollIntoView` when that is available:
-                // compositor.
+                // See https://github.com/servo/servo/issues/24059.
                 let rect = element.upcast::<Node>().bounding_content_box_or_zero();
 
                 // In order to align with element edges, we snap to unscaled pixel boundaries, since
                 // the paint thread currently does the same for drawing elements. This is important
                 // for pages that require pixel perfect scroll positioning for proper display
-                // (like Acid2). Since we don't have the device pixel ratio here, this might not be
+                // (like Acid2).
-                // accurate, but should work as long as the ratio is a whole number. Once #8275 is
+                let device_pixel_ratio = self.window.device_pixel_ratio().get();
-                // fixed this should actually take into account the real device pixel ratio.
                 (
-                    rect.origin.x.to_nearest_px() as f32,
+                    rect.origin.x.to_nearest_pixel(device_pixel_ratio) as f32,
-                    rect.origin.y.to_nearest_px() as f32,
+                    rect.origin.y.to_nearest_pixel(device_pixel_ratio) as f32,
                 )
             })
             .or_else(|| {
@@ -1008,16 +1007,8 @@ impl Document {
             });
 
         if let Some((x, y)) = point {
-            // Step 3
+            self.window
-            let global_scope = self.window.upcast::<GlobalScope>();
+                .scroll(x as f64, y as f64, ScrollBehavior::Instant)
-            self.window.update_viewport_for_scroll(x, y);
-            self.window.perform_a_scroll(
-                x,
-                y,
-                global_scope.pipeline_id().root_scroll_id(),
-                ScrollBehavior::Instant,
-                target.as_deref(),
-            );
         }
     }
 

components/script/dom/window.rs

@@ -2560,6 +2560,12 @@ impl Window {
             pipelineid,
             script_chan: Arc::new(Mutex::new(control_chan)),
         };
+
+        let initial_viewport = f32_rect_to_au_rect(UntypedRect::new(
+            Point2D::zero(),
+            window_size.initial_viewport.to_untyped(),
+        ));
+
         let win = Box::new(Self {
             globalscope: GlobalScope::new_inherited(
                 pipelineid,
@@ -2602,7 +2608,7 @@ impl Window {
             page_clip_rect: Cell::new(MaxRect::max_rect()),
             resize_event: Default::default(),
             window_size: Cell::new(window_size),
-            current_viewport: Cell::new(Rect::zero()),
+            current_viewport: Cell::new(initial_viewport.to_untyped()),
             suppress_reflow: Cell::new(true),
             pending_reflow_count: Default::default(),
             current_state: Cell::new(WindowState::Alive),

tests/wpt/meta-legacy-layout/html/browsers/browsing-the-web/scroll-to-fragid/scroll-position-vertical-lr.html.ini

@@ -0,0 +1,3 @@
+[scroll-position-vertical-lr.html]
+  [Fragment Navigation: Scroll to block start position in vertical-lr writing mode]
+    expected: FAIL

tests/wpt/meta/MANIFEST.json

@@ -621660,6 +621660,13 @@
          }
         ]
        ],
+       "scroll-position-inline-nearest.html": [
+        "4aab0aa5e5a1e4a3eea82282be2344586a9a6d02",
+        [
+         null,
+         {}
+        ]
+       ],
        "scroll-position-vertical-lr.html": [
         "57d99440e114968e7dcd1b61ebf2d18c7bca987b",
         [

tests/wpt/meta/html/browsers/browsing-the-web/scroll-to-fragid/scroll-position-vertical-lr.html.ini

@@ -0,0 +1,3 @@
+[scroll-position-vertical-lr.html]
+  [Fragment Navigation: Scroll to block start position in vertical-lr writing mode]
+    expected: FAIL

tests/wpt/tests/html/browsers/browsing-the-web/scroll-to-fragid/scroll-position-inline-nearest.html

@@ -0,0 +1,30 @@
+<!DOCTYPE html>
+<html style="writing-mode: vertical-lr;">
+<head>
+<meta charset="UTF-8">
+<title>Fragment Navigation: inline start position should not scroll out of content range</title>
+<link rel="help" href="https://html.spec.whatwg.org/multipage/#scroll-to-the-fragment-identifier">
+<link rel="author" href="mailto:mrobinson@igalia.com" title="Martin Robinson">
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+</head>
+<body>
+    <!-- When scrolling to this fragment the viewport inline position should not
+        change because, it is already fully enclosed by the viewport and page width. -->
+    <div id="test1" style="position: absolute; top: 5000px; left: 100px; height: 100px; width: 100px;"></div>
+<script>
+
+var t = async_test("ScrollToFragment");
+t.step(() => {
+    location.hash = "test1";
+    setTimeout(t.step_func(() => {
+        assert_true(window.scrollY > 0);
+        assert_true(window.scrollY < 5000);
+        assert_equals(window.scrollX, 0);
+        t.done();
+    }));
+});
+
+</script>
+</body>
+</html>