Title: [190423] trunk
Revision
190423
Author
bfulg...@apple.com
Date
2015-10-01 12:31:22 -0700 (Thu, 01 Oct 2015)

Log Message

Latch does not clear when a scroll snap animation is triggered
https://bugs.webkit.org/show_bug.cgi?id=149681
<rdar://problem/22733922>

Reviewed by Simon Fraser.

Source/WebCore:

Test: tiled-drawing/scrolling/latched-div-with-scroll-snap.html

The scroll snap animation logic bypasses some of the normal clean-up code used
for wheel event handling. When this happens, the scroll latching state is not
cleared. In some cases this "stale" latch state is used when starting a new
gesture in the same frame. This causes the wrong page element to receive the
mouse wheel events, and hilarity ensues.

* page/mac/EventHandlerMac.mm:
(WebCore::EventHandler::platformNotifyIfEndGesture): Clear latched state when
ending the gesture.

LayoutTests:

Add a new test that confirms latched state is cleared when scroll snap animations
are triggered.

* tiled-drawing/scrolling/latched-div-with-scroll-snap-expected.txt: Added.
* tiled-drawing/scrolling/latched-div-with-scroll-snap.html: Added.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (190422 => 190423)


--- trunk/LayoutTests/ChangeLog	2015-10-01 19:12:51 UTC (rev 190422)
+++ trunk/LayoutTests/ChangeLog	2015-10-01 19:31:22 UTC (rev 190423)
@@ -1,3 +1,17 @@
+2015-10-01  Brent Fulgham  <bfulg...@apple.com>
+
+        Latch does not clear when a scroll snap animation is triggered
+        https://bugs.webkit.org/show_bug.cgi?id=149681
+        <rdar://problem/22733922>
+
+        Reviewed by Simon Fraser.
+
+        Add a new test that confirms latched state is cleared when scroll snap animations
+        are triggered.
+
+        * tiled-drawing/scrolling/latched-div-with-scroll-snap-expected.txt: Added.
+        * tiled-drawing/scrolling/latched-div-with-scroll-snap.html: Added.
+
 2015-10-01  Simon Fraser  <simon.fra...@apple.com>
 
         Remove some tests that now pass.

Added: trunk/LayoutTests/tiled-drawing/scrolling/latched-div-with-scroll-snap-expected.txt (0 => 190423)


--- trunk/LayoutTests/tiled-drawing/scrolling/latched-div-with-scroll-snap-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/tiled-drawing/scrolling/latched-div-with-scroll-snap-expected.txt	2015-10-01 19:31:22 UTC (rev 190423)
@@ -0,0 +1,10 @@
+Tests that the scroll-snap feature works properly in overflow regions.
+PASS div scrolled to next window.
+PASS div honored snap points.
+Testing that latch moves to bottom div:
+PASS top div did not move.
+PASS div scrolled to next window.
+PASS successfullyParsed is true
+
+TEST COMPLETE
+

Added: trunk/LayoutTests/tiled-drawing/scrolling/latched-div-with-scroll-snap.html (0 => 190423)


--- trunk/LayoutTests/tiled-drawing/scrolling/latched-div-with-scroll-snap.html	                        (rev 0)
+++ trunk/LayoutTests/tiled-drawing/scrolling/latched-div-with-scroll-snap.html	2015-10-01 19:31:22 UTC (rev 190423)
@@ -0,0 +1,227 @@
+<!DOCTYPE html>
+<html>
+    <head>
+        <style>
+            .gallery {
+                width: 200px;
+                height: 200px;
+                overflow-y: hidden;
+                overflow-x: auto;
+                margin-bottom: 2px;
+                -webkit-scroll-snap-points-x: repeat(100%);
+                -webkit-scroll-snap-type: mandatory;
+            }
+            .galleryDrawer {
+                width: 1200px;
+                height: 200px;
+            }
+            .colorBox {
+                height: 200px;
+                width: 200px;
+                float: left;
+            }
+            #itemH0, #itemV0 { background-color: red; }
+            #itemH1, #itemV1 { background-color: green; }
+            #itemH2, #itemV2 { background-color: blue; }
+            #itemH3, #itemV3 { background-color: aqua; }
+            #itemH4, #itemV4 { background-color: yellow; }
+            #itemH5, #itemV5 { background-color: fuchsia; }
+        </style>
+        <script src=""
+        <script>
+        window.jsTestIsAsync = true;
+
+        var divScrollPositionBeforeGlide;
+        var divScrollPositionBeforeSnap;
+        var divScrollPositionBeforeSecondaryMove;
+        var bottomDivScrollPositionBeforeSecondaryMove;
+
+        function locationInWindowCoordinates(element)
+        {
+            var position = {};
+            position.x = element.offsetLeft;
+            position.y = element.offsetTop;
+
+            while (element.offsetParent) {
+                position.x = position.x + element.offsetParent.offsetLeft;
+                position.y = position.y + element.offsetParent.offsetTop;
+                if (element == document.getElementsByTagName("body")[0])
+                    break;
+
+                element = element.offsetParent;
+            }
+
+            return position;
+        }
+
+
+        function checkForSecondaryScrollGlide(targetLabel)
+        {
+            var topDivTarget = document.getElementById("topTarget");
+
+            var actualTopPosition = topDivTarget.scrollLeft;
+            var expectedTopPosition = divScrollPositionBeforeSecondaryMove;
+
+            // The top div should NOT have scrolled (glided) to the next snap point.
+            if (actualTopPosition == expectedTopPosition)
+                testPassed("top div did not move.");
+            else
+                testFailed("top div was improperly latched. Expected " + expectedTopPosition + ", but got " + actualTopPosition);
+
+            var divTarget = document.getElementById("bottomTarget");
+
+            var actualPosition = divTarget.scrollLeft;
+            var expectedPosition = divTarget.clientWidth;
+
+            // The div should have scrolled (glided) to the next snap point.
+            if (actualPosition == expectedPosition)
+                testPassed("div scrolled to next window.");
+            else
+                testFailed("div did not honor snap points. Expected " + expectedPosition + ", but got " + actualPosition);
+
+            finishJSTest();
+        }
+
+        function performSwipeGestureOnElement(divTarget)
+        {
+            var windowPosition = locationInWindowCoordinates(divTarget);
+            var startPosX = windowPosition.x + 0.5 * divTarget.clientWidth;
+            var startPosY = windowPosition.y + 0.5 * divTarget.clientHeight;
+
+            eventSender.mouseMoveTo(startPosX, startPosY);
+            eventSender.mouseScrollByWithWheelAndMomentumPhases(-1, 0, 'began', 'none');
+            eventSender.mouseScrollByWithWheelAndMomentumPhases(-1, 0, 'changed', 'none');
+            eventSender.mouseScrollByWithWheelAndMomentumPhases(-1, 0, 'changed', 'none');
+            eventSender.mouseScrollByWithWheelAndMomentumPhases(-1, 0, 'changed', 'none');
+            eventSender.mouseScrollByWithWheelAndMomentumPhases(0, 0, 'ended', 'none');
+            eventSender.mouseScrollByWithWheelAndMomentumPhases(-1, 0, 'none', 'begin');
+            eventSender.mouseScrollByWithWheelAndMomentumPhases(-1, 0, 'none', 'continue');
+            eventSender.mouseScrollByWithWheelAndMomentumPhases(0, 0, 'none', 'end');
+        }
+
+        function scrollInSecondDiv()
+        {
+            debug("Testing that latch moves to bottom div:");
+            var topDivTarget = document.getElementById("topTarget");
+            divScrollPositionBeforeSecondaryMove = topDivTarget.scrollLeft;
+
+            var divTarget = document.getElementById("bottomTarget");
+
+            performSwipeGestureOnElement(divTarget);
+
+            eventSender.callAfterScrollingCompletes(function() { return checkForSecondaryScrollGlide(); });           
+        }
+
+        function checkForScrollSnap()
+        {
+            var divTarget = document.getElementById("topTarget");
+
+            var actualPosition = divTarget.scrollLeft;
+
+            // The div should have snapped back to the previous position
+            if (actualPosition != divScrollPositionBeforeSnap)
+                testFailed("div did not snap back to proper location for " + targetLabel +". Expected " + divScrollPositionBeforeSnap + ", but got " + actualPosition);
+            else
+                testPassed("div honored snap points.");
+
+            setTimeout(function() { scrollInSecondDiv() }, 0);
+        }
+
+        function scrollSnapTest()
+        {
+            var divTarget = document.getElementById("topTarget");
+ 
+            divScrollPositionBeforeSnap = divTarget.scrollLeft;
+
+            var windowPosition = locationInWindowCoordinates(divTarget);
+            var startPosX = windowPosition.x + 0.5 * divTarget.clientWidth;
+            var startPosY = windowPosition.y + 0.5 * divTarget.clientHeight;
+
+            eventSender.mouseMoveTo(startPosX, startPosY); // Make sure we are just outside the iFrame
+            eventSender.mouseScrollByWithWheelAndMomentumPhases(-1, 0, 'began', 'none');
+            eventSender.mouseScrollByWithWheelAndMomentumPhases(-1, 0, 'changed', 'none');
+            eventSender.mouseScrollByWithWheelAndMomentumPhases(-1, 0, 'changed', 'none');
+            eventSender.mouseScrollByWithWheelAndMomentumPhases(0, 0, 'ended', 'none');
+            eventSender.callAfterScrollingCompletes(function() { return checkForScrollSnap(); });
+        }
+
+        function checkForScrollGlide(targetLabel)
+        {
+            var divTarget = document.getElementById("topTarget");
+
+            var actualPosition = divTarget.scrollLeft;
+            var expectedPosition = divTarget.clientWidth;
+
+            // The div should have scrolled (glided) to the next snap point.
+            if (actualPosition == expectedPosition)
+                testPassed("div scrolled to next window.");
+            else
+                testFailed("div did not honor snap points. Expected " + expectedPosition + ", but got " + actualPosition);
+
+            setTimeout(function() { scrollSnapTest() }, 0);
+        }
+
+        function scrollGlideTest()
+        {
+            var divTarget = document.getElementById("topTarget");
+ 
+            divScrollPositionBeforeGlide = divTarget.scrollLeft;
+
+            performSwipeGestureOnElement(divTarget);
+    
+            eventSender.callAfterScrollingCompletes(function() { return checkForScrollGlide(); });
+        }
+
+        function onLoad()
+        {
+            if (window.eventSender) {
+                eventSender.monitorWheelEvents();
+                testRunner.dumpAsText();
+                testRunner.waitUntilDone();
+                setTimeout(function() { scrollGlideTest() }, 0);
+            } else {
+                var messageLocationH = document.getElementById('itemH0');
+                var message = document.createElement('div');
+                message.innerHTML = "<p>This test is better run under WebKitTestRunner.<br/>To manually test it, place the mouse pointer<br/>"
+                    + "inside the red region at the top of the page,<br/>and then use the mouse wheel or a two-finger<br/>swipe to make a"
+                    + "small swipe gesture with<br/>some momentum.<br/><br/>"
+                    + "The region should scroll to show a green region.<br/><br/>"
+                    + "Next, perform a small scroll gesture that does<br/>not involve momentum. You should begin to<br/>see one of the colors "
+                    + "to the side of the current<br/>green box. When you release the wheel, the<br/>region should scroll back to a single color.";
+                messageLocationH.appendChild(message);
+
+                var messageLocationV = document.getElementById('itemV0');
+                var message = document.createElement('div');
+                message.innerHTML = "<p>You should also be able to repeat these tests steps for this vertical region.<br/>"
+                messageLocationV.appendChild(message);
+            }
+        }
+        </script>
+    </head>
+    <body _onload_="onLoad();">
+        <div style="position: relative; width: 200px">
+            <div>Tests that the scroll-snap feature works properly in overflow regions.</div>
+            <div class="gallery" id="topTarget">
+                <div class="galleryDrawer">
+                    <div id="itemH0" class="colorBox"></div>
+                    <div id="itemH1" class="colorBox"></div>
+                    <div id="itemH2" class="colorBox"></div>
+                    <div id="itemH3" class="colorBox"></div>
+                    <div id="itemH4" class="colorBox"></div>
+                    <div id="itemH5" class="colorBox"></div>
+                </div>
+            </div>
+            <div class="gallery" id="bottomTarget">
+                <div class="galleryDrawer">
+                    <div id="itemV0" class="colorBox"></div>
+                    <div id="itemV1" class="colorBox"></div>
+                    <div id="itemV2" class="colorBox"></div>
+                    <div id="itemV3" class="colorBox"></div>
+                    <div id="itemV4" class="colorBox"></div>
+                    <div id="itemV5" class="colorBox"></div>
+                </div>
+            </div>
+            <div id="console"></div>
+        </div>
+    </body>
+</html>
\ No newline at end of file

Modified: trunk/Source/WebCore/ChangeLog (190422 => 190423)


--- trunk/Source/WebCore/ChangeLog	2015-10-01 19:12:51 UTC (rev 190422)
+++ trunk/Source/WebCore/ChangeLog	2015-10-01 19:31:22 UTC (rev 190423)
@@ -1,5 +1,25 @@
 2015-10-01  Brent Fulgham  <bfulg...@apple.com>
 
+        Latch does not clear when a scroll snap animation is triggered
+        https://bugs.webkit.org/show_bug.cgi?id=149681
+        <rdar://problem/22733922>
+
+        Reviewed by Simon Fraser.
+
+        Test: tiled-drawing/scrolling/latched-div-with-scroll-snap.html
+
+        The scroll snap animation logic bypasses some of the normal clean-up code used
+        for wheel event handling. When this happens, the scroll latching state is not
+        cleared. In some cases this "stale" latch state is used when starting a new
+        gesture in the same frame. This causes the wrong page element to receive the
+        mouse wheel events, and hilarity ensues.
+
+        * page/mac/EventHandlerMac.mm:
+        (WebCore::EventHandler::platformNotifyIfEndGesture): Clear latched state when
+        ending the gesture.
+
+2015-10-01  Brent Fulgham  <bfulg...@apple.com>
+
         [Win] Unreviewed CMake build fixes.
 
         * CMakeLists.txt: Match original Windows build flags for WebCoreTest.

Modified: trunk/Source/WebCore/page/mac/EventHandlerMac.mm (190422 => 190423)


--- trunk/Source/WebCore/page/mac/EventHandlerMac.mm	2015-10-01 19:12:51 UTC (rev 190422)
+++ trunk/Source/WebCore/page/mac/EventHandlerMac.mm	2015-10-01 19:31:22 UTC (rev 190423)
@@ -1068,8 +1068,11 @@
         return;
 
 #if ENABLE(CSS_SCROLL_SNAP)
-    if (ScrollAnimator* scrollAnimator = scrollableArea->existingScrollAnimator())
+    if (ScrollAnimator* scrollAnimator = scrollableArea->existingScrollAnimator()) {
         scrollAnimator->processWheelEventForScrollSnap(wheelEvent);
+        if (scrollAnimator->isScrollSnapInProgress())
+            clearLatchedState();
+    }
 #endif
 }
 
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to