REGRESSION (r242132): Nested position:sticky elements move incorrectly
authorsimon.fraser@apple.com <simon.fraser@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 25 Apr 2019 00:46:23 +0000 (00:46 +0000)
committersimon.fraser@apple.com <simon.fraser@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 25 Apr 2019 00:46:23 +0000 (00:46 +0000)
https://bugs.webkit.org/show_bug.cgi?id=197255
rdar://problem/50137744

Reviewed by Zalan Bujtas.
Source/WebCore:

Revert to the behavior of the code before r242132, where we looked at the direct parent
scrolling tree node instead of walking up the ancestor chain to find an enclosing scrolling node.
This fixes nested sticky behavior.

Test: scrollingcoordinator/mac/nested-sticky.html

* page/scrolling/cocoa/ScrollingTreeStickyNode.mm:
(WebCore::ScrollingTreeStickyNode::applyLayerPositions):

LayoutTests:

* scrollingcoordinator/mac/nested-sticky-expected.html: Added.
* scrollingcoordinator/mac/nested-sticky.html: Added.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@244630 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/scrollingcoordinator/mac/nested-sticky-expected.html [new file with mode: 0644]
LayoutTests/scrollingcoordinator/mac/nested-sticky.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/page/scrolling/cocoa/ScrollingTreeStickyNode.mm

index 5bc7b6e..54c3303 100644 (file)
@@ -1,3 +1,14 @@
+2019-04-24  Simon Fraser  <simon.fraser@apple.com>
+
+        REGRESSION (r242132): Nested position:sticky elements move incorrectly
+        https://bugs.webkit.org/show_bug.cgi?id=197255
+        rdar://problem/50137744
+
+        Reviewed by Zalan Bujtas.
+
+        * scrollingcoordinator/mac/nested-sticky-expected.html: Added.
+        * scrollingcoordinator/mac/nested-sticky.html: Added.
+
 2019-04-24  Alicia Boya GarcĂ­a  <aboya@igalia.com>
 
         Unreviewed GTK test gardening
diff --git a/LayoutTests/scrollingcoordinator/mac/nested-sticky-expected.html b/LayoutTests/scrollingcoordinator/mac/nested-sticky-expected.html
new file mode 100644 (file)
index 0000000..2d1997a
--- /dev/null
@@ -0,0 +1,42 @@
+<!DOCTYPE html>
+<html>
+<head>
+    <style>
+        body {
+            height: 2000px;
+        }
+      
+        .outer {
+            background: blue;
+            margin-top: 120px;
+            height: 200px;
+            padding: 10px;
+        }
+
+        .sticky {
+            position: sticky;
+            position: -webkit-sticky;
+            top: 0px;
+        }
+
+       .inner {
+            padding: 10px;
+            background: orange;
+            top: 10px;
+            height: 80px;
+        }
+    </style>
+    <script>
+        function startTest()
+        {
+            document.scrollingElement.scrollTop = 2000;
+        }
+        
+        window.addEventListener('load', startTest, false);
+    </script>
+</head>
+<body>
+  <div class="outer sticky">
+    <div class="inner sticky"></div>
+</body>
+</html>
diff --git a/LayoutTests/scrollingcoordinator/mac/nested-sticky.html b/LayoutTests/scrollingcoordinator/mac/nested-sticky.html
new file mode 100644 (file)
index 0000000..c9d4ed2
--- /dev/null
@@ -0,0 +1,60 @@
+<!DOCTYPE html>
+<html>
+<head>
+    <style>
+        body {
+            height: 2000px;
+        }
+      
+        .outer {
+            background: blue;
+            margin-top: 120px;
+            height: 200px;
+            padding: 10px;
+        }
+
+        .sticky {
+            position: sticky;
+            position: -webkit-sticky;
+            top: 0px;
+        }
+
+       .inner {
+            padding: 10px;
+            background: orange;
+            top: 10px;
+            height: 80px;
+        }
+    </style>
+    <script>
+        function scrollTest()
+        {
+            eventSender.mouseMoveTo(20, 20);
+            eventSender.mouseScrollByWithWheelAndMomentumPhases(0, -1, "began", "none");
+            eventSender.mouseScrollByWithWheelAndMomentumPhases(0, -100, "changed", "none");
+            eventSender.mouseScrollByWithWheelAndMomentumPhases(0, -100, "none", "continue");
+            eventSender.mouseScrollByWithWheelAndMomentumPhases(0, -100, "none", "continue");
+            eventSender.mouseScrollByWithWheelAndMomentumPhases(0, 0, "none", "end");
+            eventSender.callAfterScrollingCompletes(() => {
+                testRunner.notifyDone();
+            });
+        }
+
+        function startTest()
+        {
+            if (window.eventSender) {
+                testRunner.waitUntilDone();
+
+                eventSender.monitorWheelEvents();
+                setTimeout(scrollTest, 0);
+            }
+        }
+        
+        window.addEventListener('load', startTest, false);
+    </script>
+</head>
+<body>
+  <div class="outer sticky">
+    <div class="inner sticky"></div>
+</body>
+</html>
index cb4cfdd..fbe82ef 100644 (file)
@@ -1,3 +1,20 @@
+2019-04-24  Simon Fraser  <simon.fraser@apple.com>
+
+        REGRESSION (r242132): Nested position:sticky elements move incorrectly
+        https://bugs.webkit.org/show_bug.cgi?id=197255
+        rdar://problem/50137744
+
+        Reviewed by Zalan Bujtas.
+        
+        Revert to the behavior of the code before r242132, where we looked at the direct parent
+        scrolling tree node instead of walking up the ancestor chain to find an enclosing scrolling node.
+        This fixes nested sticky behavior.
+
+        Test: scrollingcoordinator/mac/nested-sticky.html
+
+        * page/scrolling/cocoa/ScrollingTreeStickyNode.mm:
+        (WebCore::ScrollingTreeStickyNode::applyLayerPositions):
+
 2019-04-24  Eric Carlson  <eric.carlson@apple.com>
 
         Create AVFoundationSoftLink.{h,mm} to reduce duplicate code
index 2727fb1..404c735 100644 (file)
@@ -69,7 +69,7 @@ void ScrollingTreeStickyNode::applyLayerPositions(const FloatRect& layoutViewpor
 {
     FloatRect constrainingRect;
 
-    auto* enclosingScrollingNode = enclosingScrollingNodeIncludingSelf();
+    auto* enclosingScrollingNode = parent();
     if (is<ScrollingTreeOverflowScrollingNode>(enclosingScrollingNode))
         constrainingRect = FloatRect(downcast<ScrollingTreeOverflowScrollingNode>(*enclosingScrollingNode).currentScrollPosition(), m_constraints.constrainingRectAtLastLayout().size());
     else if (is<ScrollingTreeFrameScrollingNode>(enclosingScrollingNode))