Bug 225240 - [iOS] Add a heuristic to determine whether a synthetic click triggered any meaningful changes
Summary: [iOS] Add a heuristic to determine whether a synthetic click triggered any me...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: UI Events (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Wenson Hsieh
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2021-04-30 11:30 PDT by Wenson Hsieh
Modified: 2021-04-30 14:30 PDT (History)
4 users (show)

See Also:


Attachments
For EWS (40.30 KB, patch)
2021-04-30 12:10 PDT, Wenson Hsieh
no flags Details | Formatted Diff | Diff
Add missing WK_API_AVAILABLE (40.32 KB, patch)
2021-04-30 12:37 PDT, Wenson Hsieh
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Wenson Hsieh 2021-04-30 11:30:47 PDT
rdar://77221196
Comment 1 Wenson Hsieh 2021-04-30 12:10:26 PDT
Created attachment 427438 [details]
For EWS
Comment 2 Tim Horton 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?
Comment 3 Wenson Hsieh 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.
Comment 4 Wenson Hsieh 2021-04-30 12:37:25 PDT
Created attachment 427441 [details]
Add missing WK_API_AVAILABLE
Comment 5 EWS 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].