- Revision
- 278515
- Author
- [email protected]
- Date
- 2021-06-04 17:57:43 -0700 (Fri, 04 Jun 2021)
Log Message
[iOS] Meaningful click heuristic should account for media state changes
https://bugs.webkit.org/show_bug.cgi?id=226655
rdar://78330664
Reviewed by Tim Horton and Devin Rousso.
Source/WebKit:
Teach the "meaningful click" heuristic about changes to media element state flags. See comments below for more
details.
Test: fast/events/ios/meaningful-click-when-playing-media.html
* WebProcess/WebCoreSupport/WebChromeClient.cpp:
(WebKit::WebChromeClient::isPlayingMediaDidChange):
* WebProcess/WebPage/WebPage.cpp:
(WebKit::WebPage::isPlayingMediaDidChange):
Refactor some logic here so that the WebChromeClient calls into WebPage, which then sends an IPC message to the
UI process and additionally calls into a private method for platform-specific logic (see WebPageIOS.mm).
* WebProcess/WebPage/WebPage.h:
Replace `m_didHandleOrPreventMouseDownOrMouseUpEventDuringSyntheticClick` with another flag,
`m_currentSyntheticClickMayNotBeMeaningful`, that is initially set to `true` at the beginning of
`WebPage::completeSyntheticClick`, and consulted after the events have been dispatched to see if anything has
set it to `false` (currently, this includes only playing media state changes and handled click events by the
page). If the flag is set to `false`, we then consider the click to have been "meaningful", with respect to the
`-_webView:didNotHandleTapAsMeaningfulClickAtPoint:` UI delegate method.
(WebKit::WebPage::platformIsPlayingMediaDidChange):
* WebProcess/WebPage/ios/WebPageIOS.mm:
(WebKit::WebPage::completeSyntheticClick):
(WebKit::WebPage::didHandleOrPreventMouseDownOrMouseUpEvent):
(WebKit::WebPage::platformIsPlayingMediaDidChange):
Make these set `m_currentSyntheticClickMayNotBeMeaningful` to `false`.
LayoutTests:
Add a layout test to verify that tapping to play or pause a video triggers "meaningful" synthetic clicks.
* fast/events/ios/meaningful-click-when-playing-media-expected.txt: Added.
* fast/events/ios/meaningful-click-when-playing-media.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (278514 => 278515)
--- trunk/LayoutTests/ChangeLog 2021-06-05 00:24:16 UTC (rev 278514)
+++ trunk/LayoutTests/ChangeLog 2021-06-05 00:57:43 UTC (rev 278515)
@@ -1,3 +1,16 @@
+2021-06-04 Wenson Hsieh <[email protected]>
+
+ [iOS] Meaningful click heuristic should account for media state changes
+ https://bugs.webkit.org/show_bug.cgi?id=226655
+ rdar://78330664
+
+ Reviewed by Tim Horton and Devin Rousso.
+
+ Add a layout test to verify that tapping to play or pause a video triggers "meaningful" synthetic clicks.
+
+ * fast/events/ios/meaningful-click-when-playing-media-expected.txt: Added.
+ * fast/events/ios/meaningful-click-when-playing-media.html: Added.
+
2021-06-04 Devin Rousso <[email protected]>
Web Inspector: Uncaught Exception: undefined is not an object (evaluating 'InspectorBackend.Enum.Page.ResourceType')
Added: trunk/LayoutTests/fast/events/ios/meaningful-click-when-playing-media-expected.txt (0 => 278515)
--- trunk/LayoutTests/fast/events/ios/meaningful-click-when-playing-media-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/events/ios/meaningful-click-when-playing-media-expected.txt 2021-06-05 00:57:43 UTC (rev 278515)
@@ -0,0 +1,12 @@
+This test exercises the 'meaningful click' heuristic when playing or pausing a video, and requires WebKitTestRunner.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+PASS didPlayVideo became true
+PASS didDispatchNonMeaningfulClickCallback is false
+PASS didPauseVideo became true
+PASS didDispatchNonMeaningfulClickCallback is false
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
Added: trunk/LayoutTests/fast/events/ios/meaningful-click-when-playing-media.html (0 => 278515)
--- trunk/LayoutTests/fast/events/ios/meaningful-click-when-playing-media.html (rev 0)
+++ trunk/LayoutTests/fast/events/ios/meaningful-click-when-playing-media.html 2021-06-05 00:57:43 UTC (rev 278515)
@@ -0,0 +1,61 @@
+<!DOCTYPE html> <!-- webkit-test-runner [ useFlexibleViewport=true ] -->
+<html>
+<head>
+<meta name="viewport" content="width=device-width, initial-scale=1, user-scalable=no">
+<style>
+body, html {
+ font-size: 18px;
+ font-family: system-ui;
+ width: 100%;
+ height: 100%;
+ margin: 0;
+}
+</style>
+<script src=""
+<script src=""
+<script>
+jsTestIsAsync = true;
+didPauseVideo = false;
+didPlayVideo = false;
+
+addEventListener("load", async () => {
+ description("This test exercises the 'meaningful click' heuristic when playing or pausing a video, and requires WebKitTestRunner.");
+ document.body.addEventListener("click", async () => {
+ const video = document.querySelector("video");
+ if (video.paused) {
+ await video.play();
+ didPlayVideo = true;
+ } else {
+ await video.pause();
+ didPauseVideo = true;
+ }
+ });
+
+ if (!window.testRunner)
+ return;
+
+ async function checkForNonMeaningfulClick(shouldPlay) {
+ didDispatchNonMeaningfulClickCallback = false;
+ testRunner.installDidNotHandleTapAsMeaningfulClickCallback(() => {
+ didDispatchNonMeaningfulClickCallback = true;
+ });
+
+ await UIHelper.activateElement(document.body);
+ await UIHelper.waitForDoubleTapDelay();
+ await new Promise(resolve => shouldBecomeEqual(shouldPlay ? "didPlayVideo" : "didPauseVideo", "true", resolve));
+ shouldBeFalse("didDispatchNonMeaningfulClickCallback");
+ testRunner.clearTestRunnerCallbacks();
+ }
+
+ await checkForNonMeaningfulClick(true);
+ await checkForNonMeaningfulClick(false);
+ finishJSTest();
+});
+</script>
+</head>
+<body>
+ <div id="description"></div>
+ <div id="console"></div>
+ <video loop src=""
+</body>
+</html>
\ No newline at end of file
Modified: trunk/Source/WebKit/ChangeLog (278514 => 278515)
--- trunk/Source/WebKit/ChangeLog 2021-06-05 00:24:16 UTC (rev 278514)
+++ trunk/Source/WebKit/ChangeLog 2021-06-05 00:57:43 UTC (rev 278515)
@@ -1,3 +1,41 @@
+2021-06-04 Wenson Hsieh <[email protected]>
+
+ [iOS] Meaningful click heuristic should account for media state changes
+ https://bugs.webkit.org/show_bug.cgi?id=226655
+ rdar://78330664
+
+ Reviewed by Tim Horton and Devin Rousso.
+
+ Teach the "meaningful click" heuristic about changes to media element state flags. See comments below for more
+ details.
+
+ Test: fast/events/ios/meaningful-click-when-playing-media.html
+
+ * WebProcess/WebCoreSupport/WebChromeClient.cpp:
+ (WebKit::WebChromeClient::isPlayingMediaDidChange):
+ * WebProcess/WebPage/WebPage.cpp:
+ (WebKit::WebPage::isPlayingMediaDidChange):
+
+ Refactor some logic here so that the WebChromeClient calls into WebPage, which then sends an IPC message to the
+ UI process and additionally calls into a private method for platform-specific logic (see WebPageIOS.mm).
+
+ * WebProcess/WebPage/WebPage.h:
+
+ Replace `m_didHandleOrPreventMouseDownOrMouseUpEventDuringSyntheticClick` with another flag,
+ `m_currentSyntheticClickMayNotBeMeaningful`, that is initially set to `true` at the beginning of
+ `WebPage::completeSyntheticClick`, and consulted after the events have been dispatched to see if anything has
+ set it to `false` (currently, this includes only playing media state changes and handled click events by the
+ page). If the flag is set to `false`, we then consider the click to have been "meaningful", with respect to the
+ `-_webView:didNotHandleTapAsMeaningfulClickAtPoint:` UI delegate method.
+
+ (WebKit::WebPage::platformIsPlayingMediaDidChange):
+ * WebProcess/WebPage/ios/WebPageIOS.mm:
+ (WebKit::WebPage::completeSyntheticClick):
+ (WebKit::WebPage::didHandleOrPreventMouseDownOrMouseUpEvent):
+ (WebKit::WebPage::platformIsPlayingMediaDidChange):
+
+ Make these set `m_currentSyntheticClickMayNotBeMeaningful` to `false`.
+
2021-06-04 Ryosuke Niwa <[email protected]>
Store MediaPlayer using WeakPtr in MediaPlayerPrivateRemote
Modified: trunk/Source/WebKit/WebProcess/WebCoreSupport/WebChromeClient.cpp (278514 => 278515)
--- trunk/Source/WebKit/WebProcess/WebCoreSupport/WebChromeClient.cpp 2021-06-05 00:24:16 UTC (rev 278514)
+++ trunk/Source/WebKit/WebProcess/WebCoreSupport/WebChromeClient.cpp 2021-06-05 00:57:43 UTC (rev 278515)
@@ -1235,7 +1235,7 @@
void WebChromeClient::isPlayingMediaDidChange(MediaProducer::MediaStateFlags state)
{
- m_page.send(Messages::WebPageProxy::IsPlayingMediaDidChange(state));
+ m_page.isPlayingMediaDidChange(state);
}
void WebChromeClient::handleAutoplayEvent(AutoplayEvent event, OptionSet<AutoplayEventFlags> flags)
Modified: trunk/Source/WebKit/WebProcess/WebPage/WebPage.cpp (278514 => 278515)
--- trunk/Source/WebKit/WebProcess/WebPage/WebPage.cpp 2021-06-05 00:24:16 UTC (rev 278514)
+++ trunk/Source/WebKit/WebProcess/WebPage/WebPage.cpp 2021-06-05 00:57:43 UTC (rev 278515)
@@ -7360,6 +7360,11 @@
WebProcess::singleton().ensureNetworkProcessConnection().connection().send(Messages::NetworkConnectionToWebProcess::SetCORSDisablingPatterns(m_identifier, m_corsDisablingPatterns), 0);
}
+void WebPage::isPlayingMediaDidChange(WebCore::MediaProducer::MediaStateFlags state)
+{
+ platformIsPlayingMediaDidChange();
+ send(Messages::WebPageProxy::IsPlayingMediaDidChange(state));
+}
#if ENABLE(MEDIA_USAGE)
void WebPage::addMediaUsageManagerSession(MediaSessionIdentifier identifier, const String& bundleIdentifier, const URL& pageURL)
Modified: trunk/Source/WebKit/WebProcess/WebPage/WebPage.h (278514 => 278515)
--- trunk/Source/WebKit/WebProcess/WebPage/WebPage.h 2021-06-05 00:24:16 UTC (rev 278514)
+++ trunk/Source/WebKit/WebProcess/WebPage/WebPage.h 2021-06-05 00:57:43 UTC (rev 278515)
@@ -1401,6 +1401,8 @@
void removeMediaUsageManagerSession(WebCore::MediaSessionIdentifier);
#endif
+ void isPlayingMediaDidChange(WebCore::MediaProducer::MediaStateFlags);
+
#if ENABLE(IMAGE_EXTRACTION)
void requestImageExtraction(WebCore::Element&, CompletionHandler<void(RefPtr<WebCore::Element>&&)>&&);
void updateWithImageExtractionResult(WebCore::ImageExtractionResult&&, const WebCore::ElementContext&, const WebCore::FloatPoint& location, CompletionHandler<void(ImageExtractionUpdateResult)>&&);
@@ -1897,6 +1899,8 @@
void consumeNetworkExtensionSandboxExtensions(const SandboxExtension::HandleArray&);
+ void platformIsPlayingMediaDidChange();
+
WebCore::PageIdentifier m_identifier;
std::unique_ptr<WebCore::Page> m_page;
@@ -2185,8 +2189,8 @@
RefPtr<WebCore::Node> m_potentialTapNode;
WebCore::FloatPoint m_potentialTapLocation;
RefPtr<WebCore::SecurityOrigin> m_potentialTapSecurityOrigin;
- bool m_didHandleOrPreventMouseDownOrMouseUpEventDuringSyntheticClick { false };
+ bool m_currentSyntheticClickMayNotBeMeaningful { true };
bool m_hasReceivedVisibleContentRectsAfterDidCommitLoad { false };
bool m_hasRestoredExposedContentRectAfterDidCommitLoad { false };
bool m_scaleWasSetByUIProcess { false };
@@ -2358,6 +2362,7 @@
inline bool WebPage::platformNeedsLayoutForEditorState(const WebCore::Frame&) const { return false; }
inline void WebPage::didHandleOrPreventMouseDownOrMouseUpEvent() { }
inline void WebPage::prepareToRunModalJavaScriptDialog() { }
+inline void WebPage::platformIsPlayingMediaDidChange() { }
#endif
} // namespace WebKit
Modified: trunk/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm (278514 => 278515)
--- trunk/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm 2021-06-05 00:24:16 UTC (rev 278514)
+++ trunk/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm 2021-06-05 00:57:43 UTC (rev 278515)
@@ -874,7 +874,7 @@
bool altKey = modifiers.contains(WebEvent::Modifier::AltKey);
bool metaKey = modifiers.contains(WebEvent::Modifier::MetaKey);
- m_didHandleOrPreventMouseDownOrMouseUpEventDuringSyntheticClick = false;
+ m_currentSyntheticClickMayNotBeMeaningful = true;
tapWasHandled |= mainframe.eventHandler().handleMousePressEvent(PlatformMouseEvent(roundedAdjustedPoint, roundedAdjustedPoint, LeftButton, PlatformEvent::MousePressed, 1, shiftKey, ctrlKey, altKey, metaKey, WallTime::now(), WebCore::ForceAtClick, syntheticClickType, pointerId));
if (m_isClosed)
@@ -903,7 +903,7 @@
return;
bool shouldDispatchDidNotHandleTapAsMeaningfulClickAtPoint = ([&] {
- if (m_didHandleOrPreventMouseDownOrMouseUpEventDuringSyntheticClick)
+ if (!m_currentSyntheticClickMayNotBeMeaningful)
return false;
if (oldFocusedElement != newFocusedElement)
@@ -924,8 +924,6 @@
if (!tapWasHandled || !nodeRespondingToClick.isElementNode())
send(Messages::WebPageProxy::DidNotHandleTapAsClick(roundedIntPoint(location)));
- m_didHandleOrPreventMouseDownOrMouseUpEventDuringSyntheticClick = false;
-
send(Messages::WebPageProxy::DidCompleteSyntheticClick());
}
@@ -947,7 +945,7 @@
void WebPage::didHandleOrPreventMouseDownOrMouseUpEvent()
{
- m_didHandleOrPreventMouseDownOrMouseUpEventDuringSyntheticClick = true;
+ m_currentSyntheticClickMayNotBeMeaningful = false;
}
void WebPage::handleDoubleTapForDoubleClickAtPoint(const IntPoint& point, OptionSet<WebEvent::Modifier> modifiers, TransactionID lastLayerTreeTransactionId)
@@ -4694,6 +4692,11 @@
scheduleEditorStateUpdateForStartOrEndContainerNodeIfNeeded(endContainer.get());
}
+void WebPage::platformIsPlayingMediaDidChange()
+{
+ m_currentSyntheticClickMayNotBeMeaningful = false;
+}
+
} // namespace WebKit
#undef RELEASE_LOG_IF_ALLOWED
