REGRESSION: CSS scroll-behavior: smooth with overflow: hidden breaks …

…JS scrollTo/scrollLeft/scrollTop https://bugs.webkit.org/show_bug.cgi?id=238497 <rdar://90990040> Reviewed by Simon Fraser. Add scrollable area to frame view's list of scrollable areas if necessary in scrollToOffset. This is necessary as Document::runScrollSteps() looks through the list of scrollable areas to service animations on the scrollable area. It is necessary to add the scrollable area in scrollToOffset as for a scrollable area that is not user scrollable (such as a scrollable area with overflow: hidden) it would not be added, so the animation would not occur. * LayoutTests/fast/scrolling/smooth-scroll-with-overflow-hidden-expected.txt: Added. * LayoutTests/fast/scrolling/smooth-scroll-with-overflow-hidden.html: Added. * Source/WebCore/page/scrolling/ScrollingCoordinator.cpp: (WebCore::ScrollingCoordinator::absoluteEventTrackingRegionsForFrame const): * Source/WebCore/platform/ScrollableArea.h: (WebCore::ScrollableArea::isVisibleToHitTesting const): * Source/WebCore/rendering/RenderLayerScrollableArea.cpp: (WebCore::RenderLayerScrollableArea::clear): (WebCore::RenderLayerScrollableArea::scrollToOffset): (WebCore::RenderLayerScrollableArea::isVisibleToHitTesting const): (WebCore::RenderLayerScrollableArea::updateScrollableAreaSet): (WebCore::RenderLayerScrollableArea::registerScrollableArea): * Source/WebCore/rendering/RenderLayerScrollableArea.h: Canonical link: https://commits.webkit.org/251454@main git-svn-id: https://svn.webkit.org/repository/webkit/trunk@295448 268f45cc-cd09-0410-ab3c-d52691b4dbfc
WebKit · Jun 10, 2022 · ed7fed17c5ac886890859f1fc8682dba06424616 · ed7fed1
1 parent 66dcc4e
commit ed7fed17c5ac886890859f1fc8682dba06424616
Show file tree

Hide file tree

Showing 10 changed files with 100 additions and 6 deletions.
diff --git a/LayoutTests/fast/scrolling/mac/smooth-scroll-with-overflow-hidden-expected.txt b/LayoutTests/fast/scrolling/mac/smooth-scroll-with-overflow-hidden-expected.txt
@@ -0,0 +1,5 @@
+PASS scrollContainer.scrollLeft is 500
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/fast/scrolling/mac/smooth-scroll-with-overflow-hidden.html b/LayoutTests/fast/scrolling/mac/smooth-scroll-with-overflow-hidden.html
@@ -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;
+            overflow: hidden;
+        }
+
+        .contents {
+            height: 100%;
+            width: 400%;
+            background-image: repeating-linear-gradient(to right, white, silver 250px);
+        }
+    </style>
+    <script src="../../../resources/ui-helper.js"></script>
+    <script src="../../../resources/js-test-pre.js"></script>
+    <script>
+        jsTestIsAsync = true;
+
+        async function runTest()
+        {
+            scrollContainer = document.querySelector('.scroll-container');
+
+            eventSender.monitorWheelEvents();
+
+            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">
+        <div class="contents"></div>
+    </div>
+    <div id="console"></div>
+    <script src="../../../resources/js-test-post.js"></script>
+</body>
+</html>
diff --git a/Source/WebCore/page/FocusController.cpp b/Source/WebCore/page/FocusController.cpp
@@ -963,7 +963,9 @@ void FocusController::setIsVisibleAndActiveInternal(bool contentIsVisible)
             continue;
 
         for (auto& scrollableArea : *scrollableAreas) {
-            ASSERT(scrollableArea->scrollbarsCanBeActive() || m_page.shouldSuppressScrollbarAnimations());
+            if (!scrollableArea->scrollbarsCanBeActive())
+                continue;
+            ASSERT(m_page.shouldSuppressScrollbarAnimations());
 
             contentAreaDidShowOrHide(scrollableArea, contentIsVisible);
         }

diff --git a/Source/WebCore/page/FrameView.cpp b/Source/WebCore/page/FrameView.cpp
@@ -5842,6 +5842,15 @@ OverscrollBehavior FrameView::verticalOverscrollBehavior()  const
         return scrollingObject->style().overscrollBehaviorY();
     return OverscrollBehavior::Auto;
 }
+
+bool FrameView::isVisibleToHitTesting() const
+{
+    bool isVisibleToHitTest = true;
+    if (HTMLFrameOwnerElement* owner = frame().ownerElement())
+        isVisibleToHitTest = owner->renderer() && owner->renderer()->visibleToHitTesting();
+    return isVisibleToHitTest;
+}
+
 } // namespace WebCore
 
 #undef PAGE_ID

diff --git a/Source/WebCore/page/FrameView.h b/Source/WebCore/page/FrameView.h
@@ -117,6 +117,8 @@ class FrameView final : public ScrollView {
     WEBCORE_EXPORT void setCanHaveScrollbars(bool) final;
     WEBCORE_EXPORT void updateCanHaveScrollbars();
 
+    bool isVisibleToHitTesting() const final;
+
     Ref<Scrollbar> createScrollbar(ScrollbarOrientation) final;
 
     bool avoidScrollbarCreation() const final;

diff --git a/Source/WebCore/page/scrolling/ScrollingCoordinator.cpp b/Source/WebCore/page/scrolling/ScrollingCoordinator.cpp
@@ -37,6 +37,7 @@
 #include "PluginViewBase.h"
 #include "Region.h"
 #include "RenderLayerCompositor.h"
+#include "RenderLayerScrollableArea.h"
 #include "RenderView.h"
 #include "ScrollAnimator.h"
 #include "Settings.h"
@@ -125,7 +126,7 @@ EventTrackingRegions ScrollingCoordinator::absoluteEventTrackingRegionsForFrame(
     if (auto* scrollableAreas = frameView->scrollableAreas()) {
         for (auto& scrollableArea : *scrollableAreas) {
             // Composited scrollable areas can be scrolled off the main thread.
-            if (scrollableArea->usesAsyncScrolling())
+            if (!scrollableArea->isVisibleToHitTesting() || scrollableArea->usesAsyncScrolling())
                 continue;
 
             bool isInsideFixed;

diff --git a/Source/WebCore/platform/ScrollableArea.h b/Source/WebCore/platform/ScrollableArea.h
@@ -288,6 +288,8 @@ class ScrollableArea : public CanMakeWeakPtr<ScrollableArea> {
         LegacyIOSDocumentVisibleRect = ContentsVisibleRect
 #endif
     };
+
+    virtual bool isVisibleToHitTesting() const { return false; };
 
     WEBCORE_EXPORT IntRect visibleContentRect(VisibleContentRectBehavior = ContentsVisibleRect) const;
     WEBCORE_EXPORT IntRect visibleContentRectIncludingScrollbars(VisibleContentRectBehavior = ContentsVisibleRect) const;

diff --git a/Source/WebCore/rendering/RenderLayerScrollableArea.cpp b/Source/WebCore/rendering/RenderLayerScrollableArea.cpp
@@ -84,8 +84,6 @@ RenderLayerScrollableArea::~RenderLayerScrollableArea()
 void RenderLayerScrollableArea::clear()
 {
     auto& renderer = m_layer.renderer();
-    ASSERT(m_registeredScrollableArea == renderer.view().frameView().containsScrollableArea(this));
-
     if (m_registeredScrollableArea)
         renderer.view().frameView().removeScrollableArea(this);
 
@@ -281,6 +279,8 @@ ScrollOffset RenderLayerScrollableArea::scrollToOffset(const ScrollOffset& scrol
         scrollAnimator().cancelAnimations();
         stopAsyncAnimatedScroll();
     }
+
+    registerScrollableArea();
 
     ScrollOffset clampedScrollOffset = options.clamping == ScrollClamping::Clamped ? clampScrollOffset(scrollOffset) : scrollOffset;
     if (clampedScrollOffset == this->scrollOffset())
@@ -1624,6 +1624,12 @@ bool RenderLayerScrollableArea::scrollingMayRevealBackground() const
 {
     return scrollsOverflow() || usesCompositedScrolling();
 }
+bool RenderLayerScrollableArea::isVisibleToHitTesting() const
+{
+    auto& renderer = m_layer.renderer();
+    FrameView& frameView = renderer.view().frameView();
+    return renderer.visibleToHitTesting() && frameView.isVisibleToHitTesting();
+}
 
 void RenderLayerScrollableArea::updateScrollableAreaSet(bool hasOverflow)
 {
@@ -1637,8 +1643,6 @@ void RenderLayerScrollableArea::updateScrollableAreaSet(bool hasOverflow)
     bool isScrollable = hasOverflow && isVisibleToHitTest;
     bool addedOrRemoved = false;
 
-    ASSERT(m_registeredScrollableArea == frameView.containsScrollableArea(this));
-
     if (isScrollable) {
         if (!m_registeredScrollableArea) {
             addedOrRemoved = frameView.addScrollableArea(this);
@@ -1664,6 +1668,17 @@ void RenderLayerScrollableArea::updateScrollableAreaSet(bool hasOverflow)
 #endif
 }
 
+void RenderLayerScrollableArea::registerScrollableArea()
+{
+    auto& renderer = m_layer.renderer();
+    FrameView& frameView = renderer.view().frameView();
+
+    if (!m_registeredScrollableArea) {
+        frameView.addScrollableArea(this);
+        m_registeredScrollableArea = true;
+    }
+}
+
 void RenderLayerScrollableArea::updateScrollCornerStyle()
 {
     auto& renderer = m_layer.renderer();

diff --git a/Source/WebCore/rendering/RenderLayerScrollableArea.h b/Source/WebCore/rendering/RenderLayerScrollableArea.h
@@ -238,6 +238,7 @@ class RenderLayerScrollableArea final : public ScrollableArea {
     IntSize scrollbarOffset(const Scrollbar&) const;
 
     std::optional<LayoutRect> updateScrollPosition(const ScrollPositionChangeOptions&, const LayoutRect& revealRect, const LayoutRect& localExposeRect);
+    bool isVisibleToHitTesting() const final;
 
 private:
     bool hasHorizontalOverflow() const;
@@ -264,6 +265,7 @@ class RenderLayerScrollableArea final : public ScrollableArea {
     void clearResizer();
 
     void updateScrollbarPresenceAndState(std::optional<bool> hasHorizontalOverflow = std::nullopt, std::optional<bool> hasVerticalOverflow = std::nullopt);
+    void registerScrollableArea();
 
 private:
     bool m_scrollDimensionsDirty { true };

diff --git a/Source/WebCore/rendering/RenderListBox.h b/Source/WebCore/rendering/RenderListBox.h
@@ -65,6 +65,8 @@ class RenderListBox final : public RenderBlockFlow, public ScrollableArea {
     bool scrolledToBottom() const final;
     bool scrolledToLeft() const final;
     bool scrolledToRight() const final;
+
+    bool isVisibleToHitTesting() const final { return visibleToHitTesting(); };
 
 private:
     void willBeDestroyed() override;