Changeset 293260 in webkit

Timestamp:

Apr 22, 2022 3:52:23 PM (3 months ago)

Author:

Simon Fraser

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:

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/fast/scrolling/programmatic-smooth-scroll-after-focus-expected.txt (added)
• LayoutTests/fast/scrolling/programmatic-smooth-scroll-after-focus.html (added)
• LayoutTests/platform/win/TestExpectations (modified) (1 diff)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/dom/Element.cpp (modified) (1 diff)
• Source/WebCore/page/DOMWindow.cpp (modified) (1 diff)
• Source/WebCore/page/FrameView.cpp (modified) (6 diffs)
• Source/WebCore/page/FrameView.h (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2022-04-22  Simon Fraser  <simon.fraser@apple.com>
+
+        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  <matteo_flores@apple.com>
 

trunk/LayoutTests/platform/win/TestExpectations

@@ -534 +534 @@
 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()

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2022-04-22  Simon Fraser  <simon.fraser@apple.com>
+
+        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  <zalan@apple.com>
 

trunk/Source/WebCore/dom/Element.cpp

@@ -1101 +1101 @@
             return;
     }
+    
+    if (auto* view = document().view())
+        view->cancelScheduledScrollToFocusedElement();
 
     document().updateLayoutIgnorePendingStylesheets();

trunk/Source/WebCore/page/DOMWindow.cpp

@@ -1739 +1739 @@
     }
 
+    view->cancelScheduledScrollToFocusedElement();
     document()->updateLayoutIgnorePendingStylesheets();
 

trunk/Source/WebCore/page/FrameView.cpp

@@ -2330 +2330 @@
     if (!m_maintainScrollPositionAnchor)
         return;
-    m_shouldScrollToFocusedElement = false;
-    m_delayedScrollToFocusedElementTimer.stop();
+
+    cancelScheduledScrollToFocusedElement();
 
     // We need to update the layout before scrolling, otherwise we could
@@ -2364 +2364 @@
 
     m_maintainScrollPositionAnchor = nullptr;
-    m_shouldScrollToFocusedElement = false;
-    m_delayedScrollToFocusedElementTimer.stop();
+    cancelScheduledScrollToFocusedElement();
+
     Page* page = frame().page();
     if (page && page->isMonitoringWheelEvents())
@@ -2411 +2411 @@
     m_shouldScrollToFocusedElement = true;
     m_delayedScrollToFocusedElementTimer.startOneShot(0_s);
+}
+
+void FrameView::cancelScheduledScrollToFocusedElement()
+{
+    m_shouldScrollToFocusedElement = false;
+    m_delayedScrollToFocusedElementTimer.stop();
 }
 
@@ -3289 +3295 @@
         return;
 
-    m_shouldScrollToFocusedElement = false;
-    m_delayedScrollToFocusedElementTimer.stop();
+    cancelScheduledScrollToFocusedElement();
 
     LayoutRect rect;
@@ -3314 +3319 @@
     LOG_WITH_STREAM(Scrolling, stream << " restoring anchor node to " << anchorNode.get());
     m_maintainScrollPositionAnchor = anchorNode;
-    m_shouldScrollToFocusedElement = false;
-    m_delayedScrollToFocusedElementTimer.stop();
+    cancelScheduledScrollToFocusedElement();
 }
 
@@ -4323 +4327 @@
     LOG(Scrolling, "FrameView::setWasScrolledByUser at %d", wasScrolledByUser);
 
-    m_shouldScrollToFocusedElement = false;
-    m_delayedScrollToFocusedElementTimer.stop();
+    cancelScheduledScrollToFocusedElement();
     if (currentScrollType() == ScrollType::Programmatic)
         return;

trunk/Source/WebCore/page/FrameView.h

@@ -267 +267 @@
     void restoreScrollbar();
     void scheduleScrollToFocusedElement(SelectionRevealMode);
+    void cancelScheduledScrollToFocusedElement();
     void scrollToFocusedElementImmediatelyIfNeeded();
     void updateLayerPositionsAfterScrolling() final;