[iOS WK2] Wrong scrolling behavior for nested absolute position elements inside overf...
authorsimon.fraser@apple.com <simon.fraser@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 12 Apr 2019 14:54:12 +0000 (14:54 +0000)
committersimon.fraser@apple.com <simon.fraser@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 12 Apr 2019 14:54:12 +0000 (14:54 +0000)
https://bugs.webkit.org/show_bug.cgi?id=196146

Reviewed by Antti Koivisto.

Source/WebCore:

computeCoordinatedPositioningForLayer() failed to handle nested positions elements
inside overflow scroll, because it only walked up to the first containing block of
a nested position:absolute. We need to walk all the way up the ancestor layer chain,
looking at containing block, scroller and composited ancestor relationships.

Make this code easier to understand by writing it in terms of "is foo scrolled by bar", rather than
trying to collapse all the logic into a single ancestor walk, which was really hard. This is a few
more ancestor traversals, but we now only run this code if there's composited scrolling
in the ancestor chain.

Tests: scrollingcoordinator/scrolling-tree/nested-absolute-in-absolute-overflow.html
       scrollingcoordinator/scrolling-tree/nested-absolute-in-overflow.html
       scrollingcoordinator/scrolling-tree/nested-absolute-in-relative-in-overflow.html
       scrollingcoordinator/scrolling-tree/nested-absolute-in-sc-overflow.html

* rendering/RenderLayerCompositor.cpp:
(WebCore::enclosingCompositedScrollingLayer):
(WebCore::isScrolledByOverflowScrollLayer):
(WebCore::isNonScrolledLayerInsideScrolledCompositedAncestor):
(WebCore::RenderLayerCompositor::layerContainingBlockCrossesCoordinatedScrollingBoundary):
(WebCore::collectStationaryLayerRelatedOverflowNodes):
(WebCore::RenderLayerCompositor::computeCoordinatedPositioningForLayer const):
(WebCore::collectRelatedCoordinatedScrollingNodes):
(WebCore::layerParentedAcrossCoordinatedScrollingBoundary): Deleted.

LayoutTests:

Dump the scrolling tree for various configurations of positioned, overflow and stacking context
elements.

* fast/scrolling/ios/overflow-scroll-overlap-6-expected.txt: Progressed results.
* platform/ios-wk2/scrollingcoordinator/scrolling-tree/nested-absolute-in-absolute-overflow-expected.txt: Added.
* platform/ios-wk2/scrollingcoordinator/scrolling-tree/nested-absolute-in-overflow-expected.txt: Added.
* platform/ios-wk2/scrollingcoordinator/scrolling-tree/nested-absolute-in-relative-in-overflow-expected.txt: Added.
* platform/ios-wk2/scrollingcoordinator/scrolling-tree/nested-absolute-in-sc-overflow-expected.txt: Added.
* scrollingcoordinator/scrolling-tree/nested-absolute-in-absolute-overflow-expected.txt: Added.
* scrollingcoordinator/scrolling-tree/nested-absolute-in-absolute-overflow.html: Added.
* scrollingcoordinator/scrolling-tree/nested-absolute-in-overflow-expected.txt: Added.
* scrollingcoordinator/scrolling-tree/nested-absolute-in-overflow.html: Added.
* scrollingcoordinator/scrolling-tree/nested-absolute-in-relative-in-overflow-expected.txt: Added.
* scrollingcoordinator/scrolling-tree/nested-absolute-in-relative-in-overflow.html: Added.
* scrollingcoordinator/scrolling-tree/nested-absolute-in-sc-overflow-expected.txt: Added.
* scrollingcoordinator/scrolling-tree/nested-absolute-in-sc-overflow.html: Added.

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

16 files changed:
LayoutTests/ChangeLog
LayoutTests/fast/scrolling/ios/overflow-scroll-overlap-6-expected.txt
LayoutTests/platform/ios-wk2/scrollingcoordinator/scrolling-tree/nested-absolute-in-absolute-overflow-expected.txt [new file with mode: 0644]
LayoutTests/platform/ios-wk2/scrollingcoordinator/scrolling-tree/nested-absolute-in-overflow-expected.txt [new file with mode: 0644]
LayoutTests/platform/ios-wk2/scrollingcoordinator/scrolling-tree/nested-absolute-in-relative-in-overflow-expected.txt [new file with mode: 0644]
LayoutTests/platform/ios-wk2/scrollingcoordinator/scrolling-tree/nested-absolute-in-sc-overflow-expected.txt [new file with mode: 0644]
LayoutTests/scrollingcoordinator/scrolling-tree/nested-absolute-in-absolute-overflow-expected.txt [new file with mode: 0644]
LayoutTests/scrollingcoordinator/scrolling-tree/nested-absolute-in-absolute-overflow.html [new file with mode: 0644]
LayoutTests/scrollingcoordinator/scrolling-tree/nested-absolute-in-overflow-expected.txt [new file with mode: 0644]
LayoutTests/scrollingcoordinator/scrolling-tree/nested-absolute-in-overflow.html [new file with mode: 0644]
LayoutTests/scrollingcoordinator/scrolling-tree/nested-absolute-in-relative-in-overflow-expected.txt [new file with mode: 0644]
LayoutTests/scrollingcoordinator/scrolling-tree/nested-absolute-in-relative-in-overflow.html [new file with mode: 0644]
LayoutTests/scrollingcoordinator/scrolling-tree/nested-absolute-in-sc-overflow-expected.txt [new file with mode: 0644]
LayoutTests/scrollingcoordinator/scrolling-tree/nested-absolute-in-sc-overflow.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/rendering/RenderLayerCompositor.cpp

index 00f8cb1..26a169d 100644 (file)
@@ -1,3 +1,27 @@
+2019-04-11  Simon Fraser  <simon.fraser@apple.com>
+
+        [iOS WK2] Wrong scrolling behavior for nested absolute position elements inside overflow scroll
+        https://bugs.webkit.org/show_bug.cgi?id=196146
+
+        Reviewed by Antti Koivisto.
+        
+        Dump the scrolling tree for various configurations of positioned, overflow and stacking context
+        elements.
+
+        * fast/scrolling/ios/overflow-scroll-overlap-6-expected.txt: Progressed results.
+        * platform/ios-wk2/scrollingcoordinator/scrolling-tree/nested-absolute-in-absolute-overflow-expected.txt: Added.
+        * platform/ios-wk2/scrollingcoordinator/scrolling-tree/nested-absolute-in-overflow-expected.txt: Added.
+        * platform/ios-wk2/scrollingcoordinator/scrolling-tree/nested-absolute-in-relative-in-overflow-expected.txt: Added.
+        * platform/ios-wk2/scrollingcoordinator/scrolling-tree/nested-absolute-in-sc-overflow-expected.txt: Added.
+        * scrollingcoordinator/scrolling-tree/nested-absolute-in-absolute-overflow-expected.txt: Added.
+        * scrollingcoordinator/scrolling-tree/nested-absolute-in-absolute-overflow.html: Added.
+        * scrollingcoordinator/scrolling-tree/nested-absolute-in-overflow-expected.txt: Added.
+        * scrollingcoordinator/scrolling-tree/nested-absolute-in-overflow.html: Added.
+        * scrollingcoordinator/scrolling-tree/nested-absolute-in-relative-in-overflow-expected.txt: Added.
+        * scrollingcoordinator/scrolling-tree/nested-absolute-in-relative-in-overflow.html: Added.
+        * scrollingcoordinator/scrolling-tree/nested-absolute-in-sc-overflow-expected.txt: Added.
+        * scrollingcoordinator/scrolling-tree/nested-absolute-in-sc-overflow.html: Added.
+
 2019-04-12  Manuel Rego Casasnovas  <rego@igalia.com>
 
         [css-flex][css-grid] Fix synthesized baseline
 2019-04-12  Manuel Rego Casasnovas  <rego@igalia.com>
 
         [css-flex][css-grid] Fix synthesized baseline
index fb23ac2..26a01b7 100644 (file)
@@ -1,6 +1,6 @@
 Test that absolute positioned layer inside stacking-context overflow:scroll is correctly hit tested.
 
 case 1: 
 Test that absolute positioned layer inside stacking-context overflow:scroll is correctly hit tested.
 
 case 1: 
-case 2: Scrollable 2 
-case 3: Scrollable 3 
+case 2: 
+case 3: 
 
 
diff --git a/LayoutTests/platform/ios-wk2/scrollingcoordinator/scrolling-tree/nested-absolute-in-absolute-overflow-expected.txt b/LayoutTests/platform/ios-wk2/scrollingcoordinator/scrolling-tree/nested-absolute-in-absolute-overflow-expected.txt
new file mode 100644 (file)
index 0000000..f91ad09
--- /dev/null
@@ -0,0 +1,43 @@
+absabs
+
+(Frame scrolling node
+  (scrollable area size 800 600)
+  (contents size 800 600)
+  (parent relative scrollable rect at (0,0) size 800x600)
+  (scrollable area parameters 
+    (horizontal scroll elasticity 1)
+    (vertical scroll elasticity 1)
+    (horizontal scrollbar mode 0)
+    (vertical scrollbar mode 0))
+  (layout viewport at (0,0) size 800x600)
+  (min layout viewport origin (0,0))
+  (max layout viewport origin (0,0))
+  (behavior for fixed 0)
+  (children 3
+    (Overflow scrolling node
+      (scrollable area size 300 300)
+      (contents size 381 500)
+      (parent relative scrollable rect at (30,35) size 300x300)
+      (scrollable area parameters 
+        (horizontal scroll elasticity 1)
+        (vertical scroll elasticity 1)
+        (horizontal scrollbar mode 0)
+        (vertical scrollbar mode 0)
+        (has enabled horizontal scrollbar 1)
+        (has enabled vertical scrollbar 1))
+    )
+    (Positioned node
+      (layout constraints 
+        (layer-position-at-last-layout (50,50))
+        (positioning-behavior moves))
+      (related overflow nodes 1)
+    )
+    (Positioned node
+      (layout constraints 
+        (layer-position-at-last-layout (77,77))
+        (positioning-behavior moves))
+      (related overflow nodes 1)
+    )
+  )
+)
+
diff --git a/LayoutTests/platform/ios-wk2/scrollingcoordinator/scrolling-tree/nested-absolute-in-overflow-expected.txt b/LayoutTests/platform/ios-wk2/scrollingcoordinator/scrolling-tree/nested-absolute-in-overflow-expected.txt
new file mode 100644 (file)
index 0000000..594dcca
--- /dev/null
@@ -0,0 +1,30 @@
+absabs
+
+(Frame scrolling node
+  (scrollable area size 800 600)
+  (contents size 800 600)
+  (parent relative scrollable rect at (0,0) size 800x600)
+  (scrollable area parameters 
+    (horizontal scroll elasticity 1)
+    (vertical scroll elasticity 1)
+    (horizontal scrollbar mode 0)
+    (vertical scrollbar mode 0))
+  (layout viewport at (0,0) size 800x600)
+  (min layout viewport origin (0,0))
+  (max layout viewport origin (0,0))
+  (behavior for fixed 0)
+  (children 1
+    (Overflow scrolling node
+      (scrollable area size 300 300)
+      (contents size 300 500)
+      (parent relative scrollable rect at (30,22) size 300x300)
+      (scrollable area parameters 
+        (horizontal scroll elasticity 1)
+        (vertical scroll elasticity 1)
+        (horizontal scrollbar mode 0)
+        (vertical scrollbar mode 0)
+        (has enabled vertical scrollbar 1))
+    )
+  )
+)
+
diff --git a/LayoutTests/platform/ios-wk2/scrollingcoordinator/scrolling-tree/nested-absolute-in-relative-in-overflow-expected.txt b/LayoutTests/platform/ios-wk2/scrollingcoordinator/scrolling-tree/nested-absolute-in-relative-in-overflow-expected.txt
new file mode 100644 (file)
index 0000000..9270eee
--- /dev/null
@@ -0,0 +1,48 @@
+absabs
+
+(Frame scrolling node
+  (scrollable area size 800 600)
+  (contents size 800 600)
+  (parent relative scrollable rect at (0,0) size 800x600)
+  (scrollable area parameters 
+    (horizontal scroll elasticity 1)
+    (vertical scroll elasticity 1)
+    (horizontal scrollbar mode 0)
+    (vertical scrollbar mode 0))
+  (layout viewport at (0,0) size 800x600)
+  (min layout viewport origin (0,0))
+  (max layout viewport origin (0,0))
+  (behavior for fixed 0)
+  (children 4
+    (Overflow scrolling node
+      (scrollable area size 300 300)
+      (contents size 300 526)
+      (parent relative scrollable rect at (30,22) size 300x300)
+      (scrollable area parameters 
+        (horizontal scroll elasticity 1)
+        (vertical scroll elasticity 1)
+        (horizontal scrollbar mode 0)
+        (vertical scrollbar mode 0)
+        (has enabled vertical scrollbar 1))
+    )
+    (Positioned node
+      (layout constraints 
+        (layer-position-at-last-layout (0,0))
+        (positioning-behavior moves))
+      (related overflow nodes 1)
+    )
+    (Positioned node
+      (layout constraints 
+        (layer-position-at-last-layout (53,53))
+        (positioning-behavior moves))
+      (related overflow nodes 1)
+    )
+    (Positioned node
+      (layout constraints 
+        (layer-position-at-last-layout (80,80))
+        (positioning-behavior moves))
+      (related overflow nodes 1)
+    )
+  )
+)
+
diff --git a/LayoutTests/platform/ios-wk2/scrollingcoordinator/scrolling-tree/nested-absolute-in-sc-overflow-expected.txt b/LayoutTests/platform/ios-wk2/scrollingcoordinator/scrolling-tree/nested-absolute-in-sc-overflow-expected.txt
new file mode 100644 (file)
index 0000000..dced88a
--- /dev/null
@@ -0,0 +1,44 @@
+absabs
+
+(Frame scrolling node
+  (scrollable area size 800 600)
+  (contents size 800 600)
+  (parent relative scrollable rect at (0,0) size 800x600)
+  (scrollable area parameters 
+    (horizontal scroll elasticity 1)
+    (vertical scroll elasticity 1)
+    (horizontal scrollbar mode 0)
+    (vertical scrollbar mode 0))
+  (layout viewport at (0,0) size 800x600)
+  (min layout viewport origin (0,0))
+  (max layout viewport origin (0,0))
+  (behavior for fixed 0)
+  (children 1
+    (Overflow scrolling node
+      (scrollable area size 300 300)
+      (contents size 300 500)
+      (parent relative scrollable rect at (30,22) size 300x300)
+      (scrollable area parameters 
+        (horizontal scroll elasticity 1)
+        (vertical scroll elasticity 1)
+        (horizontal scrollbar mode 0)
+        (vertical scrollbar mode 0)
+        (has enabled vertical scrollbar 1))
+      (children 2
+        (Positioned node
+          (layout constraints 
+            (layer-position-at-last-layout (20,28))
+            (positioning-behavior stationary))
+          (related overflow nodes 1)
+        )
+        (Positioned node
+          (layout constraints 
+            (layer-position-at-last-layout (47,55))
+            (positioning-behavior stationary))
+          (related overflow nodes 1)
+        )
+      )
+    )
+  )
+)
+
diff --git a/LayoutTests/scrollingcoordinator/scrolling-tree/nested-absolute-in-absolute-overflow-expected.txt b/LayoutTests/scrollingcoordinator/scrolling-tree/nested-absolute-in-absolute-overflow-expected.txt
new file mode 100644 (file)
index 0000000..59c823c
--- /dev/null
@@ -0,0 +1,43 @@
+absabs
+
+(Frame scrolling node
+  (scrollable area size 800 600)
+  (contents size 800 600)
+  (parent relative scrollable rect at (0,0) size 800x600)
+  (scrollable area parameters 
+    (horizontal scroll elasticity 2)
+    (vertical scroll elasticity 2)
+    (horizontal scrollbar mode 0)
+    (vertical scrollbar mode 0))
+  (layout viewport at (0,0) size 800x600)
+  (min layout viewport origin (0,0))
+  (max layout viewport origin (0,0))
+  (behavior for fixed 0)
+  (children 3
+    (Overflow scrolling node
+      (scrollable area size 285 285)
+      (contents size 381 500)
+      (parent relative scrollable rect at (30,35) size 285x285)
+      (scrollable area parameters 
+        (horizontal scroll elasticity 0)
+        (vertical scroll elasticity 0)
+        (horizontal scrollbar mode 0)
+        (vertical scrollbar mode 0)
+        (has enabled horizontal scrollbar 1)
+        (has enabled vertical scrollbar 1))
+    )
+    (Positioned node
+      (layout constraints 
+        (layer-position-at-last-layout (50,50))
+        (positioning-behavior moves))
+      (related overflow nodes 1)
+    )
+    (Positioned node
+      (layout constraints 
+        (layer-position-at-last-layout (77,77))
+        (positioning-behavior moves))
+      (related overflow nodes 1)
+    )
+  )
+)
+
diff --git a/LayoutTests/scrollingcoordinator/scrolling-tree/nested-absolute-in-absolute-overflow.html b/LayoutTests/scrollingcoordinator/scrolling-tree/nested-absolute-in-absolute-overflow.html
new file mode 100644 (file)
index 0000000..526f732
--- /dev/null
@@ -0,0 +1,58 @@
+<!DOCTYPE html> <!-- webkit-test-runner [ internal:AsyncOverflowScrollingEnabled=true ] -->
+<html>
+<head>
+    <title>Nested absolutes move with the overflow but are not stacking descendants: need two 'move' nodes</title>
+    <style>
+        .scrollcontent {
+            height: 500px;
+        }
+        .scroller {
+            position:absolute;
+            margin: 20px;
+            overflow: scroll;
+            height: 300px;
+            width: 300px;
+            border: 2px solid black;
+        }
+    
+        .absolute {
+            position:absolute;
+            left: 50px;
+            top: 50px;
+            width: 100px;
+            height: 100px;
+            background: gray;
+            border: 2px solid green;
+        }
+    
+        .inner {
+            left:25px;
+            top:25px;
+            width: 300px;
+        }
+
+        .scrollcontent {
+            height: 500px;
+            background-image: repeating-linear-gradient(white, silver 200px);
+        }
+    </style>
+    <script>
+        if (window.testRunner)
+            testRunner.dumpAsText();
+
+        window.addEventListener('load', () => {
+            if (window.internals)
+                document.getElementById('tree').innerText = internals.scrollingStateTreeAsText();
+        }, false);
+    </script>
+</head>
+<body>
+    <div class="scroller">
+        <div class="absolute">abs
+            <div class="inner absolute">abs</div>
+        </div>
+        <div class="scrollcontent"></div>
+    </div>
+<pre id="tree"></pre>
+</body>
+</html>
diff --git a/LayoutTests/scrollingcoordinator/scrolling-tree/nested-absolute-in-overflow-expected.txt b/LayoutTests/scrollingcoordinator/scrolling-tree/nested-absolute-in-overflow-expected.txt
new file mode 100644 (file)
index 0000000..90837ed
--- /dev/null
@@ -0,0 +1,30 @@
+absabs
+
+(Frame scrolling node
+  (scrollable area size 800 600)
+  (contents size 800 600)
+  (parent relative scrollable rect at (0,0) size 800x600)
+  (scrollable area parameters 
+    (horizontal scroll elasticity 2)
+    (vertical scroll elasticity 2)
+    (horizontal scrollbar mode 0)
+    (vertical scrollbar mode 0))
+  (layout viewport at (0,0) size 800x600)
+  (min layout viewport origin (0,0))
+  (max layout viewport origin (0,0))
+  (behavior for fixed 0)
+  (children 1
+    (Overflow scrolling node
+      (scrollable area size 285 285)
+      (contents size 285 500)
+      (parent relative scrollable rect at (30,22) size 285x285)
+      (scrollable area parameters 
+        (horizontal scroll elasticity 0)
+        (vertical scroll elasticity 0)
+        (horizontal scrollbar mode 0)
+        (vertical scrollbar mode 0)
+        (has enabled vertical scrollbar 1))
+    )
+  )
+)
+
diff --git a/LayoutTests/scrollingcoordinator/scrolling-tree/nested-absolute-in-overflow.html b/LayoutTests/scrollingcoordinator/scrolling-tree/nested-absolute-in-overflow.html
new file mode 100644 (file)
index 0000000..14f3c98
--- /dev/null
@@ -0,0 +1,57 @@
+<!DOCTYPE html> <!-- webkit-test-runner [ internal:AsyncOverflowScrollingEnabled=true ] -->
+<html>
+<head>
+    <title>Nested absolutes don't with the overflow and are not stacking descendants: there should be no positioning nodes</title>
+    <style>
+        .scrollcontent {
+            height: 500px;
+        }
+        .scroller {
+            margin: 20px;
+            overflow: scroll;
+            height: 300px;
+            width: 300px;
+            border: 2px solid black;
+        }
+    
+        .absolute {
+            position:absolute;
+            left: 50px;
+            top: 50px;
+            width: 100px;
+            height: 100px;
+            background: gray;
+            border: 2px solid green;
+        }
+    
+        .inner {
+            left:25px;
+            top:25px;
+            width: 300px;
+        }
+
+        .scrollcontent {
+            height: 500px;
+            background-image: repeating-linear-gradient(white, silver 200px);
+        }
+    </style>
+    <script>
+        if (window.testRunner)
+            testRunner.dumpAsText();
+
+        window.addEventListener('load', () => {
+            if (window.internals)
+                document.getElementById('tree').innerText = internals.scrollingStateTreeAsText();
+        }, false);
+    </script>
+</head>
+<body>
+    <div class="scroller">
+        <div class="absolute">abs
+            <div class="inner absolute">abs</div>
+        </div>
+        <div class="scrollcontent"></div>
+    </div>
+<pre id="tree"></pre>
+</body>
+</html>
diff --git a/LayoutTests/scrollingcoordinator/scrolling-tree/nested-absolute-in-relative-in-overflow-expected.txt b/LayoutTests/scrollingcoordinator/scrolling-tree/nested-absolute-in-relative-in-overflow-expected.txt
new file mode 100644 (file)
index 0000000..f25a749
--- /dev/null
@@ -0,0 +1,48 @@
+absabs
+
+(Frame scrolling node
+  (scrollable area size 800 600)
+  (contents size 800 600)
+  (parent relative scrollable rect at (0,0) size 800x600)
+  (scrollable area parameters 
+    (horizontal scroll elasticity 2)
+    (vertical scroll elasticity 2)
+    (horizontal scrollbar mode 0)
+    (vertical scrollbar mode 0))
+  (layout viewport at (0,0) size 800x600)
+  (min layout viewport origin (0,0))
+  (max layout viewport origin (0,0))
+  (behavior for fixed 0)
+  (children 4
+    (Overflow scrolling node
+      (scrollable area size 285 285)
+      (contents size 285 526)
+      (parent relative scrollable rect at (30,22) size 285x285)
+      (scrollable area parameters 
+        (horizontal scroll elasticity 0)
+        (vertical scroll elasticity 0)
+        (horizontal scrollbar mode 0)
+        (vertical scrollbar mode 0)
+        (has enabled vertical scrollbar 1))
+    )
+    (Positioned node
+      (layout constraints 
+        (layer-position-at-last-layout (0,0))
+        (positioning-behavior moves))
+      (related overflow nodes 1)
+    )
+    (Positioned node
+      (layout constraints 
+        (layer-position-at-last-layout (53,53))
+        (positioning-behavior moves))
+      (related overflow nodes 1)
+    )
+    (Positioned node
+      (layout constraints 
+        (layer-position-at-last-layout (80,80))
+        (positioning-behavior moves))
+      (related overflow nodes 1)
+    )
+  )
+)
+
diff --git a/LayoutTests/scrollingcoordinator/scrolling-tree/nested-absolute-in-relative-in-overflow.html b/LayoutTests/scrollingcoordinator/scrolling-tree/nested-absolute-in-relative-in-overflow.html
new file mode 100644 (file)
index 0000000..8deb6d0
--- /dev/null
@@ -0,0 +1,66 @@
+<!DOCTYPE html> <!-- webkit-test-runner [ internal:AsyncOverflowScrollingEnabled=true ] -->
+<html>
+<head>
+    <title>Absolutes in relative in overflow move with it but are not stacking descendants: need 3 'move' nodes</title>
+    <style>
+        .scrollcontent {
+            height: 500px;
+        }
+        .scroller {
+            margin: 20px;
+            overflow: scroll;
+            height: 300px;
+            width: 300px;
+            border: 2px solid black;
+        }
+    
+        .relative {
+            position: relative;
+            border: 3px solid orange;
+            height: 20px;
+        }
+
+        .absolute {
+            position:absolute;
+            left: 50px;
+            top: 50px;
+            width: 100px;
+            height: 100px;
+            background: gray;
+            border: 2px solid green;
+        }
+    
+        .inner {
+            left:25px;
+            top:25px;
+            width: 200px;
+        }
+
+        .scrollcontent {
+            height: 500px;
+            background-image: repeating-linear-gradient(white, silver 200px);
+        }
+    </style>
+    <script>
+        if (window.testRunner)
+            testRunner.dumpAsText();
+
+        window.addEventListener('load', () => {
+            if (window.internals)
+                document.getElementById('tree').innerText = internals.scrollingStateTreeAsText();
+        }, false);
+    </script>
+</head>
+<body>
+
+    <div class="scroller">
+        <div class="relative">
+            <div class="absolute">abs
+                <div class="inner absolute">abs</div>
+            </div>
+        </div>
+        <div class="scrollcontent"></div>
+    </div>
+<pre id="tree"></pre>
+</body>
+</html>
diff --git a/LayoutTests/scrollingcoordinator/scrolling-tree/nested-absolute-in-sc-overflow-expected.txt b/LayoutTests/scrollingcoordinator/scrolling-tree/nested-absolute-in-sc-overflow-expected.txt
new file mode 100644 (file)
index 0000000..5a9cfd9
--- /dev/null
@@ -0,0 +1,44 @@
+absabs
+
+(Frame scrolling node
+  (scrollable area size 800 600)
+  (contents size 800 600)
+  (parent relative scrollable rect at (0,0) size 800x600)
+  (scrollable area parameters 
+    (horizontal scroll elasticity 2)
+    (vertical scroll elasticity 2)
+    (horizontal scrollbar mode 0)
+    (vertical scrollbar mode 0))
+  (layout viewport at (0,0) size 800x600)
+  (min layout viewport origin (0,0))
+  (max layout viewport origin (0,0))
+  (behavior for fixed 0)
+  (children 1
+    (Overflow scrolling node
+      (scrollable area size 285 285)
+      (contents size 285 500)
+      (parent relative scrollable rect at (30,22) size 285x285)
+      (scrollable area parameters 
+        (horizontal scroll elasticity 0)
+        (vertical scroll elasticity 0)
+        (horizontal scrollbar mode 0)
+        (vertical scrollbar mode 0)
+        (has enabled vertical scrollbar 1))
+      (children 2
+        (Positioned node
+          (layout constraints 
+            (layer-position-at-last-layout (20,28))
+            (positioning-behavior stationary))
+          (related overflow nodes 1)
+        )
+        (Positioned node
+          (layout constraints 
+            (layer-position-at-last-layout (47,55))
+            (positioning-behavior stationary))
+          (related overflow nodes 1)
+        )
+      )
+    )
+  )
+)
+
diff --git a/LayoutTests/scrollingcoordinator/scrolling-tree/nested-absolute-in-sc-overflow.html b/LayoutTests/scrollingcoordinator/scrolling-tree/nested-absolute-in-sc-overflow.html
new file mode 100644 (file)
index 0000000..6cc87bb
--- /dev/null
@@ -0,0 +1,58 @@
+<!DOCTYPE html> <!-- webkit-test-runner [ internal:AsyncOverflowScrollingEnabled=true ] -->
+<html>
+<head>
+    <title>Nested absolutes in stacking-context overflow: need two 'stationary' nodes</title>
+    <style>
+        .scrollcontent {
+            height: 500px;
+        }
+        .scroller {
+            margin: 20px;
+            overflow: scroll;
+            height: 300px;
+            width: 300px;
+            border: 2px solid black;
+            opacity: 0.9;
+        }
+
+        .absolute {
+            position:absolute;
+            left: 50px;
+            top: 50px;
+            width: 100px;
+            height: 100px;
+            background: gray;
+            border: 2px solid green;
+        }
+    
+        .inner {
+            left: 25px;
+            top: 25px;
+            width: 200px;
+        }
+
+        .scrollcontent {
+            height: 500px;
+            background-image: repeating-linear-gradient(white, silver 200px);
+        }
+    </style>
+    <script>
+        if (window.testRunner)
+            testRunner.dumpAsText();
+
+        window.addEventListener('load', () => {
+            if (window.internals)
+                document.getElementById('tree').innerText = internals.scrollingStateTreeAsText();
+        }, false);
+    </script>
+</head>
+<body>
+    <div class="scroller">
+        <div class="absolute">abs
+            <div class="inner absolute">abs</div>
+        </div>
+        <div class="scrollcontent"></div>
+    </div>
+<pre id="tree"></pre>
+</body>
+</html>
index 962e666..fcf9282 100644 (file)
@@ -1,3 +1,35 @@
+2019-04-11  Simon Fraser  <simon.fraser@apple.com>
+
+        [iOS WK2] Wrong scrolling behavior for nested absolute position elements inside overflow scroll
+        https://bugs.webkit.org/show_bug.cgi?id=196146
+
+        Reviewed by Antti Koivisto.
+        
+        computeCoordinatedPositioningForLayer() failed to handle nested positions elements
+        inside overflow scroll, because it only walked up to the first containing block of
+        a nested position:absolute. We need to walk all the way up the ancestor layer chain,
+        looking at containing block, scroller and composited ancestor relationships.
+
+        Make this code easier to understand by writing it in terms of "is foo scrolled by bar", rather than
+        trying to collapse all the logic into a single ancestor walk, which was really hard. This is a few
+        more ancestor traversals, but we now only run this code if there's composited scrolling
+        in the ancestor chain.
+
+        Tests: scrollingcoordinator/scrolling-tree/nested-absolute-in-absolute-overflow.html
+               scrollingcoordinator/scrolling-tree/nested-absolute-in-overflow.html
+               scrollingcoordinator/scrolling-tree/nested-absolute-in-relative-in-overflow.html
+               scrollingcoordinator/scrolling-tree/nested-absolute-in-sc-overflow.html
+
+        * rendering/RenderLayerCompositor.cpp:
+        (WebCore::enclosingCompositedScrollingLayer):
+        (WebCore::isScrolledByOverflowScrollLayer):
+        (WebCore::isNonScrolledLayerInsideScrolledCompositedAncestor):
+        (WebCore::RenderLayerCompositor::layerContainingBlockCrossesCoordinatedScrollingBoundary):
+        (WebCore::collectStationaryLayerRelatedOverflowNodes):
+        (WebCore::RenderLayerCompositor::computeCoordinatedPositioningForLayer const):
+        (WebCore::collectRelatedCoordinatedScrollingNodes):
+        (WebCore::layerParentedAcrossCoordinatedScrollingBoundary): Deleted.
+
 2019-04-12  Manuel Rego Casasnovas  <rego@igalia.com>
 
         [css-flex][css-grid] Fix synthesized baseline
 2019-04-12  Manuel Rego Casasnovas  <rego@igalia.com>
 
         [css-flex][css-grid] Fix synthesized baseline
index 491c17b..3f2f2e5 100644 (file)
@@ -2904,42 +2904,97 @@ bool RenderLayerCompositor::useCoordinatedScrollingForLayer(const RenderLayer& l
     return false;
 }
 
     return false;
 }
 
-// Is this layer's containingBlock an ancestor of scrollable overflow, and is the layer's compositing ancestor inside that overflow?
-bool RenderLayerCompositor::layerContainingBlockCrossesCoordinatedScrollingBoundary(const RenderLayer& layer, const RenderLayer& compositedAncestor)
+static RenderLayer* enclosingCompositedScrollingLayer(const RenderLayer& layer, const RenderLayer& intermediateLayer, bool& sawIntermediateLayer)
 {
 {
-    ASSERT(layer.renderer().isAbsolutelyPositioned());
+    const auto* currLayer = &layer;
+    while (currLayer) {
+        if (currLayer == &intermediateLayer)
+            sawIntermediateLayer = true;
 
 
-    bool sawCompositingAncestor = false;
-    for (const auto* currLayer = layer.parent(); currLayer; currLayer = currLayer->parent()) {
-        if (currLayer->renderer().canContainAbsolutelyPositionedObjects())
-            return false;
+        if (currLayer->hasCompositedScrollableOverflow())
+            return const_cast<RenderLayer*>(currLayer);
 
 
-        if (currLayer == &compositedAncestor)
-            sawCompositingAncestor = true;
+        currLayer = currLayer->parent();
+    }
 
 
-        if (currLayer->hasCompositedScrollableOverflow())
-            return sawCompositingAncestor;
+    return nullptr;
+}
+
+// Return true if overflowScrollLayer is in layer's containing block chain.
+static bool isScrolledByOverflowScrollLayer(const RenderLayer& layer, const RenderLayer& overflowScrollLayer)
+{
+    bool containingBlockCanSkipLayers = layer.renderer().isAbsolutelyPositioned();
+
+    for (const auto* currLayer = layer.parent(); currLayer; currLayer = currLayer->parent()) {
+        bool inContainingBlockChain = true;
+        if (containingBlockCanSkipLayers) {
+            inContainingBlockChain = currLayer->renderer().canContainAbsolutelyPositionedObjects();
+            if (inContainingBlockChain)
+                containingBlockCanSkipLayers = currLayer->renderer().isAbsolutelyPositioned();
+        }
+
+        if (currLayer == &overflowScrollLayer)
+            return inContainingBlockChain;
     }
 
     return false;
 }
 
     }
 
     return false;
 }
 
-// Is there scrollable overflow between this layer and its composited ancestor, and the containing block is the scroller or inside the scroller.
-static bool layerParentedAcrossCoordinatedScrollingBoundary(const RenderLayer& layer, const RenderLayer& compositedAncestor, bool checkContainingBlock, bool& containingBlockIsInsideOverflow)
+static bool isNonScrolledLayerInsideScrolledCompositedAncestor(const RenderLayer& layer, const RenderLayer& compositedAncestor, const RenderLayer& scrollingAncestor)
 {
 {
-    ASSERT(layer.isComposited());
+    bool ancestorMovedByScroller = &compositedAncestor == &scrollingAncestor || isScrolledByOverflowScrollLayer(compositedAncestor, scrollingAncestor);
+    bool layerMovedByScroller = isScrolledByOverflowScrollLayer(layer, scrollingAncestor);
 
 
-    for (const auto* currLayer = layer.parent(); currLayer != &compositedAncestor; currLayer = currLayer->parent()) {
-        if (checkContainingBlock && currLayer->renderer().canContainAbsolutelyPositionedObjects())
-            containingBlockIsInsideOverflow = true;
+    return ancestorMovedByScroller && !layerMovedByScroller;
+}
 
 
-        if (currLayer->hasCompositedScrollableOverflow())
-            return true;
+bool RenderLayerCompositor::layerContainingBlockCrossesCoordinatedScrollingBoundary(const RenderLayer& layer, const RenderLayer& compositedAncestor)
+{
+    bool compositedAncestorIsInsideScroller = false;
+    auto* scrollingAncestor = enclosingCompositedScrollingLayer(layer, compositedAncestor, compositedAncestorIsInsideScroller);
+    if (!scrollingAncestor) {
+        ASSERT_NOT_REACHED(); // layer.hasCompositedScrollingAncestor() should guarantee we have one.
+        return false;
     }
     }
+    
+    if (!compositedAncestorIsInsideScroller)
+        return false;
 
 
-    return false;
+    return isNonScrolledLayerInsideScrolledCompositedAncestor(layer, compositedAncestor, *scrollingAncestor);
 }
 
 }
 
+static void collectStationaryLayerRelatedOverflowNodes(const RenderLayer& layer, const RenderLayer& /*compositedAncestor*/, Vector<ScrollingNodeID>& scrollingNodes)
+{
+    ASSERT(layer.isComposited());
+    
+    auto appendOverflowLayerNodeID = [&scrollingNodes] (const RenderLayer& overflowLayer) {
+        ASSERT(overflowLayer.isComposited());
+        auto scrollingNodeID = overflowLayer.backing()->scrollingNodeIDForRole(ScrollCoordinationRole::Scrolling);
+        if (scrollingNodeID)
+            scrollingNodes.append(scrollingNodeID);
+        else
+            LOG(Scrolling, "Layer %p doesn't have scrolling node ID yet", &overflowLayer);
+    };
+
+    ASSERT(layer.renderer().isAbsolutelyPositioned());
+    bool containingBlockCanSkipLayers = true;
+
+    for (const auto* currLayer = layer.parent(); currLayer; currLayer = currLayer->parent()) {
+        bool inContainingBlockChain = true;
+        if (containingBlockCanSkipLayers) {
+            inContainingBlockChain = currLayer->renderer().canContainAbsolutelyPositionedObjects();
+            if (inContainingBlockChain)
+                containingBlockCanSkipLayers = currLayer->renderer().isAbsolutelyPositioned();
+        }
+
+        if (currLayer->hasCompositedScrollableOverflow()) {
+            appendOverflowLayerNodeID(*currLayer);
+            break;
+        }
+    }
+}
+
+
 ScrollPositioningBehavior RenderLayerCompositor::computeCoordinatedPositioningForLayer(const RenderLayer& layer) const
 {
     if (layer.isRenderViewLayer())
 ScrollPositioningBehavior RenderLayerCompositor::computeCoordinatedPositioningForLayer(const RenderLayer& layer) const
 {
     if (layer.isRenderViewLayer())
@@ -2955,24 +3010,32 @@ ScrollPositioningBehavior RenderLayerCompositor::computeCoordinatedPositioningFo
     if (!scrollingCoordinator)
         return ScrollPositioningBehavior::None;
 
     if (!scrollingCoordinator)
         return ScrollPositioningBehavior::None;
 
+    auto* compositedAncestor = layer.ancestorCompositingLayer();
+    if (!compositedAncestor) {
+        ASSERT_NOT_REACHED();
+        return ScrollPositioningBehavior::None;
+    }
+
+    bool compositedAncestorIsInsideScroller = false;
+    auto* scrollingAncestor = enclosingCompositedScrollingLayer(layer, *compositedAncestor, compositedAncestorIsInsideScroller);
+    if (!scrollingAncestor) {
+        ASSERT_NOT_REACHED(); // layer.hasCompositedScrollingAncestor() should guarantee we have one.
+        return ScrollPositioningBehavior::None;
+    }
+
     // There are two cases we have to deal with here:
     // 1. There's a composited overflow:scroll in the parent chain between the renderer and its containing block, and the layer's
     //    composited (z-order) ancestor is inside the scroller or is the scroller. In this case, we have to compensate for scroll position
     //    changes to make the positioned layer stay in the same place. This only applies to position:absolute (since we handle fixed elsewhere).
     // There are two cases we have to deal with here:
     // 1. There's a composited overflow:scroll in the parent chain between the renderer and its containing block, and the layer's
     //    composited (z-order) ancestor is inside the scroller or is the scroller. In this case, we have to compensate for scroll position
     //    changes to make the positioned layer stay in the same place. This only applies to position:absolute (since we handle fixed elsewhere).
-    auto* compositedAncestor = layer.ancestorCompositingLayer();
-
-    auto& renderer = layer.renderer();
-    bool containingBlockCanSkipOverflow = renderer.isOutOfFlowPositioned() && renderer.style().position() == PositionType::Absolute;
-    if (containingBlockCanSkipOverflow) {
-        if (layerContainingBlockCrossesCoordinatedScrollingBoundary(layer, *compositedAncestor))
+    if (layer.renderer().isAbsolutelyPositioned()) {
+        if (compositedAncestorIsInsideScroller && isNonScrolledLayerInsideScrolledCompositedAncestor(layer, *compositedAncestor, *scrollingAncestor))
             return ScrollPositioningBehavior::Stationary;
     }
 
     // 2. The layer's containing block is the overflow or inside the overflow:scroll, but its z-order ancestor is
     //    outside the overflow:scroll. In that case, we have to move the layer via the scrolling tree to make
     //    it move along with the overflow scrolling.
             return ScrollPositioningBehavior::Stationary;
     }
 
     // 2. The layer's containing block is the overflow or inside the overflow:scroll, but its z-order ancestor is
     //    outside the overflow:scroll. In that case, we have to move the layer via the scrolling tree to make
     //    it move along with the overflow scrolling.
-    bool containingBlockIsInsideOverflow = !containingBlockCanSkipOverflow;
-    if (layerParentedAcrossCoordinatedScrollingBoundary(layer, *compositedAncestor, containingBlockCanSkipOverflow, containingBlockIsInsideOverflow) && containingBlockIsInsideOverflow)
+    if (!compositedAncestorIsInsideScroller && isScrolledByOverflowScrollLayer(layer, *scrollingAncestor))
         return ScrollPositioningBehavior::Moves;
 
     return ScrollPositioningBehavior::None;
         return ScrollPositioningBehavior::Moves;
 
     return ScrollPositioningBehavior::None;
@@ -2998,21 +3061,12 @@ static Vector<ScrollingNodeID> collectRelatedCoordinatedScrollingNodes(const Ren
         break;
     }
     case ScrollPositioningBehavior::Stationary: {
         break;
     }
     case ScrollPositioningBehavior::Stationary: {
-        // Collect all the composited scrollers between this layer and its containing block.
-        ASSERT(layer.renderer().style().position() == PositionType::Absolute);
-        for (const auto* currLayer = layer.parent(); currLayer; currLayer = currLayer->parent()) {
-            if (currLayer->renderer().canContainAbsolutelyPositionedObjects())
-                break;
-
-            if (currLayer->hasCompositedScrollableOverflow()) {
-                auto scrollingNodeID = currLayer->backing()->scrollingNodeIDForRole(ScrollCoordinationRole::Scrolling);
-                if (scrollingNodeID)
-                    overflowNodeData.append(scrollingNodeID);
-                else
-                    LOG(Scrolling, "Layer %p doesn't have scrolling node ID yet", &layer);
-            }
-        }
-        // Don't need to do anything because the layer is a descendant of the overflow in stacking.
+        ASSERT(layer.renderer().isAbsolutelyPositioned());
+        // Collect all the composited scrollers between this layer and its composited ancestor.
+        auto* compositedAncestor = layer.ancestorCompositingLayer();
+        if (!compositedAncestor)
+            return overflowNodeData;
+        collectStationaryLayerRelatedOverflowNodes(layer, *compositedAncestor, overflowNodeData);
         break;
     }
     case ScrollPositioningBehavior::None:
         break;
     }
     case ScrollPositioningBehavior::None: