Bug 225240

Summary: [iOS] Add a heuristic to determine whether a synthetic click triggered any meaningful changes
Product: WebKit Reporter: Wenson Hsieh <wenson_hsieh>
Component: UI EventsAssignee: Wenson Hsieh <wenson_hsieh>
Status: RESOLVED FIXED    
Severity: Normal CC: hi, megan_gardner, thorton, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
For EWS
none
Add missing WK_API_AVAILABLE none

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].