- Revision
- 243513
- Author
- simon.fra...@apple.com
- Date
- 2019-03-26 12:23:52 -0700 (Tue, 26 Mar 2019)
Log Message
[iOS WK2] position:fixed inside oveflow:scroll is jumpy
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:
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (243512 => 243513)
--- trunk/LayoutTests/ChangeLog 2019-03-26 19:18:46 UTC (rev 243512)
+++ trunk/LayoutTests/ChangeLog 2019-03-26 19:23:52 UTC (rev 243513)
@@ -1,3 +1,24 @@
+2019-03-26 Simon Fraser <simon.fra...@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
Modified: trunk/LayoutTests/resources/ui-helper.js (243512 => 243513)
--- trunk/LayoutTests/resources/ui-helper.js 2019-03-26 19:18:46 UTC (rev 243512)
+++ trunk/LayoutTests/resources/ui-helper.js 2019-03-26 19:23:52 UTC (rev 243513)
@@ -286,7 +286,7 @@
});
}
- static immediateScrollElementAtContentPointToOffset(x, y, scrollX, scrollY)
+ static immediateScrollElementAtContentPointToOffset(x, y, scrollX, scrollY, scrollUpdatesDisabled = false)
{
if (!this.isWebKit2())
return Promise.resolve();
@@ -293,6 +293,7 @@
return new Promise(resolve => {
testRunner.runUIScript(`
+ uiController.scrollUpdatesDisabled = ${scrollUpdatesDisabled};
uiController.immediateScrollElementAtContentPointToOffset(${x}, ${y}, ${scrollX}, ${scrollY});`, resolve);
});
}
Added: trunk/LayoutTests/scrollingcoordinator/ios/fixed-in-overflow-scroll-expected.html (0 => 243513)
--- trunk/LayoutTests/scrollingcoordinator/ios/fixed-in-overflow-scroll-expected.html (rev 0)
+++ trunk/LayoutTests/scrollingcoordinator/ios/fixed-in-overflow-scroll-expected.html 2019-03-26 19:23:52 UTC (rev 243513)
@@ -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=""
+ <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>
Added: trunk/LayoutTests/scrollingcoordinator/ios/fixed-in-overflow-scroll-scrolling-tree-expected.txt (0 => 243513)
--- trunk/LayoutTests/scrollingcoordinator/ios/fixed-in-overflow-scroll-scrolling-tree-expected.txt (rev 0)
+++ trunk/LayoutTests/scrollingcoordinator/ios/fixed-in-overflow-scroll-scrolling-tree-expected.txt 2019-03-26 19:23:52 UTC (rev 243513)
@@ -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)))))
Copied: trunk/LayoutTests/scrollingcoordinator/ios/fixed-in-overflow-scroll-scrolling-tree.html (from rev 243512, trunk/LayoutTests/scrollingcoordinator/ios/ui-scrolling-tree.html) (0 => 243513)
--- trunk/LayoutTests/scrollingcoordinator/ios/fixed-in-overflow-scroll-scrolling-tree.html (rev 0)
+++ trunk/LayoutTests/scrollingcoordinator/ios/fixed-in-overflow-scroll-scrolling-tree.html 2019-03-26 19:23:52 UTC (rev 243513)
@@ -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>
Added: trunk/LayoutTests/scrollingcoordinator/ios/fixed-in-overflow-scroll.html (0 => 243513)
--- trunk/LayoutTests/scrollingcoordinator/ios/fixed-in-overflow-scroll.html (rev 0)
+++ trunk/LayoutTests/scrollingcoordinator/ios/fixed-in-overflow-scroll.html 2019-03-26 19:23:52 UTC (rev 243513)
@@ -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=""
+ <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>
Modified: trunk/LayoutTests/scrollingcoordinator/ios/ui-scrolling-tree.html (243512 => 243513)
--- trunk/LayoutTests/scrollingcoordinator/ios/ui-scrolling-tree.html 2019-03-26 19:18:46 UTC (rev 243512)
+++ trunk/LayoutTests/scrollingcoordinator/ios/ui-scrolling-tree.html 2019-03-26 19:23:52 UTC (rev 243513)
@@ -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;
Modified: trunk/Source/WebCore/ChangeLog (243512 => 243513)
--- trunk/Source/WebCore/ChangeLog 2019-03-26 19:18:46 UTC (rev 243512)
+++ trunk/Source/WebCore/ChangeLog 2019-03-26 19:23:52 UTC (rev 243513)
@@ -1,3 +1,19 @@
+2019-03-26 Simon Fraser <simon.fra...@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 <d...@apple.com>
vertexAttribPointer must restrict offset parameter
Modified: trunk/Source/WebCore/rendering/RenderLayerCompositor.cpp (243512 => 243513)
--- trunk/Source/WebCore/rendering/RenderLayerCompositor.cpp 2019-03-26 19:18:46 UTC (rev 243512)
+++ trunk/Source/WebCore/rendering/RenderLayerCompositor.cpp 2019-03-26 19:23:52 UTC (rev 243513)
@@ -2944,6 +2944,9 @@
if (layer.isRenderViewLayer())
return ScrollPositioningBehavior::None;
+ if (layer.renderer().isFixedPositioned())
+ return ScrollPositioningBehavior::None;
+
auto* scrollingCoordinator = this->scrollingCoordinator();
if (!scrollingCoordinator)
return ScrollPositioningBehavior::None;
