Bug 156118

Summary: [iOS] Allow clients in the bundle to know whether a field was focused by user interaction
Product: WebKit Reporter: Chelsea Pugh <cpugh>
Component: WebKit2Assignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: adele, commit-queue, cpugh, enrica, mitz
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch for [iOS] Allow clients to know whether a field was focused by user interaction
mitz: review-
[iOS] Allow clients in the bundle to know whether a field was focused by user interaction.txt
none
[iOS] Allow clients in the bundle to know whether a field was focused by user interaction.txt none

Description Chelsea Pugh 2016-04-01 15:07:51 PDT
Allow clients to know whether a field was focused by user interaction. Patch forthcoming.
Comment 1 Chelsea Pugh 2016-04-01 15:27:19 PDT
Created attachment 275435 [details]
Patch for [iOS] Allow clients to know whether a field was focused by user interaction
Comment 2 mitz 2016-04-01 15:54:39 PDT
Comment on attachment 275435 [details]
Patch for [iOS] Allow clients to know whether a field was focused by user interaction

View in context: https://bugs.webkit.org/attachment.cgi?id=275435&action=review

> Source/WebKit2/UIProcess/ios/WKContentViewInteraction.mm:3529
>      RetainPtr<WKFocusedElementInfo> focusedElementInfo = adoptNS([[WKFocusedElementInfo alloc] initWithAssistedNodeInformation:information isUserInitiated:userIsInteracting]);
>      BOOL shouldShowKeyboard;

This indicates that focusedElementInfo’s userInitiated property is initialized to the value of userIsInteracting. Given that, and given that the existing delegate method already receives focusedElementInfo as an argument, I think you shouldn’t introduce a new delegate method with a separate redundant argument.

But I am also not seeing how this method is the right place to make the change that balances out the change in WebPageIOS.mm below. Shouldn’t you be changing the implementation of willBeginInputSession in WKWebProcessPlugInBrowserContextController?
Comment 3 mitz 2016-04-01 15:58:49 PDT
To put things differently:

Currently, clients in the UI process already know whether the field was focused by user interaction, and get notified either way. Clients in the bundle get notified only when a field is focused by user interaction.

The bundle side of your patch makes it so clients in the bundle will get notified when a field gets focused, whether or not by user interaction, but it doesn’t let the clients find out whether the focus was user-initiated, and it doesn’t preserve the behavior for older clients of only being notified of user-initiated focus.
Comment 4 Chelsea Pugh 2016-04-04 13:14:00 PDT
Thanks for the feedback. New patch will be uploaded soon.
Comment 5 Chelsea Pugh 2016-04-04 13:15:29 PDT
Created attachment 275565 [details]
[iOS] Allow clients in the bundle to know whether a field was focused by user interaction.txt

Take 2 patch for this bug.
Comment 6 WebKit Commit Bot 2016-04-04 13:18:22 PDT
Attachment 275565 [details] did not pass style-queue:


ERROR: Source/WebKit2/WebProcess/InjectedBundle/API/mac/WKWebProcessPlugInBrowserContextController.mm:530:  An else should appear on the same line as the preceding }  [whitespace/newline] [4]
Total errors found: 1 in 5 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 7 Chelsea Pugh 2016-04-04 16:23:18 PDT
Created attachment 275588 [details]
[iOS] Allow clients in the bundle to know whether a field was focused by user interaction.txt

Take 3, fixed a style issue
Comment 8 WebKit Commit Bot 2016-04-04 21:22:59 PDT
Comment on attachment 275588 [details]
[iOS] Allow clients in the bundle to know whether a field was focused by user interaction.txt

Clearing flags on attachment: 275588

Committed r199040: <http://trac.webkit.org/changeset/199040>
Comment 9 WebKit Commit Bot 2016-04-04 21:23:03 PDT
All reviewed patches have been landed.  Closing bug.