Bug 221828

Summary: Unconditionally return information in _autofillContext SPI when a field is focused
Product: WebKit Reporter: Ricky Mondello <rmondello>
Component: FormsAssignee: Ricky Mondello <rmondello>
Status: RESOLVED FIXED    
Severity: Normal CC: cdumez, commit-queue, darin, rmondello, sam, thorton, webkit-bug-importer, wenson_hsieh
Priority: P2 Keywords: InRadar
Version: Other   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 222191    
Bug Blocks:    
Attachments:
Description Flags
patch
none
Patch
none
Patch none

Description Ricky Mondello 2021-02-12 10:29:54 PST
Unconditionally return information in _autofillContext SPI when a field is focused
Comment 1 Ricky Mondello 2021-02-12 10:30:11 PST
rdar://74211237
Comment 2 Ricky Mondello 2021-02-12 11:27:31 PST
Created attachment 420155 [details]
patch

Looking for review, but not to land yet.
Comment 3 Radar WebKit Bug Importer 2021-02-12 11:42:00 PST
<rdar://problem/74285044>
Comment 4 Wenson Hsieh 2021-02-12 11:49:10 PST
Comment on attachment 420155 [details]
patch

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

The change seems fine, though it would be good to have an API test for this too. There are some existing tests in WKWebViewAutoFillTests and KeyboardInputTests that exercise this code.

> Source/WebKit/ChangeLog:4
> +

Nit - extra newline here.
Comment 5 Sam Weinig 2021-02-12 12:00:28 PST
Comment on attachment 420155 [details]
patch

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

> Source/WebKit/ChangeLog:13
> +        * UIProcess/ios/WKContentViewInteraction.mm:
> +        (-[WKContentView _autofillContext]): Change the contract of _autofillContext to return information for any focused
> +            text field. Add a "version" key. Explicitly indicate whether we're in a login context. This SPI remains stringly
> +            typed for flexibility.

Can we add an API test for this?
Comment 6 Ricky Mondello 2021-02-12 14:53:26 PST
Created attachment 420182 [details]
Patch
Comment 7 Ricky Mondello 2021-02-15 11:38:38 PST
API Test update added with Wenson’s help. (Thank you Wenson!)

Looking for review, but not to merge yet.
Comment 8 Darin Adler 2021-02-15 17:11:25 PST
Comment on attachment 420182 [details]
Patch

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

> Source/WebKit/ChangeLog:11
> +            text field. Add a "version" key. Explicitly indicate whether we're in a login context. This SPI remains stringly

Ah, "stringly typed", a term of art I had not heard before.

> Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm:7904
> +    NSMutableDictionary *context = [[[NSMutableDictionary alloc] init] autorelease];

We are trying to cut down on the use of explicit calls to [x release] and [x autorelease] in WebKit code.

A better style would be to use a RetainPtr<NSMutableDictionary> like this:

    auto context = adoptNS([[NSMutableDictionary alloc] init]);

    context.get()[@"_WKAutofillContextVersion"] = @(2);

    ...

    return context.autorelease();

Those "get()" calls are ugly, but its the direction we are going.

> Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm:7908
> +    BOOL provideStrongPasswordAssistance = _focusRequiresStrongPasswordAssistance && _focusedElementInformation.elementType == WebKit::InputType::Password;
> +    if (provideStrongPasswordAssistance) {

Not sure we really need the local variable any more. Just collapse those into one line?
Comment 9 Ricky Mondello 2021-02-16 12:35:28 PST
Created attachment 420523 [details]
Patch
Comment 10 EWS 2021-02-17 13:06:35 PST
commit-queue failed to commit attachment 420523 [details] to WebKit repository. To retry, please set cq+ flag again.
Comment 11 EWS 2021-02-18 16:02:42 PST
rmondello@apple.com does not have committer permissions according to https://raw.githubusercontent.com/WebKit/WebKit/main/Tools/Scripts/webkitpy/common/config/contributors.json.

Rejecting attachment 420523 [details] from commit queue.
Comment 12 EWS 2021-02-18 16:59:01 PST
Committed r273115: <https://commits.webkit.org/r273115>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 420523 [details].
Comment 13 WebKit Commit Bot 2021-02-19 12:39:47 PST
Re-opened since this is blocked by bug 222191
Comment 14 Wenson Hsieh 2021-02-20 10:17:42 PST
Reverted r273115 for reason:

Breaks autocorrect without the accompanying change in rdar://problem/74211293

Committed r273202 (234388@main): <https://commits.webkit.org/234388@main>
Comment 15 EWS 2021-03-02 14:12:02 PST
Committed r273759: <https://commits.webkit.org/r273759>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 420523 [details].