Async suggestions retrieval is breaking autofill |
||||||||
Issue description1) Go to amazon.com 2) Sign in and add a new address 3) Address form doesn't autofill I investigated and the cause seem to be crrev.com/c/921961 password controller, as the first suggestion provider, fails to return YES/NO to checkIfSuggestionsAvailableForForm: call. Therefore autofill agent is never queried. If I'm understanding correctly, your CL is trying to fix the forms that are added after page load. olivierrobin@ has a CL in flight that might address the issue for password controller as well. crrev.com/c/943321 what do you think about sharing that logic with autofill?
,
Mar 8 2018
Yes Autofill should probably do something similar. I didn't include it in my dynamic forms fix doc because I want to take the time to detail this separately. The reason being that we may not want to fill new fields as they are added to the form, since the user may not want that.
,
Mar 8 2018
Opps - wrongly associated this with dynamic forms filling. Yes that would be useful. I think on Desktop we currently re-parse the whole page when that happens. But I don't think we catch all cases.
,
Mar 8 2018
I'll check what're the problems here. Autofill already does async suggestions (here https://cs.chromium.org/chromium/src/components/autofill/ios/browser/autofill_agent.mm?q=checkIfSuggestionsAvailableForForm&sq=package:chromium&l=543 completion handler is copied in order to respond when suggestions are ready). Async here means - async between Obj-C and C++. As far as I understand the problem, that crrev.com/c/943321 is trying to solve, is to be able to get server-side predictions.
,
Mar 8 2018
yes, autofill does query suggestions asynchronously. It's ok if password controller does the same. It just has to make sure it always invokes the callback, otherwise autofill will never be queried for suggestions. see: https://cs.chromium.org/chromium/src/ios/chrome/browser/autofill/form_suggestion_controller.mm?l=250 crrev.com/c/943321 allows autofill to be notified of dynamically added forms. In this case used for getting server predictions. Is it possible that password controller can take advantage of it somehow? The more logic we share, the better I think.
,
Mar 8 2018
The CL to fix it is in code review https://chromium-review.googlesource.com/c/chromium/src/+/955685 mahmadi@ thanks for adding in loop about new dynamic form detection in crrev.com/c/943321
,
Mar 8 2018
Thank you for the quick fix!
,
Mar 8 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/af3972380eb097187f629bf0716141b73ae38b8d commit af3972380eb097187f629bf0716141b73ae38b8d Author: Vadym Doroshenko <dvadym@chromium.org> Date: Thu Mar 08 17:24:57 2018 [IOS Password Manager] Inform that no suggestions when no password forms. On CL https://chromium-review.googlesource.com/c/chromium/src/+/921961 async retrieval of suggestions in Password Manager was implemented. The problem that when there are no password forms PasswordController doesn't call callback that there are no suggestions so in result Autofill can't provide own suggestions. This CL fixes that. Bug: 820072 Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs Change-Id: I4a7dce1540b2217b388caadb19fc2e563c79eb26 Reviewed-on: https://chromium-review.googlesource.com/955685 Commit-Queue: Vadym Doroshenko <dvadym@chromium.org> Reviewed-by: Vasilii Sukhanov <vasilii@chromium.org> Cr-Commit-Position: refs/heads/master@{#541821} [modify] https://crrev.com/af3972380eb097187f629bf0716141b73ae38b8d/ios/chrome/browser/passwords/password_controller.mm [modify] https://crrev.com/af3972380eb097187f629bf0716141b73ae38b8d/ios/chrome/browser/passwords/password_controller_unittest.mm
,
Mar 8 2018
,
Mar 12 2018
,
Mar 13 2018
Your change meets the bar and is auto-approved for M66. Please go ahead and merge the CL to branch 3359 manually. Please contact milestone owner if you have questions. Owners: cmasso@(Android), cmasso@(iOS), josafat@(ChromeOS), abdulsyed@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Mar 15 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/84cf416270891fd9d4a74fbce7f7736274413455 commit 84cf416270891fd9d4a74fbce7f7736274413455 Author: Vadym Doroshenko <dvadym@chromium.org> Date: Thu Mar 15 17:50:51 2018 [IOS Password Manager] Inform that no suggestions when no password forms. On CL https://chromium-review.googlesource.com/c/chromium/src/+/921961 async retrieval of suggestions in Password Manager was implemented. The problem that when there are no password forms PasswordController doesn't call callback that there are no suggestions so in result Autofill can't provide own suggestions. This CL fixes that. TBR=dvadym@chromium.org (cherry picked from commit af3972380eb097187f629bf0716141b73ae38b8d) Bug: 820072 Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs Change-Id: I4a7dce1540b2217b388caadb19fc2e563c79eb26 Reviewed-on: https://chromium-review.googlesource.com/955685 Commit-Queue: Vadym Doroshenko <dvadym@chromium.org> Reviewed-by: Vasilii Sukhanov <vasilii@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#541821} Reviewed-on: https://chromium-review.googlesource.com/964322 Reviewed-by: Vadym Doroshenko <dvadym@chromium.org> Cr-Commit-Position: refs/branch-heads/3359@{#272} Cr-Branched-From: 66afc5e5d10127546cc4b98b9117aff588b5e66b-refs/heads/master@{#540276} [modify] https://crrev.com/84cf416270891fd9d4a74fbce7f7736274413455/ios/chrome/browser/passwords/password_controller.mm [modify] https://crrev.com/84cf416270891fd9d4a74fbce7f7736274413455/ios/chrome/browser/passwords/password_controller_unittest.mm
,
Mar 19 2018
Verified on chrome canary version 67.0.3375.0 on iPhone 8 with iOS 11.2.6 and iPad pro with iOS 11.2.6 following steps mentioned in comment #1. Address form autofill. Looks good.
,
Mar 19 2018
,
Mar 23 2018
Verified on M66.0.3359.52 beta. New address form is autofilled correctly on amazon.com Tested on iPhone8, iOS11.2.6 |
||||||||
►
Sign in to add a comment |
||||||||
Comment 1 by mahmadi@chromium.org
, Mar 8 2018