Diff
Modified: trunk/LayoutTests/ChangeLog (284083 => 284084)
--- trunk/LayoutTests/ChangeLog 2021-10-13 07:53:47 UTC (rev 284083)
+++ trunk/LayoutTests/ChangeLog 2021-10-13 07:59:14 UTC (rev 284084)
@@ -1,3 +1,19 @@
+2021-10-13 Martin Robinson <[email protected]>
+
+ Sticky element inside another sticky element does not redraw properly on scroll
+ https://bugs.webkit.org/show_bug.cgi?id=199915
+ <rdar://problem/53375284>
+
+ Reviewed by Simon Fraser.
+
+ When calculating layer position for sticky nodes in the scrolling tree, take into
+ account situations where the sticky parents of sticky nodes don't stick (due to
+ sizing). This was handled properly in layout, but not in the scrolling tree, which
+ caused these kinds of nodes to jump around.
+
+ * scrollingcoordinator/mac/nested-sticky-with-nonsticking-sticky-parent-expected.html: Added.
+ * scrollingcoordinator/mac/nested-sticky-with-nonsticking-sticky-parent.html: Added.
+
2021-10-12 Arcady Goldmints-Orlov <[email protected]>
[GTK] Update test expectations for CSS web platform tests.
Added: trunk/LayoutTests/scrollingcoordinator/mac/nested-sticky-with-nonsticking-sticky-parent-expected.html (0 => 284084)
--- trunk/LayoutTests/scrollingcoordinator/mac/nested-sticky-with-nonsticking-sticky-parent-expected.html (rev 0)
+++ trunk/LayoutTests/scrollingcoordinator/mac/nested-sticky-with-nonsticking-sticky-parent-expected.html 2021-10-13 07:59:14 UTC (rev 284084)
@@ -0,0 +1,30 @@
+<!DOCTYPE html> <!-- webkit-test-runner [ AsyncOverflowScrollingEnabled=true ] -->
+<html>
+ <head>
+ <style>
+ html, body {
+ margin: 0px;
+ }
+ ::-webkit-scrollbar {
+ display: none;
+ }
+ .sticky {
+ position: sticky;
+ top: 0px;
+ }
+ .parent {
+ height: 150vh;
+ }
+ .child {
+ background: green;
+ height: 4vh;
+ width: 4vh;
+ }
+ <script src=""
+ </style>
+ </head>
+ <body>
+ <div class="parent">
+ <div class="sticky child"></div>
+ </body>
+</html>
Added: trunk/LayoutTests/scrollingcoordinator/mac/nested-sticky-with-nonsticking-sticky-parent.html (0 => 284084)
--- trunk/LayoutTests/scrollingcoordinator/mac/nested-sticky-with-nonsticking-sticky-parent.html (rev 0)
+++ trunk/LayoutTests/scrollingcoordinator/mac/nested-sticky-with-nonsticking-sticky-parent.html 2021-10-13 07:59:14 UTC (rev 284084)
@@ -0,0 +1,67 @@
+<!DOCTYPE html> <!-- webkit-test-runner [ AsyncOverflowScrollingEnabled=true ] -->
+<html class="reftest-wait">
+ <head>
+ <style>
+ html, body {
+ margin: 0px;
+ }
+ ::-webkit-scrollbar {
+ display: none;
+ }
+ .sticky {
+ position: sticky;
+ top: 0px;
+ }
+ .parent {
+ height: 150vh;
+ }
+ .child {
+ background: green;
+ height: 4vh;
+ width: 4vh;
+ }
+ </style>
+ <script src=""
+ <script>
+ async function runTest() {
+ if (!window.testRunner) {
+ return;
+ }
+
+ try {
+ await UIHelper.delayFor(0);
+
+ eventSender.monitorWheelEvents();
+ eventSender.mouseMoveTo(10, 10);
+
+ // `direction` is a two-element array with a one in the appropriate direction.
+ let scrollMotions = [
+ [0, -1, 'began', 'none'],
+ [0, -1, 'changed', 'none'],
+ [0, -1, 'changed', 'none'],
+ [0, -1, 'changed', 'none'],
+ [0, 0, 'ended', 'none'],
+ [0, -1, 'none', 'begin'],
+ [0, -1, 'none', 'continue'],
+ [0, 0, 'none', 'end'],
+ ];
+ scrollMotions.forEach((callArguments) => {
+ eventSender.mouseScrollByWithWheelAndMomentumPhases(...callArguments);
+ });
+
+ await UIHelper.waitForScrollCompletion();
+ } catch (e) {
+ console.log(e);
+ } finally {
+ document.documentElement.classList.remove("reftest-wait");$
+ }
+ }
+ </script>
+ </head>
+ <body _onload_="runTest();">
+ <div class="sticky parent">
+ <div class="sticky child"></div>
+ </div>
+ <div id="console"></div>
+ </body>
+</html>
Modified: trunk/Source/WebCore/ChangeLog (284083 => 284084)
--- trunk/Source/WebCore/ChangeLog 2021-10-13 07:53:47 UTC (rev 284083)
+++ trunk/Source/WebCore/ChangeLog 2021-10-13 07:59:14 UTC (rev 284084)
@@ -1,3 +1,29 @@
+2021-10-13 Martin Robinson <[email protected]>
+
+ Sticky element inside another sticky element does not redraw properly on scroll
+ https://bugs.webkit.org/show_bug.cgi?id=199915
+ <rdar://problem/53375284>
+
+ Reviewed by Simon Fraser.
+
+ When calculating layer position for sticky nodes in the scrolling tree, take into
+ account situations where the sticky parents of sticky nodes don't stick (due to
+ sizing). This was handled properly in layout, but not in the scrolling tree, which
+ caused these kinds of nodes to jump around.
+
+ Test: scrollingcoordinator/mac/nested-sticky-with-nonsticking-sticky-parent.html
+
+ * page/scrolling/ScrollingStateStickyNode.cpp:
+ (WebCore::ScrollingStateStickyNode::computeLayerPosition const):
+ (WebCore::ScrollingStateStickyNode::scrollDeltaSinceLastCommit const):
+ * page/scrolling/ScrollingStateStickyNode.h:
+ * page/scrolling/cocoa/ScrollingTreeStickyNode.mm:
+ (WebCore::ScrollingTreeStickyNode::computeLayerPosition const):
+ * page/scrolling/nicosia/ScrollingTreeStickyNode.cpp:
+ (WebCore::ScrollingTreeStickyNode::computeLayerPosition const):
+ (WebCore::ScrollingTreeStickyNode::scrollDeltaSinceLastCommit const):
+ * page/scrolling/nicosia/ScrollingTreeStickyNode.h:
+
2021-10-12 Jer Noble <[email protected]>
[Build-time perf] Forward-declare more things in Document.h
Modified: trunk/Source/WebCore/page/scrolling/ScrollingStateStickyNode.cpp (284083 => 284084)
--- trunk/Source/WebCore/page/scrolling/ScrollingStateStickyNode.cpp 2021-10-13 07:53:47 UTC (rev 284083)
+++ trunk/Source/WebCore/page/scrolling/ScrollingStateStickyNode.cpp 2021-10-13 07:59:14 UTC (rev 284084)
@@ -86,6 +86,7 @@
FloatPoint ScrollingStateStickyNode::computeLayerPosition(const LayoutRect& viewportRect) const
{
// This logic follows ScrollingTreeStickyNode::computeLayerPosition().
+ FloatSize offsetFromStickyAncestors;
auto computeLayerPositionForScrollingNode = [&](ScrollingStateNode& scrollingStateNode) {
FloatRect constrainingRect;
if (is<ScrollingStateFrameScrollingNode>(scrollingStateNode))
@@ -94,6 +95,7 @@
auto& overflowScrollingNode = downcast<ScrollingStateOverflowScrollingNode>(scrollingStateNode);
constrainingRect = FloatRect(overflowScrollingNode.scrollPosition(), m_constraints.constrainingRectAtLastLayout().size());
}
+ constrainingRect.move(offsetFromStickyAncestors);
return m_constraints.layerPositionForConstrainingRect(constrainingRect);
};
@@ -110,7 +112,10 @@
if (is<ScrollingStateScrollingNode>(*ancestor))
return computeLayerPositionForScrollingNode(*ancestor);
- if (is<ScrollingStateFixedNode>(*ancestor) || is<ScrollingStateStickyNode>(*ancestor)) {
+ if (is<ScrollingStateStickyNode>(*ancestor))
+ offsetFromStickyAncestors += downcast<ScrollingStateStickyNode>(*ancestor).scrollDeltaSinceLastCommit(viewportRect);
+
+ if (is<ScrollingStateFixedNode>(*ancestor)) {
// FIXME: Do we need scrolling tree nodes at all for nested cases?
return m_constraints.layerPositionAtLastLayout();
}
@@ -143,6 +148,12 @@
}
}
+FloatSize ScrollingStateStickyNode::scrollDeltaSinceLastCommit(const LayoutRect& viewportRect) const
+{
+ auto layerPosition = computeLayerPosition(viewportRect);
+ return layerPosition - m_constraints.layerPositionAtLastLayout();
+}
+
void ScrollingStateStickyNode::dumpProperties(TextStream& ts, ScrollingStateTreeAsTextBehavior behavior) const
{
ts << "Sticky node";
Modified: trunk/Source/WebCore/page/scrolling/ScrollingStateStickyNode.h (284083 => 284084)
--- trunk/Source/WebCore/page/scrolling/ScrollingStateStickyNode.h 2021-10-13 07:53:47 UTC (rev 284083)
+++ trunk/Source/WebCore/page/scrolling/ScrollingStateStickyNode.h 2021-10-13 07:59:14 UTC (rev 284084)
@@ -53,6 +53,7 @@
FloatPoint computeLayerPosition(const LayoutRect& viewportRect) const;
void reconcileLayerPositionForViewportRect(const LayoutRect& viewportRect, ScrollingLayerPositionAction) final;
+ FloatSize scrollDeltaSinceLastCommit(const LayoutRect& viewportRect) const;
void dumpProperties(WTF::TextStream&, ScrollingStateTreeAsTextBehavior) const final;
OptionSet<ScrollingStateNode::Property> applicableProperties() const final;
Modified: trunk/Source/WebCore/page/scrolling/cocoa/ScrollingTreeStickyNode.mm (284083 => 284084)
--- trunk/Source/WebCore/page/scrolling/cocoa/ScrollingTreeStickyNode.mm 2021-10-13 07:53:47 UTC (rev 284083)
+++ trunk/Source/WebCore/page/scrolling/cocoa/ScrollingTreeStickyNode.mm 2021-10-13 07:59:14 UTC (rev 284084)
@@ -69,6 +69,7 @@
FloatPoint ScrollingTreeStickyNode::computeLayerPosition() const
{
+ FloatSize offsetFromStickyAncestors;
auto computeLayerPositionForScrollingNode = [&](ScrollingTreeNode& scrollingNode) {
FloatRect constrainingRect;
if (is<ScrollingTreeFrameScrollingNode>(scrollingNode)) {
@@ -79,6 +80,7 @@
constrainingRect = m_constraints.constrainingRectAtLastLayout();
constrainingRect.move(overflowScrollingNode.scrollDeltaSinceLastCommit());
}
+ constrainingRect.move(-offsetFromStickyAncestors);
return m_constraints.layerPositionForConstrainingRect(constrainingRect);
};
@@ -95,7 +97,10 @@
if (is<ScrollingTreeScrollingNode>(*ancestor))
return computeLayerPositionForScrollingNode(*ancestor);
- if (is<ScrollingTreeFixedNode>(*ancestor) || is<ScrollingTreeStickyNode>(*ancestor)) {
+ if (is<ScrollingTreeStickyNode>(*ancestor))
+ offsetFromStickyAncestors += downcast<ScrollingTreeStickyNode>(*ancestor).scrollDeltaSinceLastCommit();
+
+ if (is<ScrollingTreeFixedNode>(*ancestor)) {
// FIXME: Do we need scrolling tree nodes at all for nested cases?
return m_constraints.layerPositionAtLastLayout();
}
Modified: trunk/Source/WebCore/page/scrolling/nicosia/ScrollingTreeStickyNode.cpp (284083 => 284084)
--- trunk/Source/WebCore/page/scrolling/nicosia/ScrollingTreeStickyNode.cpp 2021-10-13 07:53:47 UTC (rev 284083)
+++ trunk/Source/WebCore/page/scrolling/nicosia/ScrollingTreeStickyNode.cpp 2021-10-13 07:59:14 UTC (rev 284084)
@@ -111,6 +111,7 @@
FloatPoint ScrollingTreeStickyNode::computeLayerPosition() const
{
+ FloatSize offsetFromStickyAncestors;
auto computeLayerPositionForScrollingNode = [&](ScrollingTreeNode& scrollingNode) {
FloatRect constrainingRect;
if (is<ScrollingTreeFrameScrollingNode>(scrollingNode)) {
@@ -121,6 +122,7 @@
constrainingRect = m_constraints.constrainingRectAtLastLayout();
constrainingRect.move(overflowScrollingNode.scrollDeltaSinceLastCommit());
}
+ constrainingRect.move(-offsetFromStickyAncestors);
return m_constraints.layerPositionForConstrainingRect(constrainingRect);
};
@@ -137,7 +139,10 @@
if (is<ScrollingTreeScrollingNode>(*ancestor))
return computeLayerPositionForScrollingNode(*ancestor);
- if (is<ScrollingTreeFixedNode>(*ancestor) || is<ScrollingTreeStickyNode>(*ancestor)) {
+ if (is<ScrollingTreeStickyNode>(*ancestor))
+ offsetFromStickyAncestors += downcast<ScrollingTreeStickyNode>(*ancestor).scrollDeltaSinceLastCommit();
+
+ if (is<ScrollingTreeFixedNode>(*ancestor)) {
// FIXME: Do we need scrolling tree nodes at all for nested cases?
return m_constraints.layerPositionAtLastLayout();
}
@@ -146,6 +151,12 @@
return m_constraints.layerPositionAtLastLayout();
}
+FloatSize ScrollingTreeStickyNode::scrollDeltaSinceLastCommit() const
+{
+ auto layerPosition = computeLayerPosition();
+ return layerPosition - m_constraints.layerPositionAtLastLayout();
+}
+
} // namespace WebCore
#endif // ENABLE(ASYNC_SCROLLING) && USE(NICOSIA)
Modified: trunk/Source/WebCore/page/scrolling/nicosia/ScrollingTreeStickyNode.h (284083 => 284084)
--- trunk/Source/WebCore/page/scrolling/nicosia/ScrollingTreeStickyNode.h 2021-10-13 07:53:47 UTC (rev 284083)
+++ trunk/Source/WebCore/page/scrolling/nicosia/ScrollingTreeStickyNode.h 2021-10-13 07:59:14 UTC (rev 284084)
@@ -54,6 +54,7 @@
void dumpProperties(WTF::TextStream&, ScrollingStateTreeAsTextBehavior) const override;
FloatPoint computeLayerPosition() const;
+ FloatSize scrollDeltaSinceLastCommit() const;
StickyPositionViewportConstraints m_constraints;
RefPtr<Nicosia::CompositionLayer> m_layer;
