- Revision
- 293260
- Author
- [email protected]
- Date
- 2022-04-22 15:52:23 -0700 (Fri, 22 Apr 2022)
Log Message
Focusing scroll container before scrolling breaks smooth scrolling
https://bugs.webkit.org/show_bug.cgi?id=239605
Reviewed by Wenson Hsieh.
Source/WebCore:
If script calls focus() and then scrollTo({ behavior: smooth }) in the same runloop, the
scrollToFocusedElementInternal() that happens on a zero-delay timer in FrameView will stop
the animated scroll.
Fix by having programmatic scrolls on Element and DOMWindow cancel any pending focus-related
scroll.
Test: fast/scrolling/programmatic-smooth-scroll-after-focus.html
* dom/Element.cpp:
(WebCore::Element::scrollTo):
* page/DOMWindow.cpp:
(WebCore::DOMWindow::scrollTo const):
* page/FrameView.cpp:
(WebCore::FrameView::maintainScrollPositionAtAnchor):
(WebCore::FrameView::setScrollPosition):
(WebCore::FrameView::cancelScheduledScrollToFocusedElement):
(WebCore::FrameView::scrollToAnchor):
(WebCore::FrameView::setWasScrolledByUser):
* page/FrameView.h:
LayoutTests:
* fast/scrolling/programmatic-smooth-scroll-after-focus-expected.txt: Added.
* fast/scrolling/programmatic-smooth-scroll-after-focus.html: Added.
* platform/win/TestExpectations:
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (293259 => 293260)
--- trunk/LayoutTests/ChangeLog 2022-04-22 22:45:41 UTC (rev 293259)
+++ trunk/LayoutTests/ChangeLog 2022-04-22 22:52:23 UTC (rev 293260)
@@ -1,3 +1,14 @@
+2022-04-22 Simon Fraser <[email protected]>
+
+ Focusing scroll container before scrolling breaks smooth scrolling
+ https://bugs.webkit.org/show_bug.cgi?id=239605
+
+ Reviewed by Wenson Hsieh.
+
+ * fast/scrolling/programmatic-smooth-scroll-after-focus-expected.txt: Added.
+ * fast/scrolling/programmatic-smooth-scroll-after-focus.html: Added.
+ * platform/win/TestExpectations:
+
2022-04-22 Matteo Flores <[email protected]>
REBASLINE: [ Monterey ] fast/text/khmer-lao-font.html is a constant text failure
Added: trunk/LayoutTests/fast/scrolling/programmatic-smooth-scroll-after-focus-expected.txt (0 => 293260)
--- trunk/LayoutTests/fast/scrolling/programmatic-smooth-scroll-after-focus-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/scrolling/programmatic-smooth-scroll-after-focus-expected.txt 2022-04-22 22:52:23 UTC (rev 293260)
@@ -0,0 +1,5 @@
+PASS scrollContainer.scrollLeft is 500
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
Added: trunk/LayoutTests/fast/scrolling/programmatic-smooth-scroll-after-focus.html (0 => 293260)
--- trunk/LayoutTests/fast/scrolling/programmatic-smooth-scroll-after-focus.html (rev 0)
+++ trunk/LayoutTests/fast/scrolling/programmatic-smooth-scroll-after-focus.html 2022-04-22 22:52:23 UTC (rev 293260)
@@ -0,0 +1,54 @@
+<!DOCTYPE html> <!-- webkit-test-runner [ AsyncOverflowScrollingEnabled=true ] -->
+<html>
+<head>
+ <style>
+ .scroll-container {
+ height: 250px;
+ width: 300px;
+ border: 1px solid black;
+ overflow-y: scroll;
+ }
+
+ .contents {
+ height: 100%;
+ width: 400%;
+ background-image: repeating-linear-gradient(to right, white, silver 250px);
+ }
+ </style>
+ <script src=""
+ <script src=""
+ <script>
+ jsTestIsAsync = true;
+
+ async function runTest()
+ {
+ scrollContainer = document.querySelector('.scroll-container');
+
+ eventSender.monitorWheelEvents();
+
+ scrollContainer.focus();
+ scrollContainer.scrollTo({
+ top: 0,
+ left: 500,
+ behavior: 'smooth'
+ });
+
+ await UIHelper.waitForScrollCompletion();
+
+ shouldBe('scrollContainer.scrollLeft', '500');
+ finishJSTest();
+ }
+
+ window.addEventListener('load', () => {
+ setTimeout(runTest, 0);
+ }, false);
+ </script>
+</head>
+<body>
+ <div class="scroll-container" tabindex="0">
+ <div class="contents"></div>
+ </div>
+ <div id="console"></div>
+ <script src=""
+</body>
+</html>
Modified: trunk/LayoutTests/platform/win/TestExpectations (293259 => 293260)
--- trunk/LayoutTests/platform/win/TestExpectations 2022-04-22 22:45:41 UTC (rev 293259)
+++ trunk/LayoutTests/platform/win/TestExpectations 2022-04-22 22:52:23 UTC (rev 293260)
@@ -534,6 +534,9 @@
webkit.org/b/208559 fast/scrolling/rtl-point-in-iframe.html [ Skip ]
fast/scrolling/rtl-scrollbars-alternate-iframe-body-dir-attr-does-not-update-scrollbar-placement.html [ ImageOnlyFailure ]
+# waitForScrollCompletion() not supported
+fast/scrolling/programmatic-smooth-scroll-after-focus.html [ Skip ]
+
# TODO Needs testRunner.enableAutoResizeMode()
fast/autoresize/ [ Skip ]
Modified: trunk/Source/WebCore/ChangeLog (293259 => 293260)
--- trunk/Source/WebCore/ChangeLog 2022-04-22 22:45:41 UTC (rev 293259)
+++ trunk/Source/WebCore/ChangeLog 2022-04-22 22:52:23 UTC (rev 293260)
@@ -1,3 +1,31 @@
+2022-04-22 Simon Fraser <[email protected]>
+
+ Focusing scroll container before scrolling breaks smooth scrolling
+ https://bugs.webkit.org/show_bug.cgi?id=239605
+
+ Reviewed by Wenson Hsieh.
+
+ If script calls focus() and then scrollTo({ behavior: smooth }) in the same runloop, the
+ scrollToFocusedElementInternal() that happens on a zero-delay timer in FrameView will stop
+ the animated scroll.
+
+ Fix by having programmatic scrolls on Element and DOMWindow cancel any pending focus-related
+ scroll.
+
+ Test: fast/scrolling/programmatic-smooth-scroll-after-focus.html
+
+ * dom/Element.cpp:
+ (WebCore::Element::scrollTo):
+ * page/DOMWindow.cpp:
+ (WebCore::DOMWindow::scrollTo const):
+ * page/FrameView.cpp:
+ (WebCore::FrameView::maintainScrollPositionAtAnchor):
+ (WebCore::FrameView::setScrollPosition):
+ (WebCore::FrameView::cancelScheduledScrollToFocusedElement):
+ (WebCore::FrameView::scrollToAnchor):
+ (WebCore::FrameView::setWasScrolledByUser):
+ * page/FrameView.h:
+
2022-04-22 Alan Bujtas <[email protected]>
[LFC][Integration] Add blank LayoutIntegration::FlexLayout
Modified: trunk/Source/WebCore/dom/Element.cpp (293259 => 293260)
--- trunk/Source/WebCore/dom/Element.cpp 2022-04-22 22:45:41 UTC (rev 293259)
+++ trunk/Source/WebCore/dom/Element.cpp 2022-04-22 22:52:23 UTC (rev 293260)
@@ -1100,6 +1100,9 @@
if (this == document().documentElement())
return;
}
+
+ if (auto* view = document().view())
+ view->cancelScheduledScrollToFocusedElement();
document().updateLayoutIgnorePendingStylesheets();
Modified: trunk/Source/WebCore/page/DOMWindow.cpp (293259 => 293260)
--- trunk/Source/WebCore/page/DOMWindow.cpp 2022-04-22 22:45:41 UTC (rev 293259)
+++ trunk/Source/WebCore/page/DOMWindow.cpp 2022-04-22 22:52:23 UTC (rev 293260)
@@ -1738,6 +1738,7 @@
return;
}
+ view->cancelScheduledScrollToFocusedElement();
document()->updateLayoutIgnorePendingStylesheets();
IntPoint layoutPos(view->mapFromCSSToLayoutUnits(scrollToOptions.left.value()), view->mapFromCSSToLayoutUnits(scrollToOptions.top.value()));
Modified: trunk/Source/WebCore/page/FrameView.cpp (293259 => 293260)
--- trunk/Source/WebCore/page/FrameView.cpp 2022-04-22 22:45:41 UTC (rev 293259)
+++ trunk/Source/WebCore/page/FrameView.cpp 2022-04-22 22:52:23 UTC (rev 293260)
@@ -2329,9 +2329,9 @@
m_maintainScrollPositionAnchor = anchorNode;
if (!m_maintainScrollPositionAnchor)
return;
- m_shouldScrollToFocusedElement = false;
- m_delayedScrollToFocusedElementTimer.stop();
+ cancelScheduledScrollToFocusedElement();
+
// We need to update the layout before scrolling, otherwise we could
// really mess things up if an anchor scroll comes at a bad moment.
frame().document()->updateStyleIfNeeded();
@@ -2363,8 +2363,8 @@
setCurrentScrollType(options.type);
m_maintainScrollPositionAnchor = nullptr;
- m_shouldScrollToFocusedElement = false;
- m_delayedScrollToFocusedElementTimer.stop();
+ cancelScheduledScrollToFocusedElement();
+
Page* page = frame().page();
if (page && page->isMonitoringWheelEvents())
scrollAnimator().setWheelEventTestMonitor(page->wheelEventTestMonitor());
@@ -2412,6 +2412,12 @@
m_delayedScrollToFocusedElementTimer.startOneShot(0_s);
}
+void FrameView::cancelScheduledScrollToFocusedElement()
+{
+ m_shouldScrollToFocusedElement = false;
+ m_delayedScrollToFocusedElementTimer.stop();
+}
+
void FrameView::scrollToFocusedElementImmediatelyIfNeeded()
{
if (!m_shouldScrollToFocusedElement)
@@ -3288,8 +3294,7 @@
if (!anchorNode->renderer())
return;
- m_shouldScrollToFocusedElement = false;
- m_delayedScrollToFocusedElementTimer.stop();
+ cancelScheduledScrollToFocusedElement();
LayoutRect rect;
bool insideFixed = false;
@@ -3313,8 +3318,7 @@
// scrollRectToVisible can call into setScrollPosition(), which resets m_maintainScrollPositionAnchor.
LOG_WITH_STREAM(Scrolling, stream << " restoring anchor node to " << anchorNode.get());
m_maintainScrollPositionAnchor = anchorNode;
- m_shouldScrollToFocusedElement = false;
- m_delayedScrollToFocusedElementTimer.stop();
+ cancelScheduledScrollToFocusedElement();
}
void FrameView::updateEmbeddedObject(RenderEmbeddedObject& embeddedObject)
@@ -4322,8 +4326,7 @@
{
LOG(Scrolling, "FrameView::setWasScrolledByUser at %d", wasScrolledByUser);
- m_shouldScrollToFocusedElement = false;
- m_delayedScrollToFocusedElementTimer.stop();
+ cancelScheduledScrollToFocusedElement();
if (currentScrollType() == ScrollType::Programmatic)
return;
m_maintainScrollPositionAnchor = nullptr;
Modified: trunk/Source/WebCore/page/FrameView.h (293259 => 293260)
--- trunk/Source/WebCore/page/FrameView.h 2022-04-22 22:45:41 UTC (rev 293259)
+++ trunk/Source/WebCore/page/FrameView.h 2022-04-22 22:52:23 UTC (rev 293260)
@@ -266,6 +266,7 @@
WEBCORE_EXPORT void setScrollPosition(const ScrollPosition&, const ScrollPositionChangeOptions& = ScrollPositionChangeOptions::createProgrammatic()) final;
void restoreScrollbar();
void scheduleScrollToFocusedElement(SelectionRevealMode);
+ void cancelScheduledScrollToFocusedElement();
void scrollToFocusedElementImmediatelyIfNeeded();
void updateLayerPositionsAfterScrolling() final;
void updateCompositingLayersAfterScrolling() final;
