[iOS WK2] position:fixed inside oveflow:scroll is jumpy
authorsimon.fraser@apple.com <simon.fraser@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 26 Mar 2019 19:23:52 +0000 (19:23 +0000)
committersimon.fraser@apple.com <simon.fraser@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 26 Mar 2019 19:23:52 +0000 (19:23 +0000)
https://bugs.webkit.org/show_bug.cgi?id=196238

Reviewed by Antti Koivisto.
Source/WebCore:

We were inadvertently making Positioned nodes for position:fixed, which is unnecessary because
Fixed nodes handle them, and harmful because they introduced unwanted layer movement.

Tests: scrollingcoordinator/ios/fixed-in-overflow-scroll-scrolling-tree.html
       scrollingcoordinator/ios/fixed-in-overflow-scroll.html

* rendering/RenderLayerCompositor.cpp:
(WebCore::RenderLayerCompositor::computeCoordinatedPositioningForLayer const):

LayoutTests:

fixed-in-overflow-scroll-scrolling-tree.html actually tests the fix.
For some reason fixed-in-overflow-scroll.html doesn't show the jumpiness, but it's
a good test to have nonetheless.

Other minor cleanup.

* resources/ui-helper.js:
(window.UIHelper.immediateScrollElementAtContentPointToOffset):
* scrollingcoordinator/ios/fixed-in-overflow-scroll-expected.html: Added.
* scrollingcoordinator/ios/fixed-in-overflow-scroll-scrolling-tree-expected.txt: Added.
* scrollingcoordinator/ios/fixed-in-overflow-scroll-scrolling-tree.html: Copied from LayoutTests/scrollingcoordinator/ios/ui-scrolling-tree.html.
* scrollingcoordinator/ios/fixed-in-overflow-scroll.html: Added.
* scrollingcoordinator/ios/ui-scrolling-tree.html:

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

LayoutTests/ChangeLog
LayoutTests/resources/ui-helper.js
LayoutTests/scrollingcoordinator/ios/fixed-in-overflow-scroll-expected.html [new file with mode: 0644]
LayoutTests/scrollingcoordinator/ios/fixed-in-overflow-scroll-scrolling-tree-expected.txt [new file with mode: 0644]
LayoutTests/scrollingcoordinator/ios/fixed-in-overflow-scroll-scrolling-tree.html [new file with mode: 0644]
LayoutTests/scrollingcoordinator/ios/fixed-in-overflow-scroll.html [new file with mode: 0644]
LayoutTests/scrollingcoordinator/ios/ui-scrolling-tree.html
Source/WebCore/ChangeLog
Source/WebCore/rendering/RenderLayerCompositor.cpp

index 7d443d0..01c1f07 100644 (file)
@@ -1,3 +1,24 @@
+2019-03-26  Simon Fraser  <simon.fraser@apple.com>
+
+        [iOS WK2] position:fixed inside oveflow:scroll is jumpy
+        https://bugs.webkit.org/show_bug.cgi?id=196238
+
+        Reviewed by Antti Koivisto.
+
+        fixed-in-overflow-scroll-scrolling-tree.html actually tests the fix.
+        For some reason fixed-in-overflow-scroll.html doesn't show the jumpiness, but it's
+        a good test to have nonetheless.
+
+        Other minor cleanup.
+
+        * resources/ui-helper.js:
+        (window.UIHelper.immediateScrollElementAtContentPointToOffset):
+        * scrollingcoordinator/ios/fixed-in-overflow-scroll-expected.html: Added.
+        * scrollingcoordinator/ios/fixed-in-overflow-scroll-scrolling-tree-expected.txt: Added.
+        * scrollingcoordinator/ios/fixed-in-overflow-scroll-scrolling-tree.html: Copied from LayoutTests/scrollingcoordinator/ios/ui-scrolling-tree.html.
+        * scrollingcoordinator/ios/fixed-in-overflow-scroll.html: Added.
+        * scrollingcoordinator/ios/ui-scrolling-tree.html:
+
 2019-03-26  Andy VanWagoner  <andy@vanwagoner.family>
 
         Intl.DateTimeFormat should obey 2-digit hour
index 968e700..8399b79 100644 (file)
@@ -286,13 +286,14 @@ window.UIHelper = class UIHelper {
         });
     }
 
-    static immediateScrollElementAtContentPointToOffset(x, y, scrollX, scrollY)
+    static immediateScrollElementAtContentPointToOffset(x, y, scrollX, scrollY, scrollUpdatesDisabled = false)
     {
         if (!this.isWebKit2())
             return Promise.resolve();
 
         return new Promise(resolve => {
             testRunner.runUIScript(`
+                uiController.scrollUpdatesDisabled = ${scrollUpdatesDisabled};
                 uiController.immediateScrollElementAtContentPointToOffset(${x}, ${y}, ${scrollX}, ${scrollY});`, resolve);
         });
     }
diff --git a/LayoutTests/scrollingcoordinator/ios/fixed-in-overflow-scroll-expected.html b/LayoutTests/scrollingcoordinator/ios/fixed-in-overflow-scroll-expected.html
new file mode 100644 (file)
index 0000000..79ac77d
--- /dev/null
@@ -0,0 +1,50 @@
+<!DOCTYPE html> <!-- webkit-test-runner [ useFlexibleViewport=true internal:AsyncOverflowScrollingEnabled=true ] -->
+<html>
+<head>
+    <meta name="viewport" content="initial-scale=1.0">
+    <style>
+        #scroller {
+            margin: 10px;
+            height: 300px;
+            width: 300px;
+            border: 1px solid black;
+            overflow: scroll;
+        }
+        
+        .box {
+            position: fixed;
+            margin-top: 200px;
+            width: 200px;
+            height: 200px;
+            background-color: green;
+        }
+        
+        .spacer {
+            height: 800px;
+        }
+    </style>
+    <script src="../../resources/ui-helper.js"></script>
+    <script>
+        if (window.testRunner)
+            testRunner.waitUntilDone();
+
+        async function doTest()
+        {
+            await UIHelper.ensurePresentationUpdate(); // Not sure why this is necessary, but it is.
+            scroller.scrollTo(0, 200);
+            await UIHelper.ensurePresentationUpdate();
+
+            if (window.testRunner)
+                testRunner.notifyDone();
+        }
+        
+        window.addEventListener('load', doTest, false);
+    </script>
+</head>
+<body>
+    <div id="scroller">
+        <div class="box"></div>
+        <div class="spacer"></div>
+    </div>
+</body>
+</html>
diff --git a/LayoutTests/scrollingcoordinator/ios/fixed-in-overflow-scroll-scrolling-tree-expected.txt b/LayoutTests/scrollingcoordinator/ios/fixed-in-overflow-scroll-scrolling-tree-expected.txt
new file mode 100644 (file)
index 0000000..f375663
--- /dev/null
@@ -0,0 +1,30 @@
+
+(scrolling tree
+  (frame scrolling node
+    (scrollable area size width=320 height=548)
+    (total content size width=320 height=548)
+    (last committed scroll position (0,0))
+    (scrollable area parameters 
+      (horizontal scroll elasticity 1)
+      (vertical scroll elasticity 1)
+      (horizontal scrollbar mode 0)
+      (vertical scrollbar mode 0))
+    (layout viewport (0,0) width=320 height=548)
+    (min layoutViewport origin (0,0))
+    (max layoutViewport origin (0,0))
+    (behavior for fixed 0)
+    (overflow scrolling node
+      (scrollable area size width=300 height=300)
+      (total content size width=300 height=800)
+      (last committed scroll position (0,0))
+      (scrollable area parameters 
+        (horizontal scroll elasticity 1)
+        (vertical scroll elasticity 1)
+        (horizontal scrollbar mode 0)
+        (vertical scrollbar mode 0)
+        (has enabled vertical scrollbar 1)))
+    (fixed node
+      (fixed constraints 
+        (viewport-rect-at-last-layout (0,0) width=320 height=548)
+        (layer-position-at-last-layout (19,211)))
+      (layer top left (19,211)))))
diff --git a/LayoutTests/scrollingcoordinator/ios/fixed-in-overflow-scroll-scrolling-tree.html b/LayoutTests/scrollingcoordinator/ios/fixed-in-overflow-scroll-scrolling-tree.html
new file mode 100644 (file)
index 0000000..adb2c57
--- /dev/null
@@ -0,0 +1,60 @@
+<!DOCTYPE html> <!-- webkit-test-runner [ useFlexibleViewport=true internal:AsyncOverflowScrollingEnabled=true ] -->
+<html>
+<head>
+    <meta name="viewport" content="initial-scale=1.0">
+    <style>
+        .scroller {
+            margin: 10px;
+            height: 300px;
+            width: 300px;
+            border: 1px solid black;
+            overflow: scroll;
+        }
+        
+        .box {
+            position: fixed;
+            margin-top: 200px;
+            width: 200px;
+            height: 200px;
+            background-color: green;
+        }
+        
+        .spacer {
+            height: 800px;
+        }
+    </style>
+    <script>
+        if (window.testRunner) {
+            testRunner.waitUntilDone();
+            testRunner.dumpAsText();
+        }
+
+        function getScrollingTreeUIScript()
+        {
+            return `(function() {
+                return uiController.scrollingTreeAsText;
+            })();`;
+        }
+
+        function doTest()
+        {
+            if (!testRunner.runUIScript)
+                return
+
+            testRunner.runUIScript(getScrollingTreeUIScript(), function(scrollingTree) {
+                document.getElementById('layers').textContent = scrollingTree;
+                testRunner.notifyDone();
+            });
+        }
+        
+        window.addEventListener('load', doTest, false);
+    </script>
+</head>
+<body>
+    <div class="scroller">
+        <div class="box"></div>
+        <div class="spacer"></div>
+    </div>
+<pre id="layers"></pre>
+</body>
+</html>
diff --git a/LayoutTests/scrollingcoordinator/ios/fixed-in-overflow-scroll.html b/LayoutTests/scrollingcoordinator/ios/fixed-in-overflow-scroll.html
new file mode 100644 (file)
index 0000000..f3f8eae
--- /dev/null
@@ -0,0 +1,53 @@
+<!DOCTYPE html> <!-- webkit-test-runner [ useFlexibleViewport=true internal:AsyncOverflowScrollingEnabled=true ] -->
+<html>
+<head>
+    <meta name="viewport" content="initial-scale=1.0">
+    <style>
+        .scroller {
+            margin: 10px;
+            height: 300px;
+            width: 300px;
+            border: 1px solid black;
+            overflow: scroll;
+        }
+        
+        .box {
+            position: fixed;
+            margin-top: 200px;
+            width: 200px;
+            height: 200px;
+            background-color: green;
+        }
+        
+        .spacer {
+            height: 800px;
+        }
+    </style>
+    <script src="../../resources/ui-helper.js"></script>
+    <script>
+        if (window.testRunner)
+            testRunner.waitUntilDone();
+
+        async function doTest()
+        {
+            if (!window.testRunner)
+                return;
+
+            if (!testRunner.runUIScript)
+                return;
+
+            const scrollUpdatesDisabled = true;
+            await UIHelper.immediateScrollElementAtContentPointToOffset(50, 50, 0, 200, scrollUpdatesDisabled);
+            testRunner.notifyDone();
+        }
+        
+        window.addEventListener('load', doTest, false);
+    </script>
+</head>
+<body>
+    <div class="scroller">
+        <div class="box"></div>
+        <div class="spacer"></div>
+    </div>
+</body>
+</html>
index 34fd66b..4ad0413 100644 (file)
@@ -1,5 +1,4 @@
 <!DOCTYPE html> <!-- webkit-test-runner [ useFlexibleViewport=true ] -->
-
 <html>
 <head>
     <meta name="viewport" content="initial-scale=1.0">
@@ -24,7 +23,7 @@
             testRunner.dumpAsText();
         }
 
-        function getScrollingTreeUIScript(x, y)
+        function getScrollingTreeUIScript()
         {
             return `(function() {
                 return uiController.scrollingTreeAsText;
index 8717ee3..540b390 100644 (file)
@@ -1,3 +1,19 @@
+2019-03-26  Simon Fraser  <simon.fraser@apple.com>
+
+        [iOS WK2] position:fixed inside oveflow:scroll is jumpy
+        https://bugs.webkit.org/show_bug.cgi?id=196238
+
+        Reviewed by Antti Koivisto.
+        
+        We were inadvertently making Positioned nodes for position:fixed, which is unnecessary because
+        Fixed nodes handle them, and harmful because they introduced unwanted layer movement.
+
+        Tests: scrollingcoordinator/ios/fixed-in-overflow-scroll-scrolling-tree.html
+               scrollingcoordinator/ios/fixed-in-overflow-scroll.html
+
+        * rendering/RenderLayerCompositor.cpp:
+        (WebCore::RenderLayerCompositor::computeCoordinatedPositioningForLayer const):
+
 2019-03-26  Dean Jackson  <dino@apple.com>
 
         vertexAttribPointer must restrict offset parameter
index e5169e3..cc78435 100644 (file)
@@ -2944,6 +2944,9 @@ ScrollPositioningBehavior RenderLayerCompositor::computeCoordinatedPositioningFo
     if (layer.isRenderViewLayer())
         return ScrollPositioningBehavior::None;
 
+    if (layer.renderer().isFixedPositioned())
+        return ScrollPositioningBehavior::None;
+
     auto* scrollingCoordinator = this->scrollingCoordinator();
     if (!scrollingCoordinator)
         return ScrollPositioningBehavior::None;