Bug 221828 – Unconditionally return information in _autofillContext SPI when a field is focused

• Status: RESOLVED
• Resolution: FIXED
• Component: Forms

[Ricky Mondello]

Unconditionally return information in _autofillContext SPI when a field is focused

[Ricky Mondello]

rdar://74211237

[Ricky Mondello]

Created attachment 420155 [details]
patch

Looking for review, but not to land yet.

[Radar WebKit Bug Importer]

<rdar://problem/74285044>

[Wenson Hsieh]

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.

[Sam Weinig]

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?

[Ricky Mondello]

Created attachment 420182 [details]
Patch

[Ricky Mondello]

API Test update added with Wenson’s help. (Thank you Wenson!)

Looking for review, but not to merge yet.

[Darin Adler]

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?

[Ricky Mondello]

Created attachment 420523 [details]
Patch

[EWS]

commit-queue failed to commit attachment 420523 [details] to WebKit repository. To retry, please set cq+ flag again.

[EWS]

[e-mail] 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.

[EWS]

Committed r273115: <https://commits.webkit.org/r273115>

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

[WebKit Commit Bot]

Re-opened since this is blocked by bug 222191

[Wenson Hsieh]

Reverted r273115 for reason:

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

Committed r273202 (234388@main): <https://commits.webkit.org/234388@main>

[EWS]

Committed r273759: <https://commits.webkit.org/r273759>

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