Summary: | [iOS] Allow clients in the bundle to know whether a field was focused by user interaction | ||
---|---|---|---|
Product: | WebKit | Reporter: | Chelsea Pugh <cpugh> |
Component: | WebKit2 | Assignee: | 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
Chelsea Pugh
2016-04-01 15:07:51 PDT
Created attachment 275435 [details]
Patch for [iOS] Allow clients to know whether a field was focused by user interaction
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? 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. Thanks for the feedback. New patch will be uploaded soon. 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.
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.
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 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> All reviewed patches have been landed. Closing bug. |