New issue
Advanced search Search tips

Issue 855696 link

Starred by 2 users

Issue metadata

Status: WontFix
Owner:
Closed: Jul 13
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: iOS
Pri: 2
Type: Bug



Sign in to add a comment

Regression: previously hidden fields may not get filled

Project Member Reported by mahmadi@chromium.org, Jun 22 2018

Issue description

This is a regression caused by crrev.com/c/1012979. Previously, AutofillAgent would scan the whole page on form activities due to the possibility of Autofill having been enabled after page load. crrev.com/c/1012979 replaced that logic in favor of observing the Pref directly. This has a side effect where previously hidden fields can go unnoticed by AutofillManager. This CL fixes that regression by scanning the page when a field is focused. All fields receive focus before their value is set by Autofill, therefore, AutofillManager can get notified of previously hidden fields.

The billing address form in centresdirect.co.uk is an example. However, Autofill is broken on centresdirect.co.uk due to  crbug.com/855681 
 
Project Member

Comment 1 by bugdroid1@chromium.org, Jun 27 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/77318515062e22c6dbcb5b4ae8c6daa4ef70d5b9

commit 77318515062e22c6dbcb5b4ae8c6daa4ef70d5b9
Author: Moe Ahmadi <mahmadi@chromium.org>
Date: Wed Jun 27 09:43:02 2018

[AF][IOS] Scan page for form changes when a field is focused

This CL fixes a regression caused by crrev.com/c/1012979. Previously,
AutofillAgent would scan the whole page on form activities due to the
possiblity of Autofill having been enabled after page load.
crrev.com/c/1012979 replaced that logic in favor of observing the Pref
directly. This has a side effect where previously hidden fields can go
unnoticed by AutofillManager. This CL fixes that regression by scanning the
page when a field is focused. All fields receive focuse before their value
is set by Autofill, therefore, AutofillManager can get notified of
previously hidden fields.

Bug:  855696 
Change-Id: I175bf90e12d1ca356b746e6347943a295063c288
Reviewed-on: https://chromium-review.googlesource.com/1112493
Reviewed-by: Olivier Robin <olivierrobin@chromium.org>
Commit-Queue: Olivier Robin <olivierrobin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#570713}
[modify] https://crrev.com/77318515062e22c6dbcb5b4ae8c6daa4ef70d5b9/components/autofill/ios/browser/autofill_agent.mm

Project Member

Comment 2 by bugdroid1@chromium.org, Jul 12

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/4b937d53836386e51532fbe870938b33ce0455ed

commit 4b937d53836386e51532fbe870938b33ce0455ed
Author: Moe Ahmadi <mahmadi@chromium.org>
Date: Thu Jul 12 20:13:40 2018

Revert "[AF][IOS] Scan page for form changes when a field is focused"

This reverts commit 77318515062e22c6dbcb5b4ae8c6daa4ef70d5b9.

Reason for revert:
This change caused a regression (crbug.com/860225). The cause of the regression is that when user focuses a field in order to edit it, Autofill::OnFormsSeen is called which in turn updates the cached form with the new user entered values. Once submitted, form values are discarded as they're thought to be the initial form values on page load. In its current implementation Autofill::OnFormsSeen is only intended to be called on page load.

Original change's description:
> [AF][IOS] Scan page for form changes when a field is focused
>
> This CL fixes a regression caused by crrev.com/c/1012979. Previously,
> AutofillAgent would scan the whole page on form activities due to the
> possiblity of Autofill having been enabled after page load.
> crrev.com/c/1012979 replaced that logic in favor of observing the Pref
> directly. This has a side effect where previously hidden fields can go
> unnoticed by AutofillManager. This CL fixes that regression by scanning the
> page when a field is focused. All fields receive focuse before their value
> is set by Autofill, therefore, AutofillManager can get notified of
> previously hidden fields.
>
> Bug:  855696 
> Change-Id: I175bf90e12d1ca356b746e6347943a295063c288
> Reviewed-on: https://chromium-review.googlesource.com/1112493
> Reviewed-by: Olivier Robin <olivierrobin@chromium.org>
> Commit-Queue: Olivier Robin <olivierrobin@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#570713}

TBR=olivierrobin@chromium.org,mahmadi@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug:  855696 , 860225
Change-Id: Icc39e67e6d502d75451dbec83d6271255639ee4c
Reviewed-on: https://chromium-review.googlesource.com/1135671
Commit-Queue: Moe Ahmadi <mahmadi@chromium.org>
Reviewed-by: Moe Ahmadi <mahmadi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#574698}
[modify] https://crrev.com/4b937d53836386e51532fbe870938b33ce0455ed/components/autofill/ios/browser/autofill_agent.mm

Labels: -Pri-1 -M-68 M-69 Pri-2
Status: Assigned (was: Started)
Labels: OS-iOS
Status: WontFix (was: Assigned)
Turns out this wasn't a regression and really caused by  crbug.com/855681 . As elaborated in crrev.com/c/1012979, when autofill_agent is queried for suggestions, it fetches the relevant form and informs the autofill_manager to update the cached form. Therefore, the previously hidden element correctly gets updated in the cache and gets Autofilled.

Sign in to add a comment