[webkit-changes] [198805] trunk

Tue, 29 Mar 2016 15:17:17 -0700

Title: [198805] trunk
Revision
198805
Author
[email protected]
Date
2016-03-29 15:17:04 -0700 (Tue, 29 Mar 2016)

Log Message

Wheel events' latching state is not reset when appropriate
https://bugs.webkit.org/show_bug.cgi?id=155746

Reviewed by Simon Fraser.

Source/WebCore:

When one performs a two fingers scroll (with trackpad),
it might either trigger a kinetic scroll movement (by
flickering with both fingers in a given direction) or perform
a static scroll movement (without the flicker animation).

In case of the first scenario (kinetic), the container being
scrolled is "latched" during the scroll animation and properly unlatched
once the scroll ends. See the call to 'shouldResetLatching' in EventHandlerMac.mm.
When transitioning from non-momentum to momentum scroll, the following events are seen:
- 'phase PlatformWheelEventPhaseEnded' 'momentumPhase PlatformWheelEventPhaseNone' -> end of static scroll
- 'phase PlatformWheelEventPhaseNone' 'momentumPhase PlatformWheelEventPhaseBegan' -> start of momentum scroll

On the second scenario (static scroll), when the scroll movement ends,
the latched state is not reset. This might actually cause scrolling elsewhere on the page to scroll
the previously latched container.
Note that, only specific wheel event below is fired when static scroll ends:
- 'phase PlatformWheelEventPhaseEnded' 'momentumPhase PlatformWheelEventPhaseNone'

In order to fix this, patch installs a timer as soon as a non-momentum
wheel scroll event end fires. It works as follows:

- If the timer fires, the latched state is reset, preventing scrolling getting stuck to
a previously latched scrollable.
- If platform code starts sending momentum wheel scroll events before it times out, the timer is
stopped and the latched state remains untouched (this is a flick scroll case).
- If a new wheel scroll gesture starts, the latched state is manually reset
and the timer stopped.

Note that given that latched state logic applies primarily to main frames,
the timer only gets manipulated for main frame objects.

Test: tiled-drawing/scrolling/scroll-iframe-latched-selects.html

* page/EventHandler.cpp:
(WebCore::EventHandler::doOrScheduleClearLatchedStateIfNeeded):
* page/EventHandler.h:
* page/MainFrame.cpp:
(WebCore::MainFrame::MainFrame):
(WebCore::MainFrame::~MainFrame):
* page/MainFrame.h:
* page/mac/EventHandlerMac.mm:
(WebCore::scrollableAreaForContainerNode):
(WebCore::latchedToFrameOrBody):
(WebCore::areWheelEventsTransitioningToMomentumScrolling):
(WebCore::areWheelEventsEndingNonMomentumScrolling):
(WebCore::EventHandler::doOrScheduleClearLatchedStateIfNeeded):

LayoutTests:

* tiled-drawing/scrolling/resources/selects-iframe.html: Added.
* tiled-drawing/scrolling/scroll-iframe-latched-selects.html: Added.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (198804 => 198805)


--- trunk/LayoutTests/ChangeLog	2016-03-29 22:15:27 UTC (rev 198804)
+++ trunk/LayoutTests/ChangeLog	2016-03-29 22:17:04 UTC (rev 198805)
@@ -1,3 +1,13 @@
+2016-03-29  Antonio Gomes  <[email protected]>
+
+        Wheel events' latching state is not reset when appropriate
+        https://bugs.webkit.org/show_bug.cgi?id=155746
+
+        Reviewed by Simon Fraser.
+
+        * tiled-drawing/scrolling/resources/selects-iframe.html: Added.
+        * tiled-drawing/scrolling/scroll-iframe-latched-selects.html: Added.
+
 2016-03-29  Saam barati  <[email protected]>
 
         "Can not" => "cannot" in String.prototype error messages

Added: trunk/LayoutTests/tiled-drawing/scrolling/resources/selects-iframe.html (0 => 198805)


--- trunk/LayoutTests/tiled-drawing/scrolling/resources/selects-iframe.html	                        (rev 0)
+++ trunk/LayoutTests/tiled-drawing/scrolling/resources/selects-iframe.html	2016-03-29 22:17:04 UTC (rev 198805)
@@ -0,0 +1,23 @@
+<!DOCTYPE html>
+<html>
+  <body>
+    <h1>This is a Heading</h1>
+    <p>This is a paragraph.</p>
+    <select id="selectLeft" size="5" multiple="multiple">
+      <option value="" >(No Driver Filter)</option>
+      <option value="drivername=''"></option>
+      <option value="drivername='alex'">alex</option>
+      <option value="drivername='marc'">marc</option>
+      <option value="drivername='frank'">frank</option>
+      <option value="drivername='james'">james</option>
+    </select>
+    <select id="selectRight" size="5" multiple="multiple">
+      <option value="" >(No Driver Filter)</option>
+      <option value="drivername=''"></option>
+      <option value="drivername='alex'">alex</option>
+      <option value="drivername='marc'">marc</option>
+      <option value="drivername='frank'">frank</option>
+      <option value="drivername='james'">james</option>
+    </select>
+</body>
+</html>

Added: trunk/LayoutTests/tiled-drawing/scrolling/scroll-iframe-latched-selects-expected.txt (0 => 198805)


--- trunk/LayoutTests/tiled-drawing/scrolling/scroll-iframe-latched-selects-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/tiled-drawing/scrolling/scroll-iframe-latched-selects-expected.txt	2016-03-29 22:17:04 UTC (rev 198805)
@@ -0,0 +1,15 @@
+
+Tests that latched state logic does not get stuck scrolling a specific <select>. To manually run this test, place the mouse pointer
+within one left <select> boundary, and then use the mouse wheel or a two-finger to scroll down the list, without momentum scrolling.
+Do the same for the right <select>. The left <select> should not scroll when trying to scroll the right <select>.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS Leftmost <select> was scrolled
+PASS Leftmost <select> was not scrolled by the latch state logic
+PASS Rightmost <select> was properly scrolled.
+PASS successfullyParsed is true
+
+TEST COMPLETE
+

Added: trunk/LayoutTests/tiled-drawing/scrolling/scroll-iframe-latched-selects.html (0 => 198805)


--- trunk/LayoutTests/tiled-drawing/scrolling/scroll-iframe-latched-selects.html	                        (rev 0)
+++ trunk/LayoutTests/tiled-drawing/scrolling/scroll-iframe-latched-selects.html	2016-03-29 22:17:04 UTC (rev 198805)
@@ -0,0 +1,80 @@
+<!DOCTYPE html>
+<html>
+<head>
+<link rel="help" href=""
+<script src=""
+</head>
+<body _onload_="setupTopLevel();">
+<script>
+window.jsTestIsAsync = true;
+
+var iframeTarget;
+var selectLeft;
+var selectRight;
+var previousSelectLeftScrollPosition;
+
+function checkForScroll()
+{
+    if (selectLeft.scrollTop == previousSelectLeftScrollPosition)
+        testPassed("Leftmost <select> was not scrolled by the latch state logic");
+    else
+        testFailed("Leftmost <select> was incorrectly scrolled by wrong latch logic." + " " + selectLeft.scrollTop + " " + previousSelectLeftScrollPosition);
+
+    if (selectRight.scrollTop > 0)
+        testPassed("Rightmost <select> was properly scrolled.");
+    else
+        testFailed("Rightmost <select> was not scrolled though it should.");
+
+    finishJSTest();
+}
+function scrollRightMostSelect()
+{
+    if (selectLeft.scrollTop > 0)
+        testPassed("Leftmost <select> was scrolled");
+    else
+        testFailed("Leftmost <select> was not scrolled");
+    previousSelectLeftScrollPosition = selectLeft.scrollTop;
+
+    var startPosX = Math.round(selectRight.offsetLeft) + 10;
+    var startPosY = Math.round(selectRight.offsetTop) + 10;
+    eventSender.mouseMoveTo(startPosX, startPosY);
+    eventSender.mouseScrollByWithWheelAndMomentumPhases(0, -1, "began", "none");
+    eventSender.mouseScrollByWithWheelAndMomentumPhases(0, -1, "changed", "none");
+    eventSender.mouseScrollByWithWheelAndMomentumPhases(0, 0, "ended", "none");
+    eventSender.callAfterScrollingCompletes(checkForScroll);
+}
+
+function scrollLeftMostSelect()
+{
+    var startPosX = Math.round(selectLeft.offsetLeft) + 10;
+    var startPosY = Math.round(selectLeft.offsetTop) + 10;
+    eventSender.mouseMoveTo(startPosX, startPosY);
+    eventSender.mouseScrollByWithWheelAndMomentumPhases(0, -1, "began", "none");
+    eventSender.mouseScrollByWithWheelAndMomentumPhases(0, -1, "changed", "none");
+    eventSender.mouseScrollByWithWheelAndMomentumPhases(0, 0, "ended", "none");
+    eventSender.callAfterScrollingCompletes(scrollRightMostSelect);
+}
+
+function setupTopLevel()
+{
+    if (window.eventSender) {
+        eventSender.monitorWheelEvents();
+
+        iframeTarget = document.getElementById("target");
+        selectLeft = window.frames["target"].document.getElementById("selectLeft");
+        selectRight = window.frames["target"].document.getElementById("selectRight");
+
+        setTimeout(scrollLeftMostSelect, 0);
+    }
+}
+</script>
+<iframe id="target" name="target" height="400" width="400" src=""
+<div id="console"></div>
+<script>
+description("Tests that latched state logic does not get stuck scrolling a specific &lt;select&gt;. To manually run this test, place the mouse pointer<br>" +
+            "within one left &lt;select&gt; boundary, and then use the mouse wheel or a two-finger to scroll down the list, without momentum scrolling.<br>" +
+            "Do the same for the right &lt;select&gt;. The left &lt;select&gt; should not scroll when trying to scroll the right &lt;select&gt;.");
+</script>
+<script src=""
+</body>
+</html>

Modified: trunk/Source/WebCore/ChangeLog (198804 => 198805)


--- trunk/Source/WebCore/ChangeLog	2016-03-29 22:15:27 UTC (rev 198804)
+++ trunk/Source/WebCore/ChangeLog	2016-03-29 22:17:04 UTC (rev 198805)
@@ -1,3 +1,57 @@
+2016-03-29  Antonio Gomes  <[email protected]>
+
+        Wheel events' latching state is not reset when appropriate
+        https://bugs.webkit.org/show_bug.cgi?id=155746
+
+        Reviewed by Simon Fraser.
+
+        When one performs a two fingers scroll (with trackpad),
+        it might either trigger a kinetic scroll movement (by
+        flickering with both fingers in a given direction) or perform
+        a static scroll movement (without the flicker animation).
+
+        In case of the first scenario (kinetic), the container being
+        scrolled is "latched" during the scroll animation and properly unlatched
+        once the scroll ends. See the call to 'shouldResetLatching' in EventHandlerMac.mm.
+        When transitioning from non-momentum to momentum scroll, the following events are seen:
+        - 'phase PlatformWheelEventPhaseEnded' 'momentumPhase PlatformWheelEventPhaseNone' -> end of static scroll
+        - 'phase PlatformWheelEventPhaseNone' 'momentumPhase PlatformWheelEventPhaseBegan' -> start of momentum scroll
+
+        On the second scenario (static scroll), when the scroll movement ends,
+        the latched state is not reset. This might actually cause scrolling elsewhere on the page to scroll
+        the previously latched container.
+        Note that, only specific wheel event below is fired when static scroll ends:
+        - 'phase PlatformWheelEventPhaseEnded' 'momentumPhase PlatformWheelEventPhaseNone'
+
+        In order to fix this, patch installs a timer as soon as a non-momentum
+        wheel scroll event end fires. It works as follows:
+
+        - If the timer fires, the latched state is reset, preventing scrolling getting stuck to
+        a previously latched scrollable.
+        - If platform code starts sending momentum wheel scroll events before it times out, the timer is
+        stopped and the latched state remains untouched (this is a flick scroll case).
+        - If a new wheel scroll gesture starts, the latched state is manually reset
+        and the timer stopped.
+
+        Note that given that latched state logic applies primarily to main frames,
+        the timer only gets manipulated for main frame objects.
+
+        Test: tiled-drawing/scrolling/scroll-iframe-latched-selects.html
+
+        * page/EventHandler.cpp:
+        (WebCore::EventHandler::doOrScheduleClearLatchedStateIfNeeded):
+        * page/EventHandler.h:
+        * page/MainFrame.cpp:
+        (WebCore::MainFrame::MainFrame):
+        (WebCore::MainFrame::~MainFrame):
+        * page/MainFrame.h:
+        * page/mac/EventHandlerMac.mm:
+        (WebCore::scrollableAreaForContainerNode):
+        (WebCore::latchedToFrameOrBody):
+        (WebCore::areWheelEventsTransitioningToMomentumScrolling):
+        (WebCore::areWheelEventsEndingNonMomentumScrolling):
+        (WebCore::EventHandler::doOrScheduleClearLatchedStateIfNeeded):
+
 2016-03-29  Alex Christensen  <[email protected]>
 
         Fix Windows build.

Modified: trunk/Source/WebCore/page/EventHandler.cpp (198804 => 198805)


--- trunk/Source/WebCore/page/EventHandler.cpp	2016-03-29 22:15:27 UTC (rev 198804)
+++ trunk/Source/WebCore/page/EventHandler.cpp	2016-03-29 22:17:04 UTC (rev 198805)
@@ -379,6 +379,9 @@
 #if ENABLE(CURSOR_SUPPORT)
     , m_cursorUpdateTimer(*this, &EventHandler::cursorUpdateTimerFired)
 #endif
+#if PLATFORM(MAC)
+    , m_pendingMomentumWheelEventsTimer(*this, &EventHandler::clearLatchedState)
+#endif
     , m_autoscrollController(std::make_unique<AutoscrollController>())
 #if !ENABLE(IOS_TOUCH_EVENTS)
     , m_fakeMouseMoveEventTimer(*this, &EventHandler::fakeMouseMoveEventTimerFired)
@@ -2591,6 +2594,10 @@
     return m_lastKnownMousePosition;
 }
 
+void EventHandler::clearOrScheduleClearingLatchedStateIfNeeded(const PlatformWheelEvent&)
+{
+    clearLatchedState();
+}
 #endif
 
 bool EventHandler::handleWheelEvent(const PlatformWheelEvent& event)

Modified: trunk/Source/WebCore/page/EventHandler.h (198804 => 198805)


--- trunk/Source/WebCore/page/EventHandler.h	2016-03-29 22:15:27 UTC (rev 198804)
+++ trunk/Source/WebCore/page/EventHandler.h	2016-03-29 22:17:04 UTC (rev 198805)
@@ -461,6 +461,7 @@
     void autoHideCursorTimerFired();
 #endif
 
+    void clearOrScheduleClearingLatchedStateIfNeeded(const PlatformWheelEvent&);
     void clearLatchedState();
 
     Frame& m_frame;
@@ -489,6 +490,9 @@
     Timer m_cursorUpdateTimer;
 #endif
 
+#if PLATFORM(MAC)
+    Timer m_pendingMomentumWheelEventsTimer;
+#endif
     std::unique_ptr<AutoscrollController> m_autoscrollController;
     bool m_mouseDownMayStartAutoscroll { false };
     bool m_mouseDownWasInSubframe { false };

Modified: trunk/Source/WebCore/page/MainFrame.h (198804 => 198805)


--- trunk/Source/WebCore/page/MainFrame.h	2016-03-29 22:15:27 UTC (rev 198804)
+++ trunk/Source/WebCore/page/MainFrame.h	2016-03-29 22:17:04 UTC (rev 198805)
@@ -26,6 +26,7 @@
 #ifndef MainFrame_h
 #define MainFrame_h
 
+#include "EventHandler.h"
 #include "Frame.h"
 #include <wtf/Vector.h>

Modified: trunk/Source/WebCore/page/mac/EventHandlerMac.mm (198804 => 198805)


--- trunk/Source/WebCore/page/mac/EventHandlerMac.mm	2016-03-29 22:15:27 UTC (rev 198804)
+++ trunk/Source/WebCore/page/mac/EventHandlerMac.mm	2016-03-29 22:17:04 UTC (rev 198805)
@@ -75,6 +75,8 @@
 const double EventHandler::TextDragDelay = 0.15;
 #endif
 
+const double resetLatchedStateTimeout = 0.1;
+
 static RetainPtr<NSEvent>& currentNSEventSlot()
 {
     static NeverDestroyed<RetainPtr<NSEvent>> event;
@@ -931,8 +933,34 @@
     return is<HTMLFrameSetElement>(container) || is<HTMLBodyElement>(container);
 }
 
+void EventHandler::clearOrScheduleClearingLatchedStateIfNeeded(const PlatformWheelEvent& event)
+{
+    if (!m_frame.isMainFrame())
+        return;
+
+    // Platform does not provide an indication that it will switch from non-momentum to momentum scrolling
+    // when handling wheel events.
+    // Logic below installs a timer when non-momentum scrolling ends. If momentum scroll does not start within that interval,
+    // reset the latched state. If it does, stop the timer, leaving the latched state untouched.
+    if (!m_pendingMomentumWheelEventsTimer.isActive()) {
+        if (event.isEndOfNonMomentumScroll())
+            m_pendingMomentumWheelEventsTimer.startOneShot(resetLatchedStateTimeout);
+    } else {
+        // If another wheel event scrolling starts, stop the timer manually, and reset the latched state immediately.
+        if (event.shouldConsiderLatching()) {
+            m_frame.mainFrame().resetLatchingState();
+            m_pendingMomentumWheelEventsTimer.stop();
+        } else if (event.isTransitioningToMomentumScroll()) {
+            // Wheel events machinary is transitioning to momenthum scrolling, so no need to reset latched state. Stop the timer.
+            m_pendingMomentumWheelEventsTimer.stop();
+        }
+    }
+}
+
 void EventHandler::platformPrepareForWheelEvents(const PlatformWheelEvent& wheelEvent, const HitTestResult& result, RefPtr<Element>& wheelEventTarget, RefPtr<ContainerNode>& scrollableContainer, ScrollableArea*& scrollableArea, bool& isOverWidget)
 {
+    clearOrScheduleClearingLatchedStateIfNeeded(wheelEvent);
+
     FrameView* view = m_frame.view();
 
     scrollableContainer = nullptr;

Modified: trunk/Source/WebCore/platform/PlatformWheelEvent.h (198804 => 198805)


--- trunk/Source/WebCore/platform/PlatformWheelEvent.h	2016-03-29 22:15:27 UTC (rev 198804)
+++ trunk/Source/WebCore/platform/PlatformWheelEvent.h	2016-03-29 22:17:04 UTC (rev 198805)
@@ -160,6 +160,8 @@
         bool shouldConsiderLatching() const;
         bool shouldResetLatching() const;
         bool isEndOfMomentumScroll() const;
+        bool isEndOfNonMomentumScroll() const;
+        bool isTransitioningToMomentumScroll() const;
 #else
         bool useLatchedEventElement() const { return false; }
 #endif
@@ -211,6 +213,15 @@
         return m_phase == PlatformWheelEventPhaseNone && m_momentumPhase == PlatformWheelEventPhaseEnded;
     }
 
+    inline bool PlatformWheelEvent::isEndOfNonMomentumScroll() const
+    {
+        return m_phase == PlatformWheelEventPhaseEnded && m_momentumPhase == PlatformWheelEventPhaseNone;
+    }
+
+    inline bool PlatformWheelEvent::isTransitioningToMomentumScroll() const
+    {
+        return m_phase == PlatformWheelEventPhaseNone && m_momentumPhase == PlatformWheelEventPhaseBegan;
+    }
 #endif
 
 } // namespace WebCore

_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to