[iOS 13] Taps that interrupt momentum scrolling are recognized as clicks
authorwenson_hsieh@apple.com <wenson_hsieh@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 8 Aug 2019 18:54:06 +0000 (18:54 +0000)
committerwenson_hsieh@apple.com <wenson_hsieh@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 8 Aug 2019 18:54:06 +0000 (18:54 +0000)
https://bugs.webkit.org/show_bug.cgi?id=200516
<rdar://problem/53889373>

Reviewed by Tim Horton.

Source/WebKit:

After <https://trac.webkit.org/r247656>, the -tracksImmediatelyWhileDecelerating property of WKScrollView and
WKChildScrollView is set to NO. This means that if a user interacts with the page while the scroll view is
decelerating (e.g. after momentum scrolling), the pan gesture recognizer will not be immediately recognized.
This gives other gesture recognizers, such as the synthetic click (single tap) gesture a chance to instead
recognize first. In this particular bug, this causes taps on the web view that are intended to only stop
momentum scrolling to instead activate clickable elements beneath the touch, such as links and buttons.

To mitigate this, we add some logic to prevent the click gesture recognizer from firing in the case where the
tap also causes the scroll view to decelerate. This heuristic is similar to the one introduced in r219310, which
has the same purpose of hiding gestures that stop momentum scrolling from the page, and also consults
-[UIScrollView _isInterruptingDeceleration].

Tests:  fast/scrolling/ios/click-events-during-momentum-scroll-in-main-frame.html
        fast/scrolling/ios/click-events-during-momentum-scroll-in-overflow-after-tap-on-body.html
        fast/scrolling/ios/click-events-during-momentum-scroll-in-overflow.html

* UIProcess/ios/WKContentViewInteraction.mm:
(-[WKContentView gestureRecognizerShouldBegin:]):

Return NO in the case of the single tap gesture if the UIScrollView most recently touched by the single tap
gesture (or one of its enclosing scroll views, up to the main WKScrollView) is being interrupted while
decelerating.

* UIProcess/ios/WKSyntheticTapGestureRecognizer.h:
* UIProcess/ios/WKSyntheticTapGestureRecognizer.mm:
(-[WKSyntheticTapGestureRecognizer reset]):
(-[WKSyntheticTapGestureRecognizer touchesBegan:withEvent:]):

Teach WKSyntheticTapGestureRecognizer to keep track of the last WKScrollView that was touched, for later use in
-gestureRecognizerShouldBegin:. To do this, we keep a weak reference to the first UIScrollView we find in the
set of touches.

(-[WKSyntheticTapGestureRecognizer lastTouchedScrollView]):

LayoutTests:

Add new layout tests. See below for details.

* fast/scrolling/ios/click-events-during-momentum-scroll-in-main-frame-expected.txt: Added.
* fast/scrolling/ios/click-events-during-momentum-scroll-in-main-frame.html: Added.

Add a test to verify that interrupting scrolling in the main frame using a tap doesn't fire a click event.

* fast/scrolling/ios/click-events-during-momentum-scroll-in-overflow-after-tap-on-body-expected.txt: Added.
* fast/scrolling/ios/click-events-during-momentum-scroll-in-overflow-after-tap-on-body.html: Added.

Add a test to verify that after triggering momentum scrolling in a fast subscrollable region, tapping outside of
the scroller will still fire a click event.

* fast/scrolling/ios/click-events-during-momentum-scroll-in-overflow-expected.txt: Added.
* fast/scrolling/ios/click-events-during-momentum-scroll-in-overflow.html: Added.

Add a test to verify that interrupting scrolling in a fast subscrollable region using a tap doesn't fire a
click event.

* resources/ui-helper.js:
(window.UIHelper.dragFromPointToPoint):
(window.UIHelper):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@248433 268f45cc-cd09-0410-ab3c-d52691b4dbfc

12 files changed:
LayoutTests/ChangeLog
LayoutTests/fast/scrolling/ios/click-events-during-momentum-scroll-in-main-frame-expected.txt [new file with mode: 0644]
LayoutTests/fast/scrolling/ios/click-events-during-momentum-scroll-in-main-frame.html [new file with mode: 0644]
LayoutTests/fast/scrolling/ios/click-events-during-momentum-scroll-in-overflow-after-tap-on-body-expected.txt [new file with mode: 0644]
LayoutTests/fast/scrolling/ios/click-events-during-momentum-scroll-in-overflow-after-tap-on-body.html [new file with mode: 0644]
LayoutTests/fast/scrolling/ios/click-events-during-momentum-scroll-in-overflow-expected.txt [new file with mode: 0644]
LayoutTests/fast/scrolling/ios/click-events-during-momentum-scroll-in-overflow.html [new file with mode: 0644]
LayoutTests/resources/ui-helper.js
Source/WebKit/ChangeLog
Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm
Source/WebKit/UIProcess/ios/WKSyntheticTapGestureRecognizer.h
Source/WebKit/UIProcess/ios/WKSyntheticTapGestureRecognizer.mm

index 8ae7f61..11b64fb 100644 (file)
@@ -1,3 +1,34 @@
+2019-08-08  Wenson Hsieh  <wenson_hsieh@apple.com>
+
+        [iOS 13] Taps that interrupt momentum scrolling are recognized as clicks
+        https://bugs.webkit.org/show_bug.cgi?id=200516
+        <rdar://problem/53889373>
+
+        Reviewed by Tim Horton.
+
+        Add new layout tests. See below for details.
+
+        * fast/scrolling/ios/click-events-during-momentum-scroll-in-main-frame-expected.txt: Added.
+        * fast/scrolling/ios/click-events-during-momentum-scroll-in-main-frame.html: Added.
+
+        Add a test to verify that interrupting scrolling in the main frame using a tap doesn't fire a click event.
+
+        * fast/scrolling/ios/click-events-during-momentum-scroll-in-overflow-after-tap-on-body-expected.txt: Added.
+        * fast/scrolling/ios/click-events-during-momentum-scroll-in-overflow-after-tap-on-body.html: Added.
+
+        Add a test to verify that after triggering momentum scrolling in a fast subscrollable region, tapping outside of
+        the scroller will still fire a click event.
+
+        * fast/scrolling/ios/click-events-during-momentum-scroll-in-overflow-expected.txt: Added.
+        * fast/scrolling/ios/click-events-during-momentum-scroll-in-overflow.html: Added.
+
+        Add a test to verify that interrupting scrolling in a fast subscrollable region using a tap doesn't fire a
+        click event.
+
+        * resources/ui-helper.js:
+        (window.UIHelper.dragFromPointToPoint):
+        (window.UIHelper):
+
 2019-08-08  Russell Epstein  <repstein@apple.com>
 
         Add Catalina Baselines for Font-related Tests.
diff --git a/LayoutTests/fast/scrolling/ios/click-events-during-momentum-scroll-in-main-frame-expected.txt b/LayoutTests/fast/scrolling/ios/click-events-during-momentum-scroll-in-main-frame-expected.txt
new file mode 100644 (file)
index 0000000..d977128
--- /dev/null
@@ -0,0 +1,10 @@
+Verifies that tapping the page during momentum scrolling does not dispatch click events to the page. To run the test manually, swipe up to scroll down; while the page is momentum scrolling, tap the screen to interrupt scrolling. The page should not observe any click events.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS document.scrollingElement.scrollTop passed 1000
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/fast/scrolling/ios/click-events-during-momentum-scroll-in-main-frame.html b/LayoutTests/fast/scrolling/ios/click-events-during-momentum-scroll-in-main-frame.html
new file mode 100644 (file)
index 0000000..aa1b7d6
--- /dev/null
@@ -0,0 +1,62 @@
+<!DOCTYPE html> <!-- webkit-test-runner [ useFlexibleViewport=true internal:AsyncOverflowScrollingEnabled=true internal:AsyncFrameScrollingEnabled=true ] -->
+<html>
+<head>
+<meta name="viewport" content="width=device-width, initial-scale=1, user-scalable=no">
+<script src="../../../resources/js-test.js"></script>
+<script src="../../../resources/ui-helper.js"></script>
+<style>
+html, body {
+    width: 100%;
+    margin: 0px;
+    padding: 0px;
+}
+
+#content {
+    width: 100px;
+    height: 5000px;
+    box-sizing: border-box;
+    padding: 20px;
+    background: linear-gradient(to bottom, red 0%, green 50%, blue 100%);
+}
+</style>
+<body onload="runTest()">
+<div id="content"></div>
+</body>
+<script>
+jsTestIsAsync = true;
+const minimumExpectedScrollTop = 1000;
+let reachedMinimumScrollPosition = false;
+
+addEventListener("scroll", observeScrollEvent);
+document.body.addEventListener("click", () => testFailed("Observed unexpected click event."), { once: true });
+
+function noteTestProgress() {
+    if (!window.progress)
+        progress = 0;
+    if (++progress == 2)
+        finishJSTest();
+}
+
+async function observeScrollEvent() {
+    if (!window.testRunner || document.scrollingElement.scrollTop < minimumExpectedScrollTop || reachedMinimumScrollPosition)
+        return;
+
+    reachedMinimumScrollPosition = true;
+    testPassed(`document.scrollingElement.scrollTop passed ${minimumExpectedScrollTop}`);
+    removeEventListener("scroll", observeScrollEvent);
+    await UIHelper.activateAt(160, document.scrollingElement.scrollTop + 350);
+    noteTestProgress();
+}
+
+async function runTest()
+{
+    description("Verifies that tapping the page during momentum scrolling does not dispatch click events to the page. To run the test manually, swipe up to scroll down; while the page is momentum scrolling, tap the screen to interrupt scrolling. The page should not observe any click events.");
+
+    if (!window.testRunner)
+        return;
+
+    await UIHelper.dragFromPointToPoint(160, 450, 160, 50, 0.1);
+    noteTestProgress();
+}
+</script>
+</html>
diff --git a/LayoutTests/fast/scrolling/ios/click-events-during-momentum-scroll-in-overflow-after-tap-on-body-expected.txt b/LayoutTests/fast/scrolling/ios/click-events-during-momentum-scroll-in-overflow-after-tap-on-body-expected.txt
new file mode 100644 (file)
index 0000000..000a31a
--- /dev/null
@@ -0,0 +1,11 @@
+Verifies that tapping outside of a subscrollable region during momentum scrolling dispatches click events. To run the test manually, swipe up on the black box to scroll it; while scrolling, tap outside of the scrollable area. The page should observe click events.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS scroller.scrollTop passed 400
+PASS Observed click event.
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/fast/scrolling/ios/click-events-during-momentum-scroll-in-overflow-after-tap-on-body.html b/LayoutTests/fast/scrolling/ios/click-events-during-momentum-scroll-in-overflow-after-tap-on-body.html
new file mode 100644 (file)
index 0000000..9369041
--- /dev/null
@@ -0,0 +1,80 @@
+<!DOCTYPE html> <!-- webkit-test-runner [ useFlexibleViewport=true internal:AsyncOverflowScrollingEnabled=true internal:AsyncFrameScrollingEnabled=true ] -->
+<html>
+<head>
+<meta name="viewport" content="width=device-width, initial-scale=1, user-scalable=no">
+<script src="../../../resources/js-test.js"></script>
+<script src="../../../resources/ui-helper.js"></script>
+<style>
+html, body {
+    width: 100%;
+    height: 100%;
+    margin: 0px;
+    padding: 0px;
+}
+
+#scroller {
+    width: 100%;
+    height: 50%;
+    overflow: scroll;
+    border: 4px solid black;
+    box-sizing: border-box;
+}
+
+#content {
+    width: 100px;
+    height: 5000px;
+    box-sizing: border-box;
+    padding: 20px;
+    background: linear-gradient(to bottom, red 0%, green 50%, blue 100%);
+}
+</style>
+<body onload="runTest()">
+    <div id="scroller">
+        <div id="content"></div>
+    </div>
+    <p id="description"></p>
+    <p id="console"></p>
+</body>
+<script>
+jsTestIsAsync = true;
+
+let reachedMinimumScrollPosition = false;
+const minimumExpectedScrollTop = 400;
+const scroller = document.getElementById("scroller");
+
+scroller.addEventListener("scroll", observeScrollEvent);
+document.body.addEventListener("click", () => {
+    testPassed("Observed click event.");
+    noteTestProgress();
+}, { once: true });
+
+function noteTestProgress() {
+    if (!window.progress)
+        progress = 0;
+    if (++progress == 3)
+        finishJSTest();
+}
+
+async function observeScrollEvent() {
+    if (!window.testRunner || scroller.scrollTop < minimumExpectedScrollTop || reachedMinimumScrollPosition)
+        return;
+
+    reachedMinimumScrollPosition = true;
+    testPassed(`scroller.scrollTop passed ${minimumExpectedScrollTop}`);
+    removeEventListener("scroll", observeScrollEvent);
+    await UIHelper.activateAt(160, 350);
+    noteTestProgress();
+}
+
+async function runTest()
+{
+    description("Verifies that tapping outside of a subscrollable region during momentum scrolling dispatches click events. To run the test manually, swipe up on the black box to scroll it; while scrolling, tap outside of the scrollable area. The page should observe click events.");
+
+    if (!window.testRunner)
+        return;
+
+    await UIHelper.dragFromPointToPoint(160, 200, 160, 50, 0.1);
+    noteTestProgress();
+}
+</script>
+</html>
diff --git a/LayoutTests/fast/scrolling/ios/click-events-during-momentum-scroll-in-overflow-expected.txt b/LayoutTests/fast/scrolling/ios/click-events-during-momentum-scroll-in-overflow-expected.txt
new file mode 100644 (file)
index 0000000..a1cf810
--- /dev/null
@@ -0,0 +1,10 @@
+Verifies that tapping a subscrollable region during momentum scrolling does not dispatch click events. To run the test manually, swipe up on the black box to scroll it; while scrolling, tap the scrollable area to interrupt scrolling. The scrollable area should not observe any click events.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS scroller.scrollTop passed 500
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/fast/scrolling/ios/click-events-during-momentum-scroll-in-overflow.html b/LayoutTests/fast/scrolling/ios/click-events-during-momentum-scroll-in-overflow.html
new file mode 100644 (file)
index 0000000..aa9979a
--- /dev/null
@@ -0,0 +1,77 @@
+<!DOCTYPE html> <!-- webkit-test-runner [ useFlexibleViewport=true internal:AsyncOverflowScrollingEnabled=true internal:AsyncFrameScrollingEnabled=true ] -->
+<html>
+<head>
+<meta name="viewport" content="width=device-width, initial-scale=1, user-scalable=no">
+<script src="../../../resources/js-test.js"></script>
+<script src="../../../resources/ui-helper.js"></script>
+<style>
+html, body {
+    width: 100%;
+    height: 100%;
+    margin: 0px;
+    padding: 0px;
+}
+
+#scroller {
+    width: 100%;
+    height: 75%;
+    overflow: scroll;
+    border: 4px solid black;
+    box-sizing: border-box;
+}
+
+#content {
+    width: 100px;
+    height: 5000px;
+    box-sizing: border-box;
+    padding: 20px;
+    background: linear-gradient(to bottom, red 0%, green 50%, blue 100%);
+}
+</style>
+<body onload="runTest()">
+    <div id="scroller">
+        <div id="content"></div>
+    </div>
+    <p id="description"></p>
+    <p id="console"></p>
+</body>
+<script>
+jsTestIsAsync = true;
+
+let reachedMinimumScrollPosition = false;
+const minimumExpectedScrollTop = 500;
+const scroller = document.getElementById("scroller");
+
+scroller.addEventListener("scroll", observeScrollEvent);
+scroller.addEventListener("click", () => testFailed("Observed unexpected click event."), { once: true });
+
+function noteTestProgress() {
+    if (!window.progress)
+        progress = 0;
+    if (++progress == 2)
+        finishJSTest();
+}
+
+async function observeScrollEvent() {
+    if (!window.testRunner || scroller.scrollTop < minimumExpectedScrollTop || reachedMinimumScrollPosition)
+        return;
+
+    reachedMinimumScrollPosition = true;
+    testPassed(`scroller.scrollTop passed ${minimumExpectedScrollTop}`);
+    removeEventListener("scroll", observeScrollEvent);
+    await UIHelper.activateAt(160, 250);
+    noteTestProgress();
+}
+
+async function runTest()
+{
+    description("Verifies that tapping a subscrollable region during momentum scrolling does not dispatch click events. To run the test manually, swipe up on the black box to scroll it; while scrolling, tap the scrollable area to interrupt scrolling. The scrollable area should not observe any click events.");
+
+    if (!window.testRunner)
+        return;
+
+    await UIHelper.dragFromPointToPoint(160, 250, 160, 50, 0.1);
+    noteTestProgress();
+}
+</script>
+</html>
index f18e4c9..8431edb 100644 (file)
@@ -1017,4 +1017,24 @@ window.UIHelper = class UIHelper {
             })()`, resolve);
         });
     }
+
+    static dragFromPointToPoint(fromX, fromY, toX, toY, duration)
+    {
+        if (!this.isWebKit2() || !this.isIOSFamily()) {
+            eventSender.mouseMoveTo(fromX, fromY);
+            eventSender.mouseDown();
+            eventSender.leapForward(duration * 1000);
+            eventSender.mouseMoveTo(toX, toY);
+            eventSender.mouseUp();
+            return Promise.resolve();
+        }
+
+        return new Promise(resolve => {
+            testRunner.runUIScript(`(() => {
+                uiController.dragFromPointToPoint(${fromX}, ${fromY}, ${toX}, ${toY}, ${duration}, () => {
+                    uiController.uiScriptComplete("");
+                });
+            })();`, resolve);
+        });
+    }
 }
index b710347..b3ecdf0 100644 (file)
@@ -1,3 +1,45 @@
+2019-08-08  Wenson Hsieh  <wenson_hsieh@apple.com>
+
+        [iOS 13] Taps that interrupt momentum scrolling are recognized as clicks
+        https://bugs.webkit.org/show_bug.cgi?id=200516
+        <rdar://problem/53889373>
+
+        Reviewed by Tim Horton.
+
+        After <https://trac.webkit.org/r247656>, the -tracksImmediatelyWhileDecelerating property of WKScrollView and
+        WKChildScrollView is set to NO. This means that if a user interacts with the page while the scroll view is
+        decelerating (e.g. after momentum scrolling), the pan gesture recognizer will not be immediately recognized.
+        This gives other gesture recognizers, such as the synthetic click (single tap) gesture a chance to instead
+        recognize first. In this particular bug, this causes taps on the web view that are intended to only stop
+        momentum scrolling to instead activate clickable elements beneath the touch, such as links and buttons.
+
+        To mitigate this, we add some logic to prevent the click gesture recognizer from firing in the case where the
+        tap also causes the scroll view to decelerate. This heuristic is similar to the one introduced in r219310, which
+        has the same purpose of hiding gestures that stop momentum scrolling from the page, and also consults
+        -[UIScrollView _isInterruptingDeceleration].
+
+        Tests:  fast/scrolling/ios/click-events-during-momentum-scroll-in-main-frame.html
+                fast/scrolling/ios/click-events-during-momentum-scroll-in-overflow-after-tap-on-body.html
+                fast/scrolling/ios/click-events-during-momentum-scroll-in-overflow.html
+
+        * UIProcess/ios/WKContentViewInteraction.mm:
+        (-[WKContentView gestureRecognizerShouldBegin:]):
+
+        Return NO in the case of the single tap gesture if the UIScrollView most recently touched by the single tap
+        gesture (or one of its enclosing scroll views, up to the main WKScrollView) is being interrupted while
+        decelerating.
+
+        * UIProcess/ios/WKSyntheticTapGestureRecognizer.h:
+        * UIProcess/ios/WKSyntheticTapGestureRecognizer.mm:
+        (-[WKSyntheticTapGestureRecognizer reset]):
+        (-[WKSyntheticTapGestureRecognizer touchesBegan:withEvent:]):
+
+        Teach WKSyntheticTapGestureRecognizer to keep track of the last WKScrollView that was touched, for later use in
+        -gestureRecognizerShouldBegin:. To do this, we keep a weak reference to the first UIScrollView we find in the
+        set of touches.
+
+        (-[WKSyntheticTapGestureRecognizer lastTouchedScrollView]):
+
 2019-08-08  Dean Jackson  <dino@apple.com>
 
         Use "safari" glyph for "Show Link Previews" contextual menu
index 38b5c57..21299b1 100644 (file)
@@ -2090,6 +2090,21 @@ static inline bool isSamePair(UIGestureRecognizer *a, UIGestureRecognizer *b, UI
     if (gestureRecognizer == _stylusSingleTapGestureRecognizer)
         return _webView._stylusTapGestureShouldCreateEditableImage;
 
+    if (gestureRecognizer == _singleTapGestureRecognizer) {
+        UIScrollView *mainScroller = _webView.scrollView;
+        UIView *view = [_singleTapGestureRecognizer lastTouchedScrollView] ?: mainScroller;
+        while (view) {
+            if ([view isKindOfClass:UIScrollView.class] && [(UIScrollView *)view _isInterruptingDeceleration])
+                return NO;
+
+            if (mainScroller == view)
+                break;
+
+            view = view.superview;
+        }
+        return YES;
+    }
+
     if (gestureRecognizer == _highlightLongPressGestureRecognizer
         || gestureRecognizer == _doubleTapGestureRecognizer
         || gestureRecognizer == _nonBlockingDoubleTapGestureRecognizer
index 977e44c..fd14326 100644 (file)
@@ -39,6 +39,7 @@
 @property (nonatomic, weak) UIWebTouchEventsGestureRecognizer *supportingWebTouchEventsGestureRecognizer;
 @property (nonatomic, readonly) NSNumber *lastActiveTouchIdentifier;
 #endif
+@property (nonatomic, readonly, weak) UIScrollView *lastTouchedScrollView;
 @end
 
 #endif
index 2a40854..b0f9bbd 100644 (file)
@@ -30,6 +30,7 @@
 
 #import <UIKit/UIGestureRecognizerSubclass.h>
 #import <wtf/RetainPtr.h>
+#import <wtf/WeakObjCPtr.h>
 
 @implementation WKSyntheticTapGestureRecognizer {
     id _gestureIdentifiedTarget;
@@ -39,6 +40,7 @@
     id _resetTarget;
     SEL _resetAction;
     RetainPtr<NSNumber> _lastActiveTouchIdentifier;
+    WeakObjCPtr<UIScrollView> _lastTouchedScrollView;
 }
 
 - (void)setGestureIdentifiedTarget:(id)target action:(SEL)action
 #if ENABLE(POINTER_EVENTS) 
     _lastActiveTouchIdentifier = nil;
 #endif
+    _lastTouchedScrollView = nil;
+}
+
+- (void)touchesBegan:(NSSet<UITouch *> *)touches withEvent:(UIEvent *)event
+{
+    [super touchesBegan:touches withEvent:event];
+
+    for (UITouch *touch in touches) {
+        if ([touch.view isKindOfClass:UIScrollView.class]) {
+            _lastTouchedScrollView = (UIScrollView *)touch.view;
+            break;
+        }
+    }
 }
 
 - (void)touchesEnded:(NSSet<UITouch *> *)touches withEvent:(UIEvent *)event
 #endif
 }
 
+- (UIScrollView *)lastTouchedScrollView
+{
+    return _lastTouchedScrollView.get().get();
+}
+
 - (NSNumber*)lastActiveTouchIdentifier
 {
     return _lastActiveTouchIdentifier.get();