- Revision
- 242313
- Author
- simon.fra...@apple.com
- Date
- 2019-03-02 11:15:21 -0800 (Sat, 02 Mar 2019)
Log 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.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (242312 => 242313)
--- trunk/LayoutTests/ChangeLog 2019-03-02 17:50:12 UTC (rev 242312)
+++ trunk/LayoutTests/ChangeLog 2019-03-02 19:15:21 UTC (rev 242313)
@@ -1,3 +1,16 @@
+2019-03-02 Simon Fraser <simon.fra...@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_hs...@apple.com>
[Datalist] fast/forms/datalist/datalist-child-validation.html crashes with a debug assertion in isValidFormControlElement()
Modified: trunk/LayoutTests/platform/ios-wk2/TestExpectations (242312 => 242313)
--- trunk/LayoutTests/platform/ios-wk2/TestExpectations 2019-03-02 17:50:12 UTC (rev 242312)
+++ trunk/LayoutTests/platform/ios-wk2/TestExpectations 2019-03-02 19:15:21 UTC (rev 242313)
@@ -12,6 +12,7 @@
fast/viewport/ios [ Pass ]
fast/visual-viewport/ios/ [ Pass ]
scrollingcoordinator [ Pass ]
+scrollingcoordinator/mac [ Skip ]
fast/web-share [ Pass ]
editing/find [ Pass ]
editing/input/ios [ Pass ]
Added: trunk/LayoutTests/scrollingcoordinator/mac/multiple-fixed-expected.html (0 => 242313)
--- trunk/LayoutTests/scrollingcoordinator/mac/multiple-fixed-expected.html (rev 0)
+++ trunk/LayoutTests/scrollingcoordinator/mac/multiple-fixed-expected.html 2019-03-02 19:15:21 UTC (rev 242313)
@@ -0,0 +1,35 @@
+<!DOCTYPE html>
+<html>
+<head>
+ <style>
+ body {
+ height: 2000px;
+ }
+ .fixed {
+ position: fixed;
+ width: 100%;
+ height: 100px;
+ background-color: green;
+ }
+
+ .top {
+ top: 0;
+ }
+ .bottom {
+ bottom: 0;
+ }
+ </style>
+ <script>
+ function startTest()
+ {
+ document.scrollingElement.scrollTop = 2000;
+ }
+
+ window.addEventListener('load', startTest, false);
+ </script>
+</head>
+<body>
+ <div class="fixed top"></div>
+ <div class="fixed bottom"></div>
+</body>
+</html>
Added: trunk/LayoutTests/scrollingcoordinator/mac/multiple-fixed.html (0 => 242313)
--- trunk/LayoutTests/scrollingcoordinator/mac/multiple-fixed.html (rev 0)
+++ trunk/LayoutTests/scrollingcoordinator/mac/multiple-fixed.html 2019-03-02 19:15:21 UTC (rev 242313)
@@ -0,0 +1,53 @@
+<!DOCTYPE html>
+<html>
+<head>
+ <style>
+ body {
+ height: 2000px;
+ }
+ .fixed {
+ position: fixed;
+ width: 100%;
+ height: 100px;
+ background-color: green;
+ }
+
+ .top {
+ top: 0;
+ }
+ .bottom {
+ bottom: 0;
+ }
+ </style>
+ <script>
+ function scrollTest()
+ {
+ eventSender.mouseMoveTo(20, 20);
+ eventSender.mouseScrollByWithWheelAndMomentumPhases(0, -1, "began", "none");
+ eventSender.mouseScrollByWithWheelAndMomentumPhases(0, -100, "changed", "none");
+ eventSender.mouseScrollByWithWheelAndMomentumPhases(0, -100, "none", "continue");
+ eventSender.mouseScrollByWithWheelAndMomentumPhases(0, -100, "none", "continue");
+ eventSender.mouseScrollByWithWheelAndMomentumPhases(0, 0, "none", "end");
+ eventSender.callAfterScrollingCompletes(() => {
+ testRunner.notifyDone();
+ });
+ }
+
+ function startTest()
+ {
+ if (window.eventSender) {
+ testRunner.waitUntilDone();
+
+ eventSender.monitorWheelEvents();
+ setTimeout(scrollTest, 0);
+ }
+ }
+
+ window.addEventListener('load', startTest, false);
+ </script>
+</head>
+<body>
+ <div class="fixed top"></div>
+ <div class="fixed bottom"></div>
+</body>
+</html>
Modified: trunk/Source/WebCore/ChangeLog (242312 => 242313)
--- trunk/Source/WebCore/ChangeLog 2019-03-02 17:50:12 UTC (rev 242312)
+++ trunk/Source/WebCore/ChangeLog 2019-03-02 19:15:21 UTC (rev 242313)
@@ -1,3 +1,20 @@
+2019-03-02 Simon Fraser <simon.fra...@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 <da...@apple.com>
Improve some comments
Modified: trunk/Source/WebCore/page/scrolling/ScrollingTree.cpp (242312 => 242313)
--- trunk/Source/WebCore/page/scrolling/ScrollingTree.cpp 2019-03-02 17:50:12 UTC (rev 242312)
+++ trunk/Source/WebCore/page/scrolling/ScrollingTree.cpp 2019-03-02 19:15:21 UTC (rev 242313)
@@ -265,7 +265,7 @@
notifyRelatedNodesRecursive(changedNode, changedNode, currentFrameLayoutViewport, deltaFromLastCommittedScrollPosition);
}
-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);
@@ -272,13 +272,12 @@
if (!currNode.children())
return;
- auto deltaForChildren = cumulativeDelta;
for (auto& child : *currNode.children()) {
// Never need to cross frame boundaries, since scroll layer adjustments are isolated to each document.
if (is<ScrollingTreeFrameScrollingNode>(child))
continue;
- notifyRelatedNodesRecursive(changedNode, *child, layoutViewport, deltaForChildren);
+ notifyRelatedNodesRecursive(changedNode, *child, layoutViewport, cumulativeDelta);
}
}
Modified: trunk/Source/WebCore/page/scrolling/ScrollingTree.h (242312 => 242313)
--- trunk/Source/WebCore/page/scrolling/ScrollingTree.h 2019-03-02 17:50:12 UTC (rev 242312)
+++ trunk/Source/WebCore/page/scrolling/ScrollingTree.h 2019-03-02 19:15:21 UTC (rev 242313)
@@ -161,7 +161,7 @@
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;
