Changeset 242313 in webkit

Timestamp:

Mar 2, 2019, 11:15:21 AM (6 years ago)

Author:

Simon Fraser

Message:

REGRESSION (r242132): Incorrect positioning with multiple position:fixed elements
​https://bugs.webkit.org/show_bug.cgi?id=195246

Reviewed by Frederic Wang.

Source/WebCore:

r242132 introduced a bug where the management of 'cumulativeDelta' in ScrollingTree::notifyRelatedNodesRecursive
was incorrect. This value should propagate from ancestors to descendants, but not between siblings in the scrolling
tree, which it did, causing sibling position:fixed to behave incorrectly.

Test: scrollingcoordinator/mac/multiple-fixed.html

• page/scrolling/ScrollingTree.cpp:

(WebCore::ScrollingTree::notifyRelatedNodesRecursive):

• page/scrolling/ScrollingTree.h:

LayoutTests:

Test that uses eventSender to scroll (and is thus macOS-only).

• platform/ios-wk2/TestExpectations:
• scrollingcoordinator/mac/multiple-fixed-expected.html: Added.
• scrollingcoordinator/mac/multiple-fixed.html: Added.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/platform/ios-wk2/TestExpectations (modified) (1 diff)
• LayoutTests/scrollingcoordinator/mac (added)
• LayoutTests/scrollingcoordinator/mac/multiple-fixed-expected.html (added)
• LayoutTests/scrollingcoordinator/mac/multiple-fixed.html (added)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/page/scrolling/ScrollingTree.cpp (modified) (3 diffs)
• Source/WebCore/page/scrolling/ScrollingTree.h (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2019-03-02  Simon Fraser  <simon.fraser@apple.com>
+
+        REGRESSION (r242132): Incorrect positioning with multiple position:fixed elements
+        https://bugs.webkit.org/show_bug.cgi?id=195246
+
+        Reviewed by Frederic Wang.
+
+        Test that uses eventSender to scroll (and is thus macOS-only).
+
+        * platform/ios-wk2/TestExpectations:
+        * scrollingcoordinator/mac/multiple-fixed-expected.html: Added.
+        * scrollingcoordinator/mac/multiple-fixed.html: Added.
+
 2019-03-01  Wenson Hsieh  <wenson_hsieh@apple.com>
 

trunk/LayoutTests/platform/ios-wk2/TestExpectations

@@ -13 +13 @@
 fast/visual-viewport/ios/ [ Pass ]
 scrollingcoordinator [ Pass ]
+scrollingcoordinator/mac [ Skip ]
 fast/web-share [ Pass ]
 editing/find [ Pass ]

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2019-03-02  Simon Fraser  <simon.fraser@apple.com>
+
+        REGRESSION (r242132): Incorrect positioning with multiple position:fixed elements
+        https://bugs.webkit.org/show_bug.cgi?id=195246
+
+        Reviewed by Frederic Wang.
+
+        r242132 introduced a bug where the management of 'cumulativeDelta' in ScrollingTree::notifyRelatedNodesRecursive
+        was incorrect. This value should propagate from ancestors to descendants, but not between siblings in the scrolling
+        tree, which it did, causing sibling position:fixed to behave incorrectly.
+
+        Test: scrollingcoordinator/mac/multiple-fixed.html
+
+        * page/scrolling/ScrollingTree.cpp:
+        (WebCore::ScrollingTree::notifyRelatedNodesRecursive):
+        * page/scrolling/ScrollingTree.h:
+
 2019-03-02  Darin Adler  <darin@apple.com>
 

trunk/Source/WebCore/page/scrolling/ScrollingTree.cpp

@@ -266 +266 @@
 }
 
-void ScrollingTree::notifyRelatedNodesRecursive(ScrollingTreeScrollingNode& changedNode, ScrollingTreeNode& currNode, const FloatRect& layoutViewport, FloatSize& cumulativeDelta)
+void ScrollingTree::notifyRelatedNodesRecursive(ScrollingTreeScrollingNode& changedNode, ScrollingTreeNode& currNode, const FloatRect& layoutViewport, FloatSize cumulativeDelta)
 {
     currNode.relatedNodeScrollPositionDidChange(changedNode, layoutViewport, cumulativeDelta);
@@ -273 +273 @@
         return;
     
-    auto deltaForChildren = cumulativeDelta;
     for (auto& child : *currNode.children()) {
         // Never need to cross frame boundaries, since scroll layer adjustments are isolated to each document.
@@ -279 +278 @@
             continue;
 
-        notifyRelatedNodesRecursive(changedNode, *child, layoutViewport, deltaForChildren);
+        notifyRelatedNodesRecursive(changedNode, *child, layoutViewport, cumulativeDelta);
     }
 }

trunk/Source/WebCore/page/scrolling/ScrollingTree.h

@@ -162 +162 @@
     ScrollingTreeNode* nodeForID(ScrollingNodeID) const;
 
-    void notifyRelatedNodesRecursive(ScrollingTreeScrollingNode& changedNode, ScrollingTreeNode& currNode, const FloatRect& layoutViewport, FloatSize& cumulativeDelta);
+    void notifyRelatedNodesRecursive(ScrollingTreeScrollingNode& changedNode, ScrollingTreeNode& currNode, const FloatRect& layoutViewport, FloatSize cumulativeDelta);
 
     RefPtr<ScrollingTreeNode> m_rootNode;