Summary: | Unconditionally return information in _autofillContext SPI when a field is focused | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Ricky Mondello <rmondello> | ||||||||
Component: | Forms | Assignee: | 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
Ricky Mondello
2021-02-12 10:29:54 PST
Created attachment 420155 [details]
patch
Looking for review, but not to land yet.
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 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? Created attachment 420182 [details]
Patch
API Test update added with Wenson’s help. (Thank you Wenson!) Looking for review, but not to merge yet. 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? Created attachment 420523 [details]
Patch
commit-queue failed to commit attachment 420523 [details] to WebKit repository. To retry, please set cq+ flag again.
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. Committed r273115: <https://commits.webkit.org/r273115> All reviewed patches have been landed. Closing bug and clearing flags on attachment 420523 [details]. Re-opened since this is blocked by bug 222191 Reverted r273115 for reason: Breaks autocorrect without the accompanying change in rdar://problem/74211293 Committed r273202 (234388@main): <https://commits.webkit.org/234388@main> Committed r273759: <https://commits.webkit.org/r273759> All reviewed patches have been landed. Closing bug and clearing flags on attachment 420523 [details]. |