Bug 149567 - Expose a WKWebView API for allowing programmatic focus to trigger node assistance
Summary: Expose a WKWebView API for allowing programmatic focus to trigger node assist...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: WebKit Nightly Build
Hardware: iPhone / iPad iOS 9.0
: P2 Normal
Assignee: Nobody
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2015-09-25 14:01 PDT by Wenson Hsieh
Modified: 2015-10-09 11:26 PDT (History)
7 users (show)

See Also:


Attachments
Patch (11.18 KB, patch)
2015-09-25 19:20 PDT, Wenson Hsieh
darin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Wenson Hsieh 2015-09-25 14:01:53 PDT
Add a new API to WKWebView that allows programmatic focus (i.e. focus without user interaction) to bring up the keyboard on iOS.
Comment 1 Wenson Hsieh 2015-09-25 19:20:36 PDT
Created attachment 261960 [details]
Patch
Comment 2 Darin Adler 2015-09-26 18:54:54 PDT
Comment on attachment 261960 [details]
Patch

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

> Source/WebKit2/UIProcess/WebPageProxy.h:258
> +    NodeAssistanceArguments(const AssistedNodeInformation& nodeInformation, bool userIsInteracting, bool blurPreviousNode, API::Object* userData)
> +        : m_nodeInformation(nodeInformation)
> +        , m_userIsInteracting(userIsInteracting)
> +        , m_blurPreviousNode(blurPreviousNode)
> +        , m_userData(userData)
> +    {
> +    }

A struct does not need a constructor like this in modern C++. We can use aggregate initializer syntax instead.

> Source/WebKit2/UIProcess/ios/WebPageProxyIOS.mm:816
> +        m_deferredNodeAssistanceArguments = std::make_unique<NodeAssistanceArguments>(information, userIsInteracting, blurPreviousNode, userDataObject);

I believe the only change needed when we remove the constructor is to add braces here:

    std::make_unique<NodeAssistanceArguments>({ information, userIsInteracting, blurPreviousNode, userDataObject });
Comment 3 Wenson Hsieh 2015-09-26 21:01:33 PDT
Comment on attachment 261960 [details]
Patch

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

Thanks for taking a look! I'll switch to using an aggregate initializer before landing.

>> Source/WebKit2/UIProcess/WebPageProxy.h:258
>> +    }
> 
> A struct does not need a constructor like this in modern C++. We can use aggregate initializer syntax instead.

Got it. I'll take out the constructor.

>> Source/WebKit2/UIProcess/ios/WebPageProxyIOS.mm:816
>> +        m_deferredNodeAssistanceArguments = std::make_unique<NodeAssistanceArguments>(information, userIsInteracting, blurPreviousNode, userDataObject);
> 
> I believe the only change needed when we remove the constructor is to add braces here:
> 
>     std::make_unique<NodeAssistanceArguments>({ information, userIsInteracting, blurPreviousNode, userDataObject });

Got it. I'll keep this in mind.
Comment 4 Wenson Hsieh 2015-09-28 06:54:02 PDT
Committed r190278: <http://trac.webkit.org/changeset/190278>
Comment 5 mitz 2015-09-28 07:16:44 PDT
Comment on attachment 261960 [details]
Patch

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

> Source/WebKit2/UIProcess/API/Cocoa/WKWebView.h:248
> +@property (nonatomic) BOOL canAssistOnProgrammaticFocus;

This API declaration is missing availability information.

It’s surprising that this API uses the term “assist”. Is this something that’s used anywhere else in WebKit or UIKit API?

Have you considered other ways to expose this ability to clients? For example, calling the delegate when an element is focused and asking it to make a decision seems like it would afford more flexibility.
Comment 6 Wenson Hsieh 2015-09-28 08:33:25 PDT
<rdar://problem/22878323>
Comment 7 Wenson Hsieh 2015-09-28 09:08:30 PDT
Comment on attachment 261960 [details]
Patch

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

>> Source/WebKit2/UIProcess/API/Cocoa/WKWebView.h:248
>> +@property (nonatomic) BOOL canAssistOnProgrammaticFocus;
> 
> This API declaration is missing availability information.
> 
> It’s surprising that this API uses the term “assist”. Is this something that’s used anywhere else in WebKit or UIKit API?
> 
> Have you considered other ways to expose this ability to clients? For example, calling the delegate when an element is focused and asking it to make a decision seems like it would afford more flexibility.

Just to make sure -- would I add availability information via something like WK_AVAILABLE(10_12, 10_0)?

Good point, it looks like assistance is not really mentioned in the WebKit API and in UIKit it refers to accessibility-related things rather than bringing up the keyboard. Maybe I could rephrase it to be something like "A Boolean value indicating whether programmatic focus on an element is allowed to display the keyboard." Also, now that I'm aware keyboardDisplayRequiresUserAction exists for UIWebView, I think we should rename this flag to keyboardDisplayRequiresUserAction for better consistency.
Comment 8 jacob berkman 2015-10-09 11:23:32 PDT
will this fix https://bugs.webkit.org/show_bug.cgi?id=142757 ?
Comment 9 Wenson Hsieh 2015-10-09 11:26:33 PDT
Yes -- this is designed to tackle that problem. However, we're also working on an alternate (more powerful) means of providing ways to customize assistance behavior through https://bugs.webkit.org/show_bug.cgi?id=149646. Please keep in mind nothing is official yet, though.