Title: [261368] trunk
Revision
261368
Author
simon.fra...@apple.com
Date
2020-05-07 22:48:59 -0700 (Thu, 07 May 2020)

Log Message

MayBegin wheel event in a <select> doesn't flash the scrollers
https://bugs.webkit.org/show_bug.cgi?id=211605

Reviewed by Antti Koivisto.

Source/WebCore:

We need to special-case scrollable <select> elements, because they are ScrollableAreas
which are never asynchronously scrolled, so the ScrollingTree never dispatches the handleWheelEventPhase()
which is necessary to flash overlay scrollbars. Scrollable <select> elements are always in the
non-fast scrollable region.

Fix findEnclosingScrollableContainer() to return a ScrollableArea for the "maybegin" and "canceled" events
that have no delta.

Remove some "inline" and make some things references.

Tests: fast/scrolling/mac/scrollbars/select-overlay-scrollbar-hovered.html
       fast/scrolling/mac/scrollbars/select-overlay-scrollbar-reveal.html

* page/EventHandler.cpp:
(WebCore::handleWheelEventPhaseInScrollableArea):
(WebCore::didScrollInScrollableArea):
(WebCore::handleWheelEventInAppropriateEnclosingBox):
(WebCore::shouldGesturesTriggerActive):
(WebCore::EventHandler::eventLoopHandleMouseUp):
(WebCore::EventHandler::eventLoopHandleMouseDragged):
* page/mac/EventHandlerMac.mm:
(WebCore::findEnclosingScrollableContainer):
(WebCore::EventHandler::determineWheelEventTarget):
* testing/Internals.cpp:
(WebCore::Internals::scrollableAreaForNode): Fix to find the ScrollableArea for RenderListBoxes.

LayoutTests:

* fast/scrolling/mac/scrollbars/select-overlay-scrollbar-hovered-expected.txt: Added.
* fast/scrolling/mac/scrollbars/select-overlay-scrollbar-hovered.html: Added.
* fast/scrolling/mac/scrollbars/select-overlay-scrollbar-reveal-expected.txt: Added.
* fast/scrolling/mac/scrollbars/select-overlay-scrollbar-reveal.html: Added.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (261367 => 261368)


--- trunk/LayoutTests/ChangeLog	2020-05-08 04:23:01 UTC (rev 261367)
+++ trunk/LayoutTests/ChangeLog	2020-05-08 05:48:59 UTC (rev 261368)
@@ -1,3 +1,15 @@
+2020-05-07  Simon Fraser  <simon.fra...@apple.com>
+
+        MayBegin wheel event in a <select> doesn't flash the scrollers
+        https://bugs.webkit.org/show_bug.cgi?id=211605
+
+        Reviewed by Antti Koivisto.
+
+        * fast/scrolling/mac/scrollbars/select-overlay-scrollbar-hovered-expected.txt: Added.
+        * fast/scrolling/mac/scrollbars/select-overlay-scrollbar-hovered.html: Added.
+        * fast/scrolling/mac/scrollbars/select-overlay-scrollbar-reveal-expected.txt: Added.
+        * fast/scrolling/mac/scrollbars/select-overlay-scrollbar-reveal.html: Added.
+
 2020-05-07  Lauro Moura  <lmo...@igalia.com>
 
         [GTK] Gardening two poster tests and a webgl failure

Added: trunk/LayoutTests/fast/scrolling/mac/scrollbars/select-overlay-scrollbar-hovered-expected.txt (0 => 261368)


--- trunk/LayoutTests/fast/scrolling/mac/scrollbars/select-overlay-scrollbar-hovered-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/fast/scrolling/mac/scrollbars/select-overlay-scrollbar-hovered-expected.txt	2020-05-08 05:48:59 UTC (rev 261368)
@@ -0,0 +1,16 @@
+
+Test hover over the overlay scrollbar of a scrolling list
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+Initial state
+enabled
+Hovering vertical scrollbar should show expanded scrollbar
+PASS Scrollbar state: enabled,expanded,visible_track,visible_thumb
+Unhovering vertical scrollbar should hide it
+PASS Thumb and track hidden
+PASS successfullyParsed is true
+
+TEST COMPLETE
+

Added: trunk/LayoutTests/fast/scrolling/mac/scrollbars/select-overlay-scrollbar-hovered.html (0 => 261368)


--- trunk/LayoutTests/fast/scrolling/mac/scrollbars/select-overlay-scrollbar-hovered.html	                        (rev 0)
+++ trunk/LayoutTests/fast/scrolling/mac/scrollbars/select-overlay-scrollbar-hovered.html	2020-05-08 05:48:59 UTC (rev 261368)
@@ -0,0 +1,88 @@
+<!DOCTYPE html> <!-- webkit-test-runner [ internal:useMockScrollbars=false internal:AsyncOverflowScrollingEnabled=true ] -->
+<html>
+<head>
+    <style>
+        body {
+            height: 1000px;
+        }
+        select {
+            display: block;
+            font-size: 12pt;
+            margin: 10px;
+            border: 1px solid black;
+        }
+    </style>
+    <script src=""
+    <script src=""
+    
+    <script>
+        jsTestIsAsync = true;
+
+        if (window.internals)
+            internals.setUsesOverlayScrollbars(true);
+
+        async function doTest()
+        {
+            description('Test hover over the overlay scrollbar of a scrolling list');
+            if (!window.internals) {
+                finishJSTest();                
+                return;
+            }
+            
+            const select = document.getElementsByTagName('select')[0];
+            const selectBounds = select.getBoundingClientRect();
+            const x = selectBounds.right - 8;
+            const y = selectBounds.top + 10;
+
+            debug('Initial state');
+            debug(internals.verticalScrollbarState(select));
+
+            debug('Hovering vertical scrollbar should show expanded scrollbar');
+            await UIHelper.mouseWheelScrollAt(x, y);
+            await UIHelper.waitForCondition(() => {
+                let state = internals.verticalScrollbarState(select);
+                let expanded = state.indexOf('expanded') != -1;
+                if (expanded)
+                    testPassed('Scrollbar state: ' + state);
+                return expanded;
+            });
+
+            debug('Unhovering vertical scrollbar should hide it');
+            await UIHelper.moveMouseAndWaitForFrame(x - 10, y);
+            await UIHelper.moveMouseAndWaitForFrame(x - 20, y);
+            await UIHelper.waitForCondition(() => {
+                let state = internals.verticalScrollbarState(select);
+                let thumbHidden = state.indexOf('visible_thumb') == -1;
+                let trackHidden = state.indexOf('visible_track') == -1;
+                if (thumbHidden && trackHidden)
+                    testPassed('Thumb and track hidden');
+                return thumbHidden && trackHidden;
+            });
+
+            finishJSTest();
+        }
+
+        window.addEventListener('load', () => {
+            doTest();
+        }, false);
+    </script>
+</head>
+<body>
+    <select size="5">
+        <option>January</option>
+        <option>February</option>
+        <option>March</option>
+        <option>April</option>
+        <option>May</option>
+        <option>June</option>
+        <option>July</option>
+        <option>August</option>
+        <option>September</option>
+        <option>October</option>
+        <option>November</option>
+        <option>December</option>
+    </select>
+    <div id="console"></div>
+    <script src=""
+</body>
+</html>

Added: trunk/LayoutTests/fast/scrolling/mac/scrollbars/select-overlay-scrollbar-reveal-expected.txt (0 => 261368)


--- trunk/LayoutTests/fast/scrolling/mac/scrollbars/select-overlay-scrollbar-reveal-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/fast/scrolling/mac/scrollbars/select-overlay-scrollbar-reveal-expected.txt	2020-05-08 05:48:59 UTC (rev 261368)
@@ -0,0 +1,16 @@
+
+Test maybegin and cancelled on a scrolling list
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+Initial state
+enabled
+MayBegin should show the scrollbar
+PASS Scrollbar state: enabled,visible_thumb
+Cancelled should hide the scrollbar
+PASS Scrollbar state: enabled
+PASS successfullyParsed is true
+
+TEST COMPLETE
+

Added: trunk/LayoutTests/fast/scrolling/mac/scrollbars/select-overlay-scrollbar-reveal.html (0 => 261368)


--- trunk/LayoutTests/fast/scrolling/mac/scrollbars/select-overlay-scrollbar-reveal.html	                        (rev 0)
+++ trunk/LayoutTests/fast/scrolling/mac/scrollbars/select-overlay-scrollbar-reveal.html	2020-05-08 05:48:59 UTC (rev 261368)
@@ -0,0 +1,87 @@
+<!DOCTYPE html> <!-- webkit-test-runner [ internal:useMockScrollbars=false internal:AsyncOverflowScrollingEnabled=true ] -->
+<html>
+<head>
+    <style>
+        body {
+            height: 1000px;
+        }
+        select {
+            display: block;
+            font-size: 12pt;
+            margin: 10px;
+            border: 1px solid black;
+        }
+    </style>
+    <script src=""
+    <script src=""
+    
+    <script>
+        jsTestIsAsync = true;
+        
+        if (window.internals)
+            internals.setUsesOverlayScrollbars(true);
+
+        async function doTest()
+        {
+            description('Test maybegin and cancelled on a scrolling list');
+            if (!window.internals) {
+                finishJSTest();
+                return;
+            }
+            
+            const select = document.getElementsByTagName('select')[0];
+            const selectBounds = select.getBoundingClientRect();
+            const x = selectBounds.left + 10;
+            const y = selectBounds.top + 10;
+
+            debug('Initial state');
+            debug(internals.verticalScrollbarState(select));
+
+            debug('MayBegin should show the scrollbar');
+            await UIHelper.mouseWheelMayBeginAt(x, y);
+            await UIHelper.waitForCondition(() => {
+                let state = internals.verticalScrollbarState(select);
+                let visible = state.indexOf('visible_thumb') != -1;
+                if (visible)
+                    testPassed('Scrollbar state: ' + state);
+                return visible;
+            });
+
+            debug('Cancelled should hide the scrollbar');
+            await UIHelper.mouseWheelCancelAt(x, y);
+
+            await UIHelper.waitForCondition(() => {
+                let state = internals.verticalScrollbarState(select);
+                let hidden = state.indexOf('visible_thumb') == -1;
+                if (hidden)
+                    testPassed('Scrollbar state: ' + state);
+                return hidden;
+            });
+
+            finishJSTest();
+        }
+
+        window.addEventListener('load', () => {
+            doTest();
+        }, false);
+    </script>
+</head>
+<body>
+    <select size="5">
+        <option>January</option>
+        <option>February</option>
+        <option>March</option>
+        <option>April</option>
+        <option>May</option>
+        <option>June</option>
+        <option>July</option>
+        <option>August</option>
+        <option>September</option>
+        <option>October</option>
+        <option>November</option>
+        <option>December</option>
+    </select>
+    <div id="console"></div>
+    <script src=""
+</body>
+</html>

Modified: trunk/Source/WebCore/ChangeLog (261367 => 261368)


--- trunk/Source/WebCore/ChangeLog	2020-05-08 04:23:01 UTC (rev 261367)
+++ trunk/Source/WebCore/ChangeLog	2020-05-08 05:48:59 UTC (rev 261368)
@@ -1,3 +1,36 @@
+2020-05-07  Simon Fraser  <simon.fra...@apple.com>
+
+        MayBegin wheel event in a <select> doesn't flash the scrollers
+        https://bugs.webkit.org/show_bug.cgi?id=211605
+
+        Reviewed by Antti Koivisto.
+
+        We need to special-case scrollable <select> elements, because they are ScrollableAreas
+        which are never asynchronously scrolled, so the ScrollingTree never dispatches the handleWheelEventPhase()
+        which is necessary to flash overlay scrollbars. Scrollable <select> elements are always in the
+        non-fast scrollable region.
+        
+        Fix findEnclosingScrollableContainer() to return a ScrollableArea for the "maybegin" and "canceled" events
+        that have no delta.
+        
+        Remove some "inline" and make some things references.
+
+        Tests: fast/scrolling/mac/scrollbars/select-overlay-scrollbar-hovered.html
+               fast/scrolling/mac/scrollbars/select-overlay-scrollbar-reveal.html
+
+        * page/EventHandler.cpp:
+        (WebCore::handleWheelEventPhaseInScrollableArea):
+        (WebCore::didScrollInScrollableArea):
+        (WebCore::handleWheelEventInAppropriateEnclosingBox):
+        (WebCore::shouldGesturesTriggerActive):
+        (WebCore::EventHandler::eventLoopHandleMouseUp):
+        (WebCore::EventHandler::eventLoopHandleMouseDragged):
+        * page/mac/EventHandlerMac.mm:
+        (WebCore::findEnclosingScrollableContainer):
+        (WebCore::EventHandler::determineWheelEventTarget):
+        * testing/Internals.cpp:
+        (WebCore::Internals::scrollableAreaForNode): Fix to find the ScrollableArea for RenderListBoxes.
+
 2020-05-07  Chris Dumez  <cdu...@apple.com>
 
         Unreviewed, reverting r261252.

Modified: trunk/Source/WebCore/page/EventHandler.cpp (261367 => 261368)


--- trunk/Source/WebCore/page/EventHandler.cpp	2020-05-08 04:23:01 UTC (rev 261367)
+++ trunk/Source/WebCore/page/EventHandler.cpp	2020-05-08 05:48:59 UTC (rev 261368)
@@ -88,6 +88,7 @@
 #include "RuntimeApplicationChecks.h"
 #include "SVGDocument.h"
 #include "SVGNames.h"
+#include "ScrollAnimator.h"
 #include "ScrollLatchingState.h"
 #include "Scrollbar.h"
 #include "Settings.h"
@@ -290,20 +291,31 @@
     }
 }
 
-static inline bool didScrollInScrollableArea(ScrollableArea* scrollableArea, WheelEvent& wheelEvent)
+static void handleWheelEventPhaseInScrollableArea(ScrollableArea& scrollableArea, const WheelEvent& wheelEvent)
 {
+#if PLATFORM(MAC)
+    if (wheelEvent.phase() == PlatformWheelEventPhaseMayBegin || wheelEvent.phase() == PlatformWheelEventPhaseCancelled)
+        scrollableArea.scrollAnimator().handleWheelEventPhase(wheelEvent.phase());
+#else
+    UNUSED_PARAM(scrollableArea);
+    UNUSED_PARAM(wheelEvent);
+#endif
+}
+
+static bool didScrollInScrollableArea(ScrollableArea& scrollableArea, const WheelEvent& wheelEvent)
+{
     ScrollGranularity scrollGranularity = wheelGranularityToScrollGranularity(wheelEvent.deltaMode());
     bool didHandleWheelEvent = false;
     if (float absoluteDelta = std::abs(wheelEvent.deltaX()))
-        didHandleWheelEvent |= scrollableArea->scroll(wheelEvent.deltaX() > 0 ? ScrollRight : ScrollLeft, scrollGranularity, absoluteDelta);
+        didHandleWheelEvent |= scrollableArea.scroll(wheelEvent.deltaX() > 0 ? ScrollRight : ScrollLeft, scrollGranularity, absoluteDelta);
     
     if (float absoluteDelta = std::abs(wheelEvent.deltaY()))
-        didHandleWheelEvent |= scrollableArea->scroll(wheelEvent.deltaY() > 0 ? ScrollDown : ScrollUp, scrollGranularity, absoluteDelta);
+        didHandleWheelEvent |= scrollableArea.scroll(wheelEvent.deltaY() > 0 ? ScrollDown : ScrollUp, scrollGranularity, absoluteDelta);
     
     return didHandleWheelEvent;
 }
 
-static inline bool handleWheelEventInAppropriateEnclosingBox(Node* startNode, WheelEvent& wheelEvent, RefPtr<Element>& stopElement, const FloatSize& filteredPlatformDelta, const FloatSize& filteredVelocity)
+static bool handleWheelEventInAppropriateEnclosingBox(Node* startNode, const WheelEvent& wheelEvent, RefPtr<Element>& stopElement, const FloatSize& filteredPlatformDelta, const FloatSize& filteredVelocity)
 {
     bool shouldHandleEvent = wheelEvent.deltaX() || wheelEvent.deltaY();
 #if ENABLE(WHEEL_EVENT_LATCHING)
@@ -312,13 +324,21 @@
     shouldHandleEvent |= wheelEvent.momentumPhase() == PlatformWheelEventPhaseEnded;
 #endif
 #endif
-    if (!startNode->renderer() || !shouldHandleEvent)
+    if (!startNode->renderer())
         return false;
 
     RenderBox& initialEnclosingBox = startNode->renderer()->enclosingBox();
-    if (initialEnclosingBox.isListBox())
-        return didScrollInScrollableArea(static_cast<RenderListBox*>(&initialEnclosingBox), wheelEvent);
 
+    // RenderListBox is special because it's a ScrollableArea that the scrolling tree doesn't know about.
+    if (is<RenderListBox>(initialEnclosingBox))
+        handleWheelEventPhaseInScrollableArea(downcast<RenderListBox>(initialEnclosingBox), wheelEvent);
+
+    if (!shouldHandleEvent)
+        return false;
+
+    if (is<RenderListBox>(initialEnclosingBox))
+        return didScrollInScrollableArea(downcast<RenderListBox>(initialEnclosingBox), wheelEvent);
+
     RenderBox* currentEnclosingBox = &initialEnclosingBox;
     while (currentEnclosingBox) {
         if (RenderLayer* boxLayer = currentEnclosingBox->layer()) {
@@ -328,7 +348,7 @@
                 auto copiedEvent = platformEvent->copyWithDeltasAndVelocity(filteredPlatformDelta.width(), filteredPlatformDelta.height(), filteredVelocity);
                 scrollingWasHandled = boxLayer->handleWheelEvent(copiedEvent);
             } else
-                scrollingWasHandled = didScrollInScrollableArea(boxLayer, wheelEvent);
+                scrollingWasHandled = didScrollInScrollableArea(*boxLayer, wheelEvent);
 
             if (scrollingWasHandled) {
                 stopElement = currentEnclosingBox->element();
@@ -347,7 +367,7 @@
 }
 
 #if (ENABLE(TOUCH_EVENTS) && !PLATFORM(IOS_FAMILY))
-static inline bool shouldGesturesTriggerActive()
+static bool shouldGesturesTriggerActive()
 {
     // If the platform we're on supports GestureTapDown and GestureTapCancel then we'll
     // rely on them to set the active state. Unfortunately there's no generic way to
@@ -358,13 +378,13 @@
 
 #if !PLATFORM(COCOA)
 
-inline bool EventHandler::eventLoopHandleMouseUp(const MouseEventWithHitTestResults&)
+bool EventHandler::eventLoopHandleMouseUp(const MouseEventWithHitTestResults&)
 {
     return false;
 }
 
 #if ENABLE(DRAG_SUPPORT)
-inline bool EventHandler::eventLoopHandleMouseDragged(const MouseEventWithHitTestResults&)
+bool EventHandler::eventLoopHandleMouseDragged(const MouseEventWithHitTestResults&)
 {
     return false;
 }

Modified: trunk/Source/WebCore/page/mac/EventHandlerMac.mm (261367 => 261368)


--- trunk/Source/WebCore/page/mac/EventHandlerMac.mm	2020-05-08 04:23:01 UTC (rev 261367)
+++ trunk/Source/WebCore/page/mac/EventHandlerMac.mm	2020-05-08 05:48:59 UTC (rev 261368)
@@ -785,7 +785,7 @@
     return box.layer();
 }
     
-static ContainerNode* findEnclosingScrollableContainer(ContainerNode* node, float deltaX, float deltaY)
+static ContainerNode* findEnclosingScrollableContainer(ContainerNode* node, const PlatformWheelEvent& wheelEvent)
 {
     // Find the first node with a valid scrollable area starting with the current
     // node and traversing its parents (or shadow hosts).
@@ -797,14 +797,21 @@
             return nullptr;
 
         RenderBox* box = candidate->renderBox();
-        if (box && box->canBeScrolledAndHasScrollableArea()) {
-            if (ScrollableArea* scrollableArea = scrollableAreaForBox(*box)) {
-                if (((deltaY > 0) && !scrollableArea->scrolledToTop()) || ((deltaY < 0) && !scrollableArea->scrolledToBottom())
-                    || ((deltaX > 0) && !scrollableArea->scrolledToLeft()) || ((deltaX < 0) && !scrollableArea->scrolledToRight())) {
-                    return candidate;
-                }
-            }
-        }
+        if (!box || !box->canBeScrolledAndHasScrollableArea())
+            continue;
+        
+        auto* scrollableArea = scrollableAreaForBox(*box);
+        if (!scrollableArea)
+            continue;
+
+        if (wheelEvent.phase() == PlatformWheelEventPhaseMayBegin || wheelEvent.phase() == PlatformWheelEventPhaseCancelled)
+            return candidate;
+
+        auto deltaX = wheelEvent.deltaX();
+        auto deltaY = wheelEvent.deltaY();
+        if ((deltaY > 0 && !scrollableArea->scrolledToTop()) || (deltaY < 0 && !scrollableArea->scrolledToBottom())
+            || (deltaX > 0 && !scrollableArea->scrolledToLeft()) || (deltaX < 0 && !scrollableArea->scrolledToRight()))
+            return candidate;
     }
     
     return nullptr;
@@ -963,7 +970,7 @@
             scrollableContainer = wheelEventTarget;
             scrollableArea = scrollableAreaForEventTarget(wheelEventTarget.get());
         } else {
-            scrollableContainer = findEnclosingScrollableContainer(wheelEventTarget.get(), wheelEvent.deltaX(), wheelEvent.deltaY());
+            scrollableContainer = findEnclosingScrollableContainer(wheelEventTarget.get(), wheelEvent);
             if (scrollableContainer && !is<HTMLIFrameElement>(wheelEventTarget))
                 scrollableArea = scrollableAreaForContainerNode(*scrollableContainer);
             else {

Modified: trunk/Source/WebCore/testing/Internals.cpp (261367 => 261368)


--- trunk/Source/WebCore/testing/Internals.cpp	2020-05-08 04:23:01 UTC (rev 261367)
+++ trunk/Source/WebCore/testing/Internals.cpp	2020-05-08 05:48:59 UTC (rev 261368)
@@ -154,6 +154,7 @@
 #include "RenderEmbeddedObject.h"
 #include "RenderLayerBacking.h"
 #include "RenderLayerCompositor.h"
+#include "RenderListBox.h"
 #include "RenderMenuList.h"
 #include "RenderTheme.h"
 #include "RenderTreeAsText.h"
@@ -2775,7 +2776,11 @@
         if (!element.renderBox())
             return Exception { InvalidAccessError };
 
-        scrollableArea = element.renderBox()->layer();
+        auto& renderBox = *element.renderBox();
+        if (is<RenderListBox>(renderBox))
+            scrollableArea = &downcast<RenderListBox>(renderBox);
+        else
+            scrollableArea = renderBox.layer();
     } else
         return Exception { InvalidNodeTypeError };
 
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to