Attachment 370850 Details for Bug 198329 – patch

[patch] patch

sticky-positioned-order.patch (text/plain), 9.32 KB, created by Antti Koivisto on 2019-05-29 08:15:04 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Antti Koivisto

Created: 2019-05-29 08:15:04 PDT

Size: 9.32 KB

patch

obsolete

>Index: Source/WebCore/ChangeLog
>===================================================================
>--- Source/WebCore/ChangeLog	(revision 245849)
>+++ Source/WebCore/ChangeLog	(working copy)
>@@ -1,3 +1,27 @@
>+2019-05-29  Antti Koivisto  <antti@apple.com>
>+
>+        Scrolling node ordering wrong when a layer has both positioning and fixed/sticky node
>+        https://bugs.webkit.org/show_bug.cgi?id=198329
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Test: scrollingcoordinator/scrolling-tree/sticky-in-overflow.html
>+
>+        With sticky positioning in non-stacking context overflow you currently get structure like
>+
>+        FrameScrollingNode
>+          OverflowScrollingNode
>+          StickyNode
>+            PositionedNode
>+
>+        where StickyNode and PositionedNode reference the same layer. Sticky doesn't get applied at all when the overflow moves.
>+
>+        This patch reverses the order of sticky and positioned. It doesn't fix sticky positioning during scrolling yet,
>+        but it does make it less jumpy. It is a prerequisite for the full fix.
>+
>+        * rendering/RenderLayerCompositor.cpp:
>+        (WebCore::RenderLayerCompositor::updateScrollCoordinationForLayer):
>+
> 2019-05-29  Ludovico de Nittis  <ludovico.denittis@collabora.com>
> 
>         PlatformEventKeyboardGtk still uses old key aliases
>Index: Source/WebCore/rendering/RenderLayerCompositor.cpp
>===================================================================
>--- Source/WebCore/rendering/RenderLayerCompositor.cpp	(revision 245848)
>+++ Source/WebCore/rendering/RenderLayerCompositor.cpp	(working copy)
>@@ -4211,6 +4211,14 @@ ScrollingNodeID RenderLayerCompositor::u
>     ScrollingTreeState childTreeState;
>     ScrollingTreeState* currentTreeState = &treeState;
> 
>+    // If there's a positioning node, it's the parent scrolling node for fixed/sticky/scrolling/frame hosting.
>+    if (roles.contains(ScrollCoordinationRole::Positioning)) {
>+        newNodeID = updateScrollingNodeForPositioningRole(layer, *currentTreeState, changes);
>+        childTreeState.parentNodeID = newNodeID;
>+        currentTreeState = &childTreeState;
>+    } else
>+        detachScrollCoordinatedLayer(layer, ScrollCoordinationRole::Positioning);
>+
>     // If is fixed or sticky, it's the parent scrolling node for scrolling/frame hosting.
>     if (roles.contains(ScrollCoordinationRole::ViewportConstrained)) {
>         newNodeID = updateScrollingNodeForViewportConstrainedRole(layer, *currentTreeState, changes);
>@@ -4220,14 +4228,6 @@ ScrollingNodeID RenderLayerCompositor::u
>     } else
>         detachScrollCoordinatedLayer(layer, ScrollCoordinationRole::ViewportConstrained);
> 
>-    // If there's a positioning node, it's the parent scrolling node for scrolling/frame hosting.
>-    if (roles.contains(ScrollCoordinationRole::Positioning)) {
>-        newNodeID = updateScrollingNodeForPositioningRole(layer, *currentTreeState, changes);
>-        childTreeState.parentNodeID = newNodeID;
>-        currentTreeState = &childTreeState;
>-    } else
>-        detachScrollCoordinatedLayer(layer, ScrollCoordinationRole::Positioning);
>-
>     if (roles.contains(ScrollCoordinationRole::Scrolling))
>         newNodeID = updateScrollingNodeForScrollingRole(layer, *currentTreeState, changes);
>     else
>Index: LayoutTests/ChangeLog
>===================================================================
>--- LayoutTests/ChangeLog	(revision 245848)
>+++ LayoutTests/ChangeLog	(working copy)
>@@ -1,3 +1,14 @@
>+2019-05-29  Antti Koivisto  <antti@apple.com>
>+
>+        Scrolling node ordering wrong when a layer has both positioning and fixed/sticky node
>+        https://bugs.webkit.org/show_bug.cgi?id=198329
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * platform/ios-wk2/scrollingcoordinator/scrolling-tree/sticky-in-overflow-expected.txt: Added.
>+        * scrollingcoordinator/scrolling-tree/sticky-in-overflow-expected.txt: Added.
>+        * scrollingcoordinator/scrolling-tree/sticky-in-overflow.html: Added.
>+
> 2019-05-28  Yacine Bandou  <yacine.bandou@softathome.com>
> 
>         [MSE][GStreamer] update the readyState correctly in MediaPlayerPrivateGStreamerMSE
>Index: LayoutTests/platform/ios-wk2/scrollingcoordinator/scrolling-tree/sticky-in-overflow-expected.txt
>===================================================================
>--- LayoutTests/platform/ios-wk2/scrollingcoordinator/scrolling-tree/sticky-in-overflow-expected.txt	(nonexistent)
>+++ LayoutTests/platform/ios-wk2/scrollingcoordinator/scrolling-tree/sticky-in-overflow-expected.txt	(working copy)
>@@ -0,0 +1,48 @@
>+Sticky content
>+
>+(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 2
>+    (Overflow scrolling node
>+      (scrollable area size 400 300)
>+      (contents size 400 1100)
>+      (parent relative scrollable rect at (8,8) size 400x300)
>+      (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 (50,50))
>+        (positioning-behavior moves))
>+      (related overflow nodes 1)
>+      (children 1
>+        (Sticky node
>+          (anchor edges: AnchorEdgeTop )
>+          (top offset 10.00)
>+          (containing block rect at (50,50) size 300x1000)
>+          (sticky box rect at (50,50) size 300x20)
>+          (constraining rect at (0,0) size 400x300)
>+          (sticky offset at last layout width=0 height=0)
>+          (layer position at last layout (50,50))
>+        )
>+      )
>+    )
>+  )
>+)
>+
>+
>Index: LayoutTests/scrollingcoordinator/scrolling-tree/sticky-in-overflow-expected.txt
>===================================================================
>--- LayoutTests/scrollingcoordinator/scrolling-tree/sticky-in-overflow-expected.txt	(nonexistent)
>+++ LayoutTests/scrollingcoordinator/scrolling-tree/sticky-in-overflow-expected.txt	(working copy)
>@@ -0,0 +1,48 @@
>+Sticky content
>+
>+(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 2
>+    (Overflow scrolling node
>+      (scrollable area size 385 285)
>+      (contents size 385 1100)
>+      (parent relative scrollable rect at (8,8) size 385x285)
>+      (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 (50,50))
>+        (positioning-behavior moves))
>+      (related overflow nodes 1)
>+      (children 1
>+        (Sticky node
>+          (anchor edges: AnchorEdgeTop )
>+          (top offset 10.00)
>+          (containing block rect at (50,50) size 285x1000)
>+          (sticky box rect at (50,50) size 285x18)
>+          (constraining rect at (0,0) size 385x285)
>+          (sticky offset at last layout width=0 height=0)
>+          (layer position at last layout (50,50))
>+        )
>+      )
>+    )
>+  )
>+)
>+
>+
>Index: LayoutTests/scrollingcoordinator/scrolling-tree/sticky-in-overflow.html
>===================================================================
>--- LayoutTests/scrollingcoordinator/scrolling-tree/sticky-in-overflow.html	(nonexistent)
>+++ LayoutTests/scrollingcoordinator/scrolling-tree/sticky-in-overflow.html	(working copy)
>@@ -0,0 +1,51 @@
>+<!DOCTYPE html> 
>+<html>
>+<head>
>+    <title>
>+        Check that overflow is nested inside fixed
>+    </title>
>+    <script>
>+        if (window.testRunner)
>+            testRunner.dumpAsText();
>+
>+        function doTest() {
>+            if (window.internals)
>+                document.getElementById('scrollingTree').innerText = window.internals.scrollingStateTreeAsText() + "\n";
>+
>+            if (window.testRunner)
>+                testRunner.notifyDone();
>+        }
>+
>+        window.addEventListener('load', doTest, false);
>+    </script>
>+    <style>
>+        .sticky {
>+            position: -webkit-sticky;
>+            background-color: silver;
>+            top: 10px;
>+        }
>+
>+        .scrolling {
>+            width: 400px;
>+            height: 300px;
>+            overflow: scroll;
>+        }
>+
>+        .scrolling-content {
>+            margin: 50px;
>+            height: 1000px;
>+        }
>+    </style>
>+</head>
>+<body>
>+    <div class="scrolling">
>+        <div class="scrolling-content">
>+            <div class="sticky">
>+                Sticky content
>+            </div>
>+        </div>
>+    </div>
>+    <pre id="scrollingTree"></pre>
>+</body>
>+</html>
>+

Actions: View | Formatted Diff | Diff

Attachments on bug 198329: 370850