[Scroll Anchoring] Anchoring can cause pages to scroll to negative offsets

smfr · smfr · commit 37264fe39996 · 2026-02-26T21:57:08.000-08:00
https://bugs.webkit.org/show_bug.cgi?id=308689 rdar://171221075 Reviewed by Tim Horton and Abrar Rahman Protyasha. Scroll anchoring adjustments are issued with `ScrollClamping::Clamped`, but the path through `AsyncScrollingCoordinator::requestScrollToPosition()` that applies the ScrollUpdate synchronously does not express clamping, so we need to do a manual clamp here, using `adjustScrollPositionWithinRange()` which is made virtual, and consults `allowsUnclampedScrollPosition` on ScrollView. This was seen in a (broken) scroll anchoring scenario on reddit, and resulted in missing content. Test: fast/scrolling/scroll-anchoring-constraints.html * LayoutTests/fast/dom/elementFromPoint-relative-to-viewport.html: Make it side scrollable. * LayoutTests/fast/scrolling/scroll-anchoring-constraints-expected.txt: Added. * LayoutTests/fast/scrolling/scroll-anchoring-constraints.html: Added. * LayoutTests/platform/ios/compositing/geometry/fixed-in-composited-expected.txt: * LayoutTests/platform/ios/compositing/geometry/horizontal-scroll-composited-expected.txt: * LayoutTests/platform/ios/imported/w3c/web-platform-tests/dom/events/scrolling/scrollend-event-fired-for-scrollIntoView_include=root-inline-end-block-end-behavior-auto-expected.txt: Removed. * Source/WebCore/page/scrolling/AsyncScrollingCoordinator.cpp: (WebCore::AsyncScrollingCoordinator::requestScrollToPosition): * Source/WebCore/platform/ScrollView.h: * Source/WebCore/platform/ScrollableArea.h: (WebCore::ScrollableArea::adjustScrollPositionWithinRange const): Canonical link: https://commits.webkit.org/308320@main
diff --git a/LayoutTests/fast/dom/elementFromPoint-relative-to-viewport.html b/LayoutTests/fast/dom/elementFromPoint-relative-to-viewport.html
@@ -1,6 +1,9 @@
 
 <script src="../../resources/js-test-pre.js"></script>
 <style>
+    body {
+        width: 1200px;
+    }
     .test {
         width: 100px;
         height: 100px;
diff --git a/LayoutTests/fast/scrolling/scroll-anchoring-constraints-expected.txt b/LayoutTests/fast/scrolling/scroll-anchoring-constraints-expected.txt
@@ -0,0 +1,5 @@
+PASS window.scrollY is 0
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/fast/scrolling/scroll-anchoring-constraints.html b/LayoutTests/fast/scrolling/scroll-anchoring-constraints.html
@@ -0,0 +1,58 @@
+<!DOCTYPE html>
+<html>
+<head>
+    <style>
+        body {
+            height: 5000px;
+        }
+        
+        .above-top {
+            margin-top: -2000px;
+            height: 2500px;
+            border: 1px solid black;
+        }
+        
+        #changer {
+            height: 1800px;
+            width: 100%;
+            background-color: orange;
+        }
+        
+        .anchor {
+            width: 100%;
+            height: 300px;
+            background-color: cyan;
+        }
+        
+        body.changed #changer {
+            height: 100px;
+        }
+    </style>
+    <script src="../../resources/js-test.js"></script>
+    <script src="../../resources/ui-helper.js"></script>
+    <script>
+        jsTestIsAsync = true;
+
+        window.addEventListener('load', async () => {
+            await UIHelper.delayFor(0);
+            // Need to scroll to trigger anchoring.
+            window.scrollTo(0, 10);
+
+            await UIHelper.renderingUpdate();
+            document.body.classList.add('changed');
+            
+            shouldBe('window.scrollY', '0');
+
+            window.scrollTo(0, 0);
+            finishJSTest();
+        }, false);
+    </script>
+</head>
+<body>
+    <div class="above-top">
+        <div id="changer"></div>
+        <div class="anchor"></div>
+    </div>
+    <div id="console"></div>
+</body>
+</html>
diff --git a/LayoutTests/platform/ios/compositing/geometry/fixed-in-composited-expected.txt b/LayoutTests/platform/ios/compositing/geometry/fixed-in-composited-expected.txt
@@ -16,7 +16,7 @@ layer at (0,0) size 60x60
   RenderBlock {DIV} at (0,0) size 60x60 [bgcolor=#0000FF33]
 layer at (95,145) size 50x50
   RenderBlock (positioned) zI: 1 {DIV} at (95,145) size 50x50
-layer at (70,70) size 100x100
+layer at (20,70) size 100x100
   RenderBlock (positioned) {DIV} at (20,20) size 100x100 [bgcolor=#008000]
 layer at (200,100) size 50x50
   RenderBlock (positioned) zI: 1 {DIV} at (200,100) size 50x50
@@ -28,4 +28,4 @@ layer at (400,100) size 100x100
   RenderBlock {DIV} at (0,0) size 100x100
 layer at (420,120) size 100x100
   RenderBlock (positioned) {DIV} at (20,20) size 100x100 [bgcolor=#008000]
-scrolled to 50,50
+scrolled to 0,50
diff --git a/LayoutTests/platform/ios/compositing/geometry/horizontal-scroll-composited-expected.txt b/LayoutTests/platform/ios/compositing/geometry/horizontal-scroll-composited-expected.txt
@@ -5,4 +5,4 @@ layer at (0,0) size 800x822
     RenderBody {BODY} at (8,8) size 600x806
 layer at (8,8) size 1006x806
   RenderBlock {DIV} at (0,0) size 1006x806 [border: (3px solid #FF0000)]
-scrolled to 300,0
+scrolled to 214,0
diff --git a/LayoutTests/platform/ios/imported/w3c/web-platform-tests/dom/events/scrolling/scrollend-event-fired-for-scrollIntoView_include=root-inline-end-block-end-behavior-auto-expected.txt b/LayoutTests/platform/ios/imported/w3c/web-platform-tests/dom/events/scrolling/scrollend-event-fired-for-scrollIntoView_include=root-inline-end-block-end-behavior-auto-expected.txt
diff --git a/Source/WebCore/page/scrolling/AsyncScrollingCoordinator.cpp b/Source/WebCore/page/scrolling/AsyncScrollingCoordinator.cpp
@@ -398,9 +398,13 @@ bool AsyncScrollingCoordinator::requestScrollToPosition(ScrollableArea& scrollab
     bool inProgrammaticScroll = scrollableArea.currentScrollType() == ScrollType::Programmatic;
 
     if ((inProgrammaticScroll && options.animated == ScrollIsAnimated::No) || inBackForwardCache) {
+        auto adjustedScrollPosition = scrollPosition;
+        if (options.clamping == ScrollClamping::Clamped)
+            adjustedScrollPosition = scrollableArea.adjustScrollPositionWithinRange(scrollPosition);
+
         auto scrollUpdate = ScrollUpdate {
             .nodeID = *scrollingNodeID,
-            .scrollPosition = scrollPosition,
+            .scrollPosition = adjustedScrollPosition,
             .data = ScrollUpdateData {
                 .updateType = ScrollUpdateType::PositionUpdate,
                 .updateLayerPositionAction = ScrollingLayerPositionAction::Set,
diff --git a/Source/WebCore/platform/ScrollView.h b/Source/WebCore/platform/ScrollView.h
@@ -255,7 +255,7 @@ class ScrollView : public Widget, public ScrollableArea, public CanMakeCheckedPt
     ScrollPosition maximumScrollPosition() const override; // The maximum position we can be scrolled to.
 
     // Adjust the passed in scroll position to keep it between the minimum and maximum positions.
-    ScrollPosition adjustScrollPositionWithinRange(const ScrollPosition&) const;
+    ScrollPosition adjustScrollPositionWithinRange(const ScrollPosition&) const override;
     int scrollX() const { return scrollPosition().x(); }
     int scrollY() const { return scrollPosition().y(); }
 
diff --git a/Source/WebCore/platform/ScrollableArea.h b/Source/WebCore/platform/ScrollableArea.h
@@ -276,6 +276,11 @@ class ScrollableArea : public CanMakeWeakPtr<ScrollableArea>, public AbstractCan
     WEBCORE_EXPORT virtual ScrollPosition minimumScrollPosition() const;
     WEBCORE_EXPORT virtual ScrollPosition maximumScrollPosition() const;
 
+    virtual ScrollPosition adjustScrollPositionWithinRange(const ScrollPosition& position) const
+    {
+        return constrainedScrollPosition(position);
+    }
+
     ScrollPosition constrainedScrollPosition(const ScrollPosition& position) const
     {
         return position.constrainedBetween(minimumScrollPosition(), maximumScrollPosition());
