Bug 149567

Summary: Expose a WKWebView API for allowing programmatic focus to trigger node assistance
Product: WebKit Reporter: Wenson Hsieh <wenson_hsieh>
Component: WebKit2Assignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: andersca, enrica, jberkman, mitz, sam, simon.fraser, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: iPhone / iPad   
OS: iOS 9.0   
Attachments:
Description Flags
Patch darin: review+

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.