iOS full-form Autofill: Handle partially filled forms correctly.

iOS full-form Autofill: Handle partially filled forms correctly.

Previous to this change, all fields would be overwritten regardless of
their contents and whether or not they had already been autofilled. With
this change the suggestion and overwrite behavior is the same as on
desktop and Android.

1. If any form fields are already autofilled (i.e. have yellow
background) then suggestions are only given for the currently active
field, and only the currently active field is modified when a suggestion
is chosen. This is achieved by setting |isAutofilled| in
autofill_controller.js. The common Autofill code shared with desktop
takes care of it from there.

2. If a field is non-empty do not autofill it, unless it is the
currently active field.

Downstream part is here: https://chromereviews.googleplex.com/222487013

BUG=504644
Committed: https://crrev.com/2c42d2c734bc02e8af8f80b5f5f3d9c39f7e8b62
Cr-Commit-Position: refs/heads/master@{#339677}

[bondd, 2015-07-17 22:52:59]

bondd@chromium.org changed reviewers:
+ jdonnelly@chromium.org

[bondd, 2015-07-17 22:53:00]

bondd@chromium.org changed required reviewers:
+ jdonnelly@chromium.org

[Justin Donnelly, 2015-07-20 17:21:41]

https://codereview.chromium.org/1234193005/diff/1/components/autofill/ios/bro...
File components/autofill/ios/browser/resources/autofill_controller.js (right):

https://codereview.chromium.org/1234193005/diff/1/components/autofill/ios/bro...
components/autofill/ios/browser/resources/autofill_controller.js:506: * @param
{string} forceFillFieldName Named field will always be filled,
Is it possible to use document.activeElement rather than having to pass this in?

https://codereview.chromium.org/1234193005/diff/1/components/autofill/ios/bro...
components/autofill/ios/browser/resources/autofill_controller.js:540: //
'select-one' elements are autofilled even if non-empty. See
This comment looks like it's meant to describe the entire if condition. Can you
expand it to be something like. "If directed, skip empty fields unless this is
the forceFillName or it's a 'select-one element See [...]."

[bondd, 2015-07-20 23:55:29]

https://codereview.chromium.org/1234193005/diff/1/components/autofill/ios/bro...
File components/autofill/ios/browser/resources/autofill_controller.js (right):

https://codereview.chromium.org/1234193005/diff/1/components/autofill/ios/bro...
components/autofill/ios/browser/resources/autofill_controller.js:506: * @param
{string} forceFillFieldName Named field will always be filled,
On 2015/07/20 17:21:41, Justin Donnelly wrote:
> Is it possible to use document.activeElement rather than having to pass this
in?

fillActiveFormField() (the function immediately above this one) does use
document.activeElement for single-field Autofill, so it is possible. However, it
needs to do some extra logic involving lastActiveElement and nameForAutofill()
to make sure it has the right field, and it passes in the field name anyway to
check that it matches. I believe this is because there are several async calls
with callbacks involved in the iOS Autofill flow, so it's not 100% guaranteed
that activeElement won't change at an unfortunate time. It seems simpler and
more reliable to pass in the known good forceFillFieldName rather than trying to
involve the possibly flaky document.activeElement.

https://codereview.chromium.org/1234193005/diff/1/components/autofill/ios/bro...
components/autofill/ios/browser/resources/autofill_controller.js:540: //
'select-one' elements are autofilled even if non-empty. See
On 2015/07/20 17:21:41, Justin Donnelly wrote:
> This comment looks like it's meant to describe the entire if condition. Can
you
> expand it to be something like. "If directed, skip empty fields unless this is
> the forceFillName or it's a 'select-one element See [...]."

Done.

[Justin Donnelly, 2015-07-21 17:07:00]

lgtm

[bondd, 2015-07-21 17:08:08]

The CQ bit was checked by bondd@chromium.org

[commit-bot: I haz the power, 2015-07-21 17:08:58]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1234193005/20001

[commit-bot: I haz the power, 2015-07-21 17:18:10]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-07-21 17:19:30]

Patchset 2 (id:??) landed as
https://crrev.com/2c42d2c734bc02e8af8f80b5f5f3d9c39f7e8b62
Cr-Commit-Position: refs/heads/master@{#339677}