Changeset 245854 in webkit

Timestamp:

May 29, 2019, 10:41:28 AM (7 years ago)

Author:

Antti Koivisto

Message:

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 Darin Adler.

Source/WebCore:

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):

LayoutTests:

• 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.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/platform/ios-wk2/scrollingcoordinator/scrolling-tree/sticky-in-overflow-expected.txt (added)
• LayoutTests/scrollingcoordinator/scrolling-tree/sticky-in-overflow-expected.txt (added)
• LayoutTests/scrollingcoordinator/scrolling-tree/sticky-in-overflow.html (added)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/rendering/RenderLayerCompositor.cpp (modified) (2 diffs)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+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 Darin Adler.
+
+        * 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>
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+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 Darin Adler.
+
+        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>
 

trunk/Source/WebCore/rendering/RenderLayerCompositor.cpp

@@ -4212 +4212 @@
     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)) {
@@ -4220 +4228 @@
     } 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))