- Revision
- 255385
- Author
- wenson_hs...@apple.com
- Date
- 2020-01-29 14:22:43 -0800 (Wed, 29 Jan 2020)
Log Message
[iPadOS] Select popovers on amazon.com sometimes dismiss immediately after appearing
https://bugs.webkit.org/show_bug.cgi?id=206948
<rdar://problem/58954516>
Reviewed by Tim Horton.
Source/WebKit:
Amazon adds active touchstart event listeners to their select elements, where they call preventDefault() and
programmatically focus the select element. Doing so prevents clicks from being dispatched by allowing the web
touch event gesture to recognize instead of the synthetic click gesture, which (inside -_singleTapRecognized:)
would otherwise dismiss any currently presented popover.
After making touchstart events dispatch asynchronously, preventing touchstart no longer causes other native
gestures (such as the synthetic click gesture) to fail in the case where there is a sync touchmove or touchend
event listener, and the touch gesture has ended before the touchstart handler has finished executing and told
the UI process whether or not the touch was handled. This is because the touchend or touchmove is dispatched
synchronously while the touchstart is still being processed; while the web page sees the correct order of
events, the UI process will end up learning that the synchronous touch was handled before the response to the
asynchronously dispatched touchstart event has arrived.
Our current logic in the sync touch event dispatch case then unconditionally ends gesture deferral by calling
`doneDeferringNativeGestures` with `handled` equal to whether or not the touch event (a "touchend", in this
case) had been handled. However, in the case where touchstart event is prevented but the touchend event is not,
this will cause us to prematurely stop deferring gestures even though the page called `preventDefault()` in the
touchstart event handler, and allow the synthetic click gesture to recognize when it shouldn't.
To fix this, keep deferring native gestures after handling a sync touch in the case where a touchstart event is
still being handled; instead, remember whether the touch event was handled using a new member variable
(`m_handledSynchronousTouchEventWhileDispatchingPreventableTouchStart`), and consult this when the response to
the touchstart has been received in the UI process to determine whether platform gestures should be allowed to
recognize. This variable is reset once we're done handling the touchstart.
Test: fast/events/touch/ios/prevent-default-with-slow-touchstart-handler.html
* UIProcess/WebPageProxy.cpp:
(WebKit::WebPageProxy::handlePreventableTouchEvent):
* UIProcess/WebPageProxy.h:
LayoutTests:
Adds a new layout test to verify that preventing default on elements with slow, active touchstart event handlers
actually prevents click events from being dispatched.
* fast/events/touch/ios/prevent-default-with-slow-touchstart-handler-expected.txt: Added.
* fast/events/touch/ios/prevent-default-with-slow-touchstart-handler.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (255384 => 255385)
--- trunk/LayoutTests/ChangeLog 2020-01-29 22:16:13 UTC (rev 255384)
+++ trunk/LayoutTests/ChangeLog 2020-01-29 22:22:43 UTC (rev 255385)
@@ -1,3 +1,17 @@
+2020-01-29 Wenson Hsieh <wenson_hs...@apple.com>
+
+ [iPadOS] Select popovers on amazon.com sometimes dismiss immediately after appearing
+ https://bugs.webkit.org/show_bug.cgi?id=206948
+ <rdar://problem/58954516>
+
+ Reviewed by Tim Horton.
+
+ Adds a new layout test to verify that preventing default on elements with slow, active touchstart event handlers
+ actually prevents click events from being dispatched.
+
+ * fast/events/touch/ios/prevent-default-with-slow-touchstart-handler-expected.txt: Added.
+ * fast/events/touch/ios/prevent-default-with-slow-touchstart-handler.html: Added.
+
2020-01-29 Jacob Uphoff <jacob_uph...@apple.com>
REGRESSION: (253807) [ macOS iOS ] storage/indexeddb/intversion-long-queue-private.html is flaky failing
Added: trunk/LayoutTests/fast/events/touch/ios/prevent-default-with-slow-touchstart-handler-expected.txt (0 => 255385)
--- trunk/LayoutTests/fast/events/touch/ios/prevent-default-with-slow-touchstart-handler-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/events/touch/ios/prevent-default-with-slow-touchstart-handler-expected.txt 2020-01-29 22:22:43 UTC (rev 255385)
@@ -0,0 +1,11 @@
+Verifies that calling preventDefault() in touchstart and touchend listeners prevents click events from being dispatched when the web page takes a long time to process the touchstart event. To manually run the test, tap the two boxes and verify that no click events are dispatched.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS handledFirstTouch became true
+PASS handledSecondTouch became true
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
Added: trunk/LayoutTests/fast/events/touch/ios/prevent-default-with-slow-touchstart-handler.html (0 => 255385)
--- trunk/LayoutTests/fast/events/touch/ios/prevent-default-with-slow-touchstart-handler.html (rev 0)
+++ trunk/LayoutTests/fast/events/touch/ios/prevent-default-with-slow-touchstart-handler.html 2020-01-29 22:22:43 UTC (rev 255385)
@@ -0,0 +1,80 @@
+<!DOCTYPE html>
+<html>
+<head>
+<meta name="viewport" content="width=device-width, initial-scale=1">
+<script src=""
+<script src=""
+<style>
+.target {
+ width: 150px;
+ height: 150px;
+ margin-bottom: 1em;
+ border: 4px solid black;
+ color: white;
+ font-size: 20px;
+}
+
+#target1 {
+ background-color: rgb(52, 199, 89);
+}
+
+#target2 {
+ background-color: rgb(52, 199, 89);
+}
+</style>
+<script>
+jsTestIsAsync = true;
+handledFirstTouch = false;
+handledSecondTouch = false;
+
+function spinForDuration(duration) {
+ const startTime = Date.now();
+ while (true) {
+ if (Date.now() - startTime > duration)
+ break;
+ }
+}
+
+function handleClick(event) {
+ event.target.style.backgroundColor = "rgb(255, 59, 48)";
+ event.target.textContent = "A click event was dispatched.";
+ testFailed("Should not have handled a click");
+}
+
+addEventListener("load", async () => {
+ const target1 = document.getElementById("target1");
+ target1.addEventListener("touchstart", event => {
+ spinForDuration(100);
+ event.preventDefault();
+ });
+ target1.addEventListener("touchend", event => handledFirstTouch = true);
+ target1.addEventListener("click", handleClick);
+
+ const target2 = document.getElementById("target2");
+ target2.addEventListener("touchstart", () => spinForDuration(100));
+ target2.addEventListener("touchend", event => {
+ event.preventDefault();
+ handledSecondTouch = true;
+ });
+ target2.addEventListener("click", handleClick);
+
+ description("Verifies that calling preventDefault() in touchstart and touchend listeners prevents click events from being dispatched when the web page takes a long time to process the touchstart event. To manually run the test, tap the two boxes and verify that no click events are dispatched.");
+
+ if (!window.testRunner)
+ return;
+
+ await UIHelper.activateElement(target1);
+ await new Promise(resolve => shouldBecomeEqual("handledFirstTouch", "true", resolve));
+
+ await UIHelper.activateElement(target2);
+ await new Promise(resolve => shouldBecomeEqual("handledSecondTouch", "true", resolve));
+
+ finishJSTest();
+});
+</script>
+</head>
+<body>
+ <div class="target" id="target1"></div>
+ <div class="target" id="target2"></div>
+</body>
+</html>
\ No newline at end of file
Modified: trunk/Source/WebKit/ChangeLog (255384 => 255385)
--- trunk/Source/WebKit/ChangeLog 2020-01-29 22:16:13 UTC (rev 255384)
+++ trunk/Source/WebKit/ChangeLog 2020-01-29 22:22:43 UTC (rev 255385)
@@ -1,3 +1,42 @@
+2020-01-29 Wenson Hsieh <wenson_hs...@apple.com>
+
+ [iPadOS] Select popovers on amazon.com sometimes dismiss immediately after appearing
+ https://bugs.webkit.org/show_bug.cgi?id=206948
+ <rdar://problem/58954516>
+
+ Reviewed by Tim Horton.
+
+ Amazon adds active touchstart event listeners to their select elements, where they call preventDefault() and
+ programmatically focus the select element. Doing so prevents clicks from being dispatched by allowing the web
+ touch event gesture to recognize instead of the synthetic click gesture, which (inside -_singleTapRecognized:)
+ would otherwise dismiss any currently presented popover.
+
+ After making touchstart events dispatch asynchronously, preventing touchstart no longer causes other native
+ gestures (such as the synthetic click gesture) to fail in the case where there is a sync touchmove or touchend
+ event listener, and the touch gesture has ended before the touchstart handler has finished executing and told
+ the UI process whether or not the touch was handled. This is because the touchend or touchmove is dispatched
+ synchronously while the touchstart is still being processed; while the web page sees the correct order of
+ events, the UI process will end up learning that the synchronous touch was handled before the response to the
+ asynchronously dispatched touchstart event has arrived.
+
+ Our current logic in the sync touch event dispatch case then unconditionally ends gesture deferral by calling
+ `doneDeferringNativeGestures` with `handled` equal to whether or not the touch event (a "touchend", in this
+ case) had been handled. However, in the case where touchstart event is prevented but the touchend event is not,
+ this will cause us to prematurely stop deferring gestures even though the page called `preventDefault()` in the
+ touchstart event handler, and allow the synthetic click gesture to recognize when it shouldn't.
+
+ To fix this, keep deferring native gestures after handling a sync touch in the case where a touchstart event is
+ still being handled; instead, remember whether the touch event was handled using a new member variable
+ (`m_handledSynchronousTouchEventWhileDispatchingPreventableTouchStart`), and consult this when the response to
+ the touchstart has been received in the UI process to determine whether platform gestures should be allowed to
+ recognize. This variable is reset once we're done handling the touchstart.
+
+ Test: fast/events/touch/ios/prevent-default-with-slow-touchstart-handler.html
+
+ * UIProcess/WebPageProxy.cpp:
+ (WebKit::WebPageProxy::handlePreventableTouchEvent):
+ * UIProcess/WebPageProxy.h:
+
2020-01-29 Chris Dumez <cdu...@apple.com>
[iOS] Make sure unused service worker processes exit promptly on low memory warning
Modified: trunk/Source/WebKit/UIProcess/WebPageProxy.cpp (255384 => 255385)
--- trunk/Source/WebKit/UIProcess/WebPageProxy.cpp 2020-01-29 22:16:13 UTC (rev 255384)
+++ trunk/Source/WebKit/UIProcess/WebPageProxy.cpp 2020-01-29 22:22:43 UTC (rev 255385)
@@ -2903,10 +2903,13 @@
if (m_handlingPreventableTouchStartCount)
--m_handlingPreventableTouchStartCount;
+ bool handledOrFailedWithError = handled || error != CallbackBase::Error::None || m_handledSynchronousTouchEventWhileDispatchingPreventableTouchStart;
+ if (!isHandlingPreventableTouchStart())
+ m_handledSynchronousTouchEventWhileDispatchingPreventableTouchStart = false;
+
if (error == CallbackBase::Error::ProcessExited)
return;
- bool handledOrFailedWithError = handled || error != CallbackBase::Error::None;
didReceiveEvent(event.type(), handledOrFailedWithError);
pageClient().doneWithTouchEvent(event, handledOrFailedWithError);
if (!isHandlingPreventableTouchStart())
@@ -2926,7 +2929,10 @@
handled = true;
didReceiveEvent(event.type(), handled);
pageClient().doneWithTouchEvent(event, handled);
- pageClient().doneDeferringNativeGestures(handled);
+ if (!isHandlingPreventableTouchStart())
+ pageClient().doneDeferringNativeGestures(handled);
+ else if (handled)
+ m_handledSynchronousTouchEventWhileDispatchingPreventableTouchStart = true;
m_process->responsivenessTimer().stop();
}
Modified: trunk/Source/WebKit/UIProcess/WebPageProxy.h (255384 => 255385)
--- trunk/Source/WebKit/UIProcess/WebPageProxy.h 2020-01-29 22:16:13 UTC (rev 255384)
+++ trunk/Source/WebKit/UIProcess/WebPageProxy.h 2020-01-29 22:22:43 UTC (rev 255385)
@@ -2474,6 +2474,7 @@
Deque<QueuedTouchEvents> m_touchEventQueue;
#endif
+ bool m_handledSynchronousTouchEventWhileDispatchingPreventableTouchStart { false };
uint64_t m_handlingPreventableTouchStartCount { 0 };
#if ENABLE(INPUT_TYPE_COLOR)
