- Revision
- 272584
- Author
- [email protected]
- Date
- 2021-02-09 09:57:55 -0800 (Tue, 09 Feb 2021)
Log Message
REGRESSION (r271660): Unable to interact with page after long-pressing image on images.google.com
https://bugs.webkit.org/show_bug.cgi?id=221584
<rdar://problem/74073581>
Reviewed by Andy Estes.
Source/WebKit:
After long pressing on an image with an active touchend event listener, it's possible to sometimes get stuck in
a state where further interaction with the page is impossible, due to all gesture recognizers (except for the
touchend deferral gestures) remaining in either Failed or Ended state.
When presenting the context menu with a long press, the touch event gesture transitions to Failed state due to
the active touch being cancelled. Normally, this invokes `-_webTouchEventsRecognized:` with all touch points
being released, which allows us to "lift" the deferred gesture gate by failing any deferring gesture recognizers
that are still in Possible state (i.e. deferring native gestures).
However, it's possible for touch deferring gestures (in particular, the touchend deferrer) introduced in r271660
to end (i.e. call `-touchesEnded:withEvent:`) before the touch event gesture gets a chance to call
`-_webTouchEventsRecognized:`. In this scenario, the touch end deferral gesture remains in Possible state, and
prevents the touch event gesture from subsequently firing its action (`-_webTouchEventsRecognized:`), presumably
because UIKit is waiting for all other gestures in the same subgraph as the touch event gesture recognizer to
Fail.
This effectively results in a gesture "deadlock", since the web touch event gesture recognizer won't call
`-_webTouchEventsRecognized:` until the touch end deferring gesture has failed, and the touch end deferring
gesture won't fail until `-_webTouchEventsRecognized:` is called. To fix this, we restore a bit of logic that
was removed with r271193, such that we allow our deferring gesture recognizers to transition to Failed state
underneath `-touchesEnded:withEvent:`, as long it's [1] not actively deferring any native gestures, and [2] the
web touch event gesture has already failed, so we aren't expecting a subsequent call to
`-_webTouchEventsRecognized:` until the deferring gesture has failed.
Note that this check for the touch event gesture recognizer's state (condition [2] above) is necessary to
prevent the touchend deferring gesture from failing prematurely (i.e. right before we're about to dispatch
preventable `touchend` events).
Test: fast/events/touch/ios/tap-after-long-press-on-image.html
* UIProcess/ios/WKContentViewInteraction.mm:
(-[WKContentView _isTouchStartDeferringGesture:]):
(-[WKContentView _isTouchEndDeferringGesture:]):
(-[WKContentView deferringGestureRecognizer:didEndTouchesWithEvent:]):
Add a delegate hook when a deferring gesture's ends touches; use this hook in `WKContentView` to "lift" the
gesture gate if needed, in the case where the touch event gesture recognizer has already failed and we can't
expect `-_webTouchEventsRecognized:` to be called with all touch points released.
(-[WKContentView contextMenuInteraction:willEndForConfiguration:animator:]):
Add a missing call to `-[WKWebView _didDismissContextMenu]` here to make `UIHelper.waitForContextMenuToHide()`
actually work with image and link context menus in WebKitTestRunner.
* UIProcess/ios/WKDeferringGestureRecognizer.h:
* UIProcess/ios/WKDeferringGestureRecognizer.mm:
(-[WKDeferringGestureRecognizer touchesEnded:withEvent:]):
LayoutTests:
Add a new layout test that (sometimes) exercises the problem. Given the nature of the bug, it doesn't seem
possible to write a test that reproduces the bug 100% of the time. See WebKit ChangeLog for more details.
* fast/events/touch/ios/tap-after-long-press-on-image-expected.txt: Added.
* fast/events/touch/ios/tap-after-long-press-on-image.html: Added.
* resources/ui-helper.js:
(UIHelper.EventStreamBuilder.prototype.wait):
Add a helper to simply increment the time offset when building an event stream.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (272583 => 272584)
--- trunk/LayoutTests/ChangeLog 2021-02-09 17:47:37 UTC (rev 272583)
+++ trunk/LayoutTests/ChangeLog 2021-02-09 17:57:55 UTC (rev 272584)
@@ -1,3 +1,21 @@
+2021-02-09 Wenson Hsieh <[email protected]>
+
+ REGRESSION (r271660): Unable to interact with page after long-pressing image on images.google.com
+ https://bugs.webkit.org/show_bug.cgi?id=221584
+ <rdar://problem/74073581>
+
+ Reviewed by Andy Estes.
+
+ Add a new layout test that (sometimes) exercises the problem. Given the nature of the bug, it doesn't seem
+ possible to write a test that reproduces the bug 100% of the time. See WebKit ChangeLog for more details.
+
+ * fast/events/touch/ios/tap-after-long-press-on-image-expected.txt: Added.
+ * fast/events/touch/ios/tap-after-long-press-on-image.html: Added.
+ * resources/ui-helper.js:
+ (UIHelper.EventStreamBuilder.prototype.wait):
+
+ Add a helper to simply increment the time offset when building an event stream.
+
2021-02-09 Youenn Fablet <[email protected]>
MediaStream-backed video elements should not compute the mediaType based on track muted states
Added: trunk/LayoutTests/fast/events/touch/ios/tap-after-long-press-on-image-expected.txt (0 => 272584)
--- trunk/LayoutTests/fast/events/touch/ios/tap-after-long-press-on-image-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/events/touch/ios/tap-after-long-press-on-image-expected.txt 2021-02-09 17:57:55 UTC (rev 272584)
@@ -0,0 +1,11 @@
+Verifies that it is possible to tap a button after long pressing an image with active event listeners. To manually run the test, long press the image to show the context menu, and then tap the button.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+PASS Presented the context menu
+PASS Dismissed the context menu
+PASS Clicked the button
+PASS successfullyParsed is true
+
+TEST COMPLETE
+Tap here
Added: trunk/LayoutTests/fast/events/touch/ios/tap-after-long-press-on-image.html (0 => 272584)
--- trunk/LayoutTests/fast/events/touch/ios/tap-after-long-press-on-image.html (rev 0)
+++ trunk/LayoutTests/fast/events/touch/ios/tap-after-long-press-on-image.html 2021-02-09 17:57:55 UTC (rev 272584)
@@ -0,0 +1,59 @@
+<!DOCTYPE html> <!-- webkit-test-runner [ useFlexibleViewport=true ] -->
+<html>
+<meta name="viewport" content="initial-scale=1, width=device-width">
+<head>
+ <style>
+ img {
+ position: absolute;
+ top: 0;
+ left: 0;
+ width: 300px;
+ height: 200px;
+ }
+
+ button {
+ margin-top: 250px;
+ font-size: 32px;
+ }
+ </style>
+ <script src=""
+ <script src=""
+ <script>
+ jsTestIsAsync = true;
+
+ addEventListener("load", async () => {
+ description("Verifies that it is possible to tap a button after long pressing an image with active event listeners. To manually run the test, long press the image to show the context menu, and then tap the button.");
+
+ const button = document.querySelector("button");
+ button.addEventListener("click", () => {
+ testPassed("Clicked the button");
+ finishJSTest();
+ });
+
+ const image = document.querySelector("img");
+ image.addEventListener("touchstart", () => { });
+ image.addEventListener("touchend", () => { });
+ image.addEventListener("touchcancel", () => { });
+
+ if (!window.testRunner || !testRunner.runUIScript)
+ return;
+
+ await UIHelper.sendEventStream(new UIHelper.EventStreamBuilder().begin(100, 100).wait(2).end().takeResult());
+ await UIHelper.waitForContextMenuToShow();
+ testPassed("Presented the context menu");
+
+ await UIHelper.tapAt(0, 0);
+ await UIHelper.waitForContextMenuToHide();
+ testPassed("Dismissed the context menu");
+
+ await UIHelper.activateElement(button);
+ });
+ </script>
+</head>
+<body>
+ <div id="description"></div>
+ <div id="console"></div>
+ <img src=""
+ <button>Tap here</button>
+</body>
+</html>
Modified: trunk/LayoutTests/resources/ui-helper.js (272583 => 272584)
--- trunk/LayoutTests/resources/ui-helper.js 2021-02-09 17:47:37 UTC (rev 272583)
+++ trunk/LayoutTests/resources/ui-helper.js 2021-02-09 17:57:55 UTC (rev 272584)
@@ -1527,6 +1527,11 @@
return this;
}
+ wait(duration) {
+ this.currentTimeOffset += duration;
+ return this;
+ }
+
move(x, y, duration = 0) {
const previousTimeOffset = this.currentTimeOffset;
this.currentTimeOffset += duration;
Modified: trunk/Source/WebKit/ChangeLog (272583 => 272584)
--- trunk/Source/WebKit/ChangeLog 2021-02-09 17:47:37 UTC (rev 272583)
+++ trunk/Source/WebKit/ChangeLog 2021-02-09 17:57:55 UTC (rev 272584)
@@ -1,3 +1,59 @@
+2021-02-09 Wenson Hsieh <[email protected]>
+
+ REGRESSION (r271660): Unable to interact with page after long-pressing image on images.google.com
+ https://bugs.webkit.org/show_bug.cgi?id=221584
+ <rdar://problem/74073581>
+
+ Reviewed by Andy Estes.
+
+ After long pressing on an image with an active touchend event listener, it's possible to sometimes get stuck in
+ a state where further interaction with the page is impossible, due to all gesture recognizers (except for the
+ touchend deferral gestures) remaining in either Failed or Ended state.
+
+ When presenting the context menu with a long press, the touch event gesture transitions to Failed state due to
+ the active touch being cancelled. Normally, this invokes `-_webTouchEventsRecognized:` with all touch points
+ being released, which allows us to "lift" the deferred gesture gate by failing any deferring gesture recognizers
+ that are still in Possible state (i.e. deferring native gestures).
+
+ However, it's possible for touch deferring gestures (in particular, the touchend deferrer) introduced in r271660
+ to end (i.e. call `-touchesEnded:withEvent:`) before the touch event gesture gets a chance to call
+ `-_webTouchEventsRecognized:`. In this scenario, the touch end deferral gesture remains in Possible state, and
+ prevents the touch event gesture from subsequently firing its action (`-_webTouchEventsRecognized:`), presumably
+ because UIKit is waiting for all other gestures in the same subgraph as the touch event gesture recognizer to
+ Fail.
+
+ This effectively results in a gesture "deadlock", since the web touch event gesture recognizer won't call
+ `-_webTouchEventsRecognized:` until the touch end deferring gesture has failed, and the touch end deferring
+ gesture won't fail until `-_webTouchEventsRecognized:` is called. To fix this, we restore a bit of logic that
+ was removed with r271193, such that we allow our deferring gesture recognizers to transition to Failed state
+ underneath `-touchesEnded:withEvent:`, as long it's [1] not actively deferring any native gestures, and [2] the
+ web touch event gesture has already failed, so we aren't expecting a subsequent call to
+ `-_webTouchEventsRecognized:` until the deferring gesture has failed.
+
+ Note that this check for the touch event gesture recognizer's state (condition [2] above) is necessary to
+ prevent the touchend deferring gesture from failing prematurely (i.e. right before we're about to dispatch
+ preventable `touchend` events).
+
+ Test: fast/events/touch/ios/tap-after-long-press-on-image.html
+
+ * UIProcess/ios/WKContentViewInteraction.mm:
+ (-[WKContentView _isTouchStartDeferringGesture:]):
+ (-[WKContentView _isTouchEndDeferringGesture:]):
+ (-[WKContentView deferringGestureRecognizer:didEndTouchesWithEvent:]):
+
+ Add a delegate hook when a deferring gesture's ends touches; use this hook in `WKContentView` to "lift" the
+ gesture gate if needed, in the case where the touch event gesture recognizer has already failed and we can't
+ expect `-_webTouchEventsRecognized:` to be called with all touch points released.
+
+ (-[WKContentView contextMenuInteraction:willEndForConfiguration:animator:]):
+
+ Add a missing call to `-[WKWebView _didDismissContextMenu]` here to make `UIHelper.waitForContextMenuToHide()`
+ actually work with image and link context menus in WebKitTestRunner.
+
+ * UIProcess/ios/WKDeferringGestureRecognizer.h:
+ * UIProcess/ios/WKDeferringGestureRecognizer.mm:
+ (-[WKDeferringGestureRecognizer touchesEnded:withEvent:]):
+
2021-02-09 Philippe Normand <[email protected]>
Unreviewed, tvOS build fix after r272573
Modified: trunk/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm (272583 => 272584)
--- trunk/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm 2021-02-09 17:47:37 UTC (rev 272583)
+++ trunk/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm 2021-02-09 17:57:55 UTC (rev 272584)
@@ -1795,6 +1795,18 @@
[gesture setDefaultPrevented:preventNativeGestures];
}
+- (BOOL)_isTouchStartDeferringGesture:(WKDeferringGestureRecognizer *)gesture
+{
+ return gesture == _touchStartDeferringGestureRecognizerForSyntheticTapGestures || gesture == _touchStartDeferringGestureRecognizerForImmediatelyResettableGestures
+ || gesture == _touchStartDeferringGestureRecognizerForDelayedResettableGestures;
+}
+
+- (BOOL)_isTouchEndDeferringGesture:(WKDeferringGestureRecognizer *)gesture
+{
+ return gesture == _touchEndDeferringGestureRecognizerForSyntheticTapGestures || gesture == _touchEndDeferringGestureRecognizerForImmediatelyResettableGestures
+ || gesture == _touchEndDeferringGestureRecognizerForDelayedResettableGestures;
+}
+
#endif // ENABLE(IOS_TOUCH_EVENTS)
static NSValue *nsSizeForTapHighlightBorderRadius(WebCore::IntSize borderRadius, CGFloat borderRadiusScale)
@@ -7370,6 +7382,28 @@
return ![self gestureRecognizer:deferringGestureRecognizer isInterruptingMomentumScrollingWithEvent:event];
}
+- (void)deferringGestureRecognizer:(WKDeferringGestureRecognizer *)deferringGestureRecognizer didEndTouchesWithEvent:(UIEvent *)event
+{
+ if (deferringGestureRecognizer.state != UIGestureRecognizerStatePossible)
+ return;
+
+#if ENABLE(IOS_TOUCH_EVENTS)
+ if (_page->isHandlingPreventableTouchStart() && [self _isTouchStartDeferringGesture:deferringGestureRecognizer])
+ return;
+
+ if (_page->isHandlingPreventableTouchEnd() && [self _isTouchEndDeferringGesture:deferringGestureRecognizer])
+ return;
+#endif
+
+ if ([_touchEventGestureRecognizer state] == UIGestureRecognizerStatePossible)
+ return;
+
+ // In the case where the touch event gesture recognizer has failed or ended already and we are not in the middle of handling
+ // an asynchronous (but preventable) touch event, this is our last chance to lift the gesture "gate" by failing the deferring
+ // gesture recognizer.
+ deferringGestureRecognizer.state = UIGestureRecognizerStateFailed;
+}
+
- (BOOL)deferringGestureRecognizer:(WKDeferringGestureRecognizer *)deferringGestureRecognizer shouldDeferOtherGestureRecognizer:(UIGestureRecognizer *)gestureRecognizer
{
#if ENABLE(IOS_TOUCH_EVENTS)
@@ -10039,6 +10073,7 @@
if (!strongSelf)
return;
[strongSelf _removeContextMenuViewIfPossible];
+ [strongSelf->_webView _didDismissContextMenu];
}];
}
Modified: trunk/Source/WebKit/UIProcess/ios/WKDeferringGestureRecognizer.h (272583 => 272584)
--- trunk/Source/WebKit/UIProcess/ios/WKDeferringGestureRecognizer.h 2021-02-09 17:47:37 UTC (rev 272583)
+++ trunk/Source/WebKit/UIProcess/ios/WKDeferringGestureRecognizer.h 2021-02-09 17:57:55 UTC (rev 272584)
@@ -31,6 +31,7 @@
@protocol WKDeferringGestureRecognizerDelegate
- (BOOL)deferringGestureRecognizer:(WKDeferringGestureRecognizer *)deferringGestureRecognizer shouldDeferGesturesAfterBeginningTouchesWithEvent:(UIEvent *)event;
+- (BOOL)deferringGestureRecognizer:(WKDeferringGestureRecognizer *)deferringGestureRecognizer didEndTouchesWithEvent:(UIEvent *)event;
- (BOOL)deferringGestureRecognizer:(WKDeferringGestureRecognizer *)deferringGestureRecognizer shouldDeferOtherGestureRecognizer:(UIGestureRecognizer *)gestureRecognizer;
@end
Modified: trunk/Source/WebKit/UIProcess/ios/WKDeferringGestureRecognizer.mm (272583 => 272584)
--- trunk/Source/WebKit/UIProcess/ios/WKDeferringGestureRecognizer.mm 2021-02-09 17:47:37 UTC (rev 272583)
+++ trunk/Source/WebKit/UIProcess/ios/WKDeferringGestureRecognizer.mm 2021-02-09 17:57:55 UTC (rev 272584)
@@ -55,6 +55,13 @@
self.state = UIGestureRecognizerStateFailed;
}
+- (void)touchesEnded:(NSSet<UITouch *> *)touches withEvent:(UIEvent *)event
+{
+ [super touchesEnded:touches withEvent:event];
+
+ [_deferringGestureDelegate deferringGestureRecognizer:self didEndTouchesWithEvent:event];
+}
+
- (void)touchesCancelled:(NSSet<UITouch *> *)touches withEvent:(UIEvent *)event
{
[super touchesCancelled:touches withEvent:event];
