RESOLVED FIXED Bug 225240
[iOS] Add a heuristic to determine whether a synthetic click triggered any meaningful changes
https://bugs.webkit.org/show_bug.cgi?id=225240
Summary [iOS] Add a heuristic to determine whether a synthetic click triggered any me...
Wenson Hsieh
Reported 2021-04-30 11:30:47 PDT
Attachments
For EWS (40.30 KB, patch)
2021-04-30 12:10 PDT, Wenson Hsieh
no flags
Add missing WK_API_AVAILABLE (40.32 KB, patch)
2021-04-30 12:37 PDT, Wenson Hsieh
no flags
Wenson Hsieh
Comment 1 2021-04-30 12:10:26 PDT
Tim Horton
Comment 2 2021-04-30 12:19:38 PDT
Comment on attachment 427438 [details] For EWS View in context: https://bugs.webkit.org/attachment.cgi?id=427438&action=review > Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm:849 > + if (elementBounds.width() >= unobscuredRect.width() / 2 && elementBounds.height() >= unobscuredRect.height() / 2) I... expect that this heuristic will require some iteration, but at least getting the plumbing going seems fine. > Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm:904 > + if (!m_didHandleOrPreventMouseDownOrMouseUpEventDuringSyntheticClick && !isProbablyMeaningfulClick(nodeRespondingToClick)) > + send(Messages::WebPageProxy::DidNotHandleTapAsMeaningfulClickAtPoint(roundedIntPoint(location))); This isn't going to come into play for e.g. events from WKMouseGR. Does it matter?
Wenson Hsieh
Comment 3 2021-04-30 12:28:48 PDT
Comment on attachment 427438 [details] For EWS View in context: https://bugs.webkit.org/attachment.cgi?id=427438&action=review >> Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm:849 >> + if (elementBounds.width() >= unobscuredRect.width() / 2 && elementBounds.height() >= unobscuredRect.height() / 2) > > I... expect that this heuristic will require some iteration, but at least getting the plumbing going seems fine. Indeed — this is definitely going to need to be reworked :P I think we'll eventually want some kind of heuristic here to detect that the synthetic click has shown (or hidden) some kind of UI on the page, or programmatically moved focus, or…many other things. This size check is kind of like a proxy for that, in lieu of more sophisticated heuristics, since tapping on huge clickable elements on the page that don't prevent default or handle any default action *tend* to not trigger any meaningful change. >> Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm:904 >> + send(Messages::WebPageProxy::DidNotHandleTapAsMeaningfulClickAtPoint(roundedIntPoint(location))); > > This isn't going to come into play for e.g. events from WKMouseGR. Does it matter? Given the way this is going to be used by the client, I think it's okay for now that trackpad-on-iPadOS won't trigger calls into this delegate method. If it's necessary though, I could extend calls to this delegate to beyond this synthetic click code as a followup, since the code to dispatch clicks when using a trackpad on iOS is pretty distinct from this synthetic click codepath.
Wenson Hsieh
Comment 4 2021-04-30 12:37:25 PDT
Created attachment 427441 [details] Add missing WK_API_AVAILABLE
EWS
Comment 5 2021-04-30 14:30:27 PDT
Committed r276853 (237204@main): <https://commits.webkit.org/237204@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 427441 [details].
Note You need to log in before you can comment on or make changes to this bug.