New issue
Advanced search Search tips

Issue 863954 link

Starred by 1 user

Issue metadata

Status: Assigned
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , iOS , Chrome , Mac
Pri: 2
Type: Bug



Sign in to add a comment

Autofill data not saved after submitting Dynamic forms

Project Member Reported by mahmadi@chromium.org, Jul 16

Issue description

Chrome Version: 69.0.3493.0
OS: all

Prerequisites:
(2) Remove all existing autofill profiles.
(1) Turn on chrome://flags/#autofill-dynamic-forms

What steps will reproduce the problem?
(1) Go to https://sebsg.github.io/autofill/autofill-smoke.html
(2) Fill the form with "president" and submit.
(3) Go to https://sebsg.github.io/autofill/dynamic_form_select_options_change.html
(4) Autofill the form, fill the "Company" field and submit.

What is the expected result?
A new autofill profile should get added after form submission.

What happens instead?
No new autofill profile is added after form submission.



Please use labels and text to provide additional information.

If this is a regression (i.e., worked before), please consider using the
bisect tool (https://www.chromium.org/developers/bisect-builds-py) to help
us identify the root cause and more rapidly triage the issue.

For graphics-related bugs, please copy/paste the contents of the about:gpu
page at the end of this report.


 
Project Member

Comment 1 by bugdroid1@chromium.org, Jul 20

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

commit e93874bea4b938d260701e54c50281854a1cc10a
Author: Moe Ahmadi <mahmadi@chromium.org>
Date: Fri Jul 20 23:33:17 2018

[AF] Do not discard form data after dynamic form submission

Prior to this change form data were being discarded after a dynamic form
submission. This was because the dynamically changed form was being added
to the cache with its user entered/autofilled values. At the time of
submission, form data were being discarded as they were thought to be the
initial values of the fields at page load. This CL changes
AutofillHandler::OnFormsSeen (which is called on dynamic form changes) to
attempt to find a previously cached version of the form first. Prior to
addition to the cache, form values are overridden by the cached values.
This prevents those values from getting confused with the initial form
values at the time of submission.

Bug: 863954

Change-Id: Iba4771ee87b9777e66a6b10b6b369379dc123a46
Reviewed-on: https://chromium-review.googlesource.com/1137021
Commit-Queue: Moe Ahmadi <mahmadi@chromium.org>
Reviewed-by: Sebastien Seguin-Gagnon <sebsg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#577033}
[modify] https://crrev.com/e93874bea4b938d260701e54c50281854a1cc10a/chrome/browser/autofill/autofill_interactive_uitest.cc
[modify] https://crrev.com/e93874bea4b938d260701e54c50281854a1cc10a/chrome/browser/autofill/autofill_uitest_util.cc
[modify] https://crrev.com/e93874bea4b938d260701e54c50281854a1cc10a/chrome/browser/autofill/autofill_uitest_util.h
[add] https://crrev.com/e93874bea4b938d260701e54c50281854a1cc10a/chrome/test/data/autofill/dynamic_form_new_field.html
[modify] https://crrev.com/e93874bea4b938d260701e54c50281854a1cc10a/components/autofill/core/browser/autofill_handler.cc
[modify] https://crrev.com/e93874bea4b938d260701e54c50281854a1cc10a/components/autofill/core/browser/autofill_manager.cc
[modify] https://crrev.com/e93874bea4b938d260701e54c50281854a1cc10a/components/autofill/core/browser/form_structure.cc
[modify] https://crrev.com/e93874bea4b938d260701e54c50281854a1cc10a/components/autofill/core/browser/form_structure.h

Status: Fixed (was: Started)
Status: Assigned (was: Fixed)
The fix has a problem. When a dynamic form adds fields after certain interaction with users, the order of the fields are messed up afterwards. 
Example: staple shipping address. Autofill a form, then press continue, three fields are added. The server prediction for them are the predictions for the fields that were in their order before the interaction (except for the state that's a select field).
Cc: parastoog@chromium.org
Project Member

Comment 5 by bugdroid1@chromium.org, Aug 1

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

commit 0b6da642b299de1dff1324f665851d037c84f243
Author: Moe Ahmadi <mahmadi@chromium.org>
Date: Wed Aug 01 18:02:32 2018

Revert "[AF] Do not discard form data after dynamic form submission"

This reverts commit e93874bea4b938d260701e54c50281854a1cc10a.

Reason for revert: http://crbug.com/863954#c3

Original change's description:
> [AF] Do not discard form data after dynamic form submission
> 
> Prior to this change form data were being discarded after a dynamic form
> submission. This was because the dynamically changed form was being added
> to the cache with its user entered/autofilled values. At the time of
> submission, form data were being discarded as they were thought to be the
> initial values of the fields at page load. This CL changes
> AutofillHandler::OnFormsSeen (which is called on dynamic form changes) to
> attempt to find a previously cached version of the form first. Prior to
> addition to the cache, form values are overridden by the cached values.
> This prevents those values from getting confused with the initial form
> values at the time of submission.
> 
> Bug: 863954
> 
> Change-Id: Iba4771ee87b9777e66a6b10b6b369379dc123a46
> Reviewed-on: https://chromium-review.googlesource.com/1137021
> Commit-Queue: Moe Ahmadi <mahmadi@chromium.org>
> Reviewed-by: Sebastien Seguin-Gagnon <sebsg@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#577033}

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

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

Bug: 863954
Change-Id: I71d0970d10fe1d4857e98ba558b9253b13ad053d
Reviewed-on: https://chromium-review.googlesource.com/1151911
Reviewed-by: Moe Ahmadi <mahmadi@chromium.org>
Reviewed-by: Sebastien Seguin-Gagnon <sebsg@chromium.org>
Commit-Queue: Moe Ahmadi <mahmadi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#579858}
[modify] https://crrev.com/0b6da642b299de1dff1324f665851d037c84f243/chrome/browser/autofill/autofill_interactive_uitest.cc
[modify] https://crrev.com/0b6da642b299de1dff1324f665851d037c84f243/chrome/browser/autofill/autofill_uitest_util.cc
[modify] https://crrev.com/0b6da642b299de1dff1324f665851d037c84f243/chrome/browser/autofill/autofill_uitest_util.h
[delete] https://crrev.com/4a1c70a7e3d1952a5864022cb944f3219551e26a/chrome/test/data/autofill/dynamic_form_new_field.html
[modify] https://crrev.com/0b6da642b299de1dff1324f665851d037c84f243/components/autofill/core/browser/autofill_handler.cc
[modify] https://crrev.com/0b6da642b299de1dff1324f665851d037c84f243/components/autofill/core/browser/autofill_manager.cc
[modify] https://crrev.com/0b6da642b299de1dff1324f665851d037c84f243/components/autofill/core/browser/form_structure.cc
[modify] https://crrev.com/0b6da642b299de1dff1324f665851d037c84f243/components/autofill/core/browser/form_structure.h

Sign in to add a comment