1350264 - [Form Autofill] Allow users to re-enable form autofill feature after all populated fields were cleared manually

Status: RESOLVED FIXED

(Toolkit :: Form Manager, enhancement, P3)

Description

[Ray Lin[:ralin]]

We should re-activate profile data if user cleared all populated fields manually.

Comment 1

[Steve Chung [:steveck]]

After some discussion with Ray, we came up with another idea that we could simply clear the guid at the startSearch, if the all the fields in the focused form are cleared. So we don't have to listen for input changes right now, and even the preview state will need to add the event listener, it'll just need to take care the focused input instead of check/update the form state.

Comment 2

[Steve Chung [:steveck]]

Attachment: Bug 1350264 - Allow users to re-enable form autofill feature after all populated fields were cleared manually,

Review commit: https://reviewboard.mozilla.org/r/124204/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/124204/

Comment 3

[Steve Chung [:steveck]]

Comment on attachment 8851956 [details]
Bug 1350264 - Allow users to re-enable form autofill feature after all populated fields were cleared manually,

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/124204/diff/1-2/

Comment 4

[Luke Chang [:lchang] (inactive)]

Comment on attachment 8851956 [details]
Bug 1350264 - Allow users to re-enable form autofill feature after all populated fields were cleared manually,

https://reviewboard.mozilla.org/r/124204/#review127982

::: browser/extensions/formautofill/FormAutofillHandler.jsm:137
(Diff revision 2)
> +    if (!this.filledProfileGUID) {
> +      return;
> +    }
> +
> +    if (this.fieldDetails.every((fieldDetail) =>
> +      !fieldDetail.element || !fieldDetail.element.value)) {

I'm thinking another edge case. There are 5 fields that are able to be autofilled. A user fills in a profile via Form Autofill feature and then empties 4 of them. Should we popup the autofill suggestion rather than the history one when the user focuses the last field and starts to modify it (note that the last field isn't empty but there isn't any field highlighted at that moment)? If so, basing on whether every fields are empty to reset `filledProfileGUID` might not be sufficient.

In my imagination, the `filledProfileGUID` should be aligned with whether there is at least one field highlighted. Could you confirm that with UX?

Comment 5

[Matthew N. [:MattN]]

Comment on attachment 8851956 [details]
Bug 1350264 - Allow users to re-enable form autofill feature after all populated fields were cleared manually,

https://reviewboard.mozilla.org/r/124204/#review130106

::: browser/extensions/formautofill/FormAutofillHandler.jsm:137
(Diff revision 2)
> +    if (!this.filledProfileGUID) {
> +      return;
> +    }
> +
> +    if (this.fieldDetails.every((fieldDetail) =>
> +      !fieldDetail.element || !fieldDetail.element.value)) {

As we discussed in person I also suspect that it would will be very rare that the `every` will succeed because even if a user chooses an "undesired" profile, it's likely that at least the country or region field will not need "correcting". I think it may be okay to land an approach like you have but it's going to be added complexity for little benefit. To properly address this use case I think we need to implement the "Clear" autocomplete item and/or ignore certain fields or only consider certain fields for this logic e.g. exclude region+country.

Comment 6

[Steve Chung [:steveck]]

Comment on attachment 8851956 [details]
Bug 1350264 - Allow users to re-enable form autofill feature after all populated fields were cleared manually,

https://reviewboard.mozilla.org/r/124204/#review130106

> As we discussed in person I also suspect that it would will be very rare that the `every` will succeed because even if a user chooses an "undesired" profile, it's likely that at least the country or region field will not need "correcting". I think it may be okay to land an approach like you have but it's going to be added complexity for little benefit. To properly address this use case I think we need to implement the "Clear" autocomplete item and/or ignore certain fields or only consider certain fields for this logic e.g. exclude region+country.

I agree that it would be more practical if it could come with clear function, and it is true that clearing form manually is a rare case. I think it might be an acceptable alternative solution before clear feature landed, otherwise it's not possible to re-enable the profile autofill right now. I confirmed with Juwie that it could only be re-enabled after clearing all the fields(and I think country or region case is not a concern since MVP will not autofill select, which are mainly applied for country/region), but please let me know if you have better idea or we could wait for clear button.

Comment 7

[Luke Chang [:lchang] (inactive)]

(In reply to Steve Chung [:steveck] from comment #6)
> (and I think country or region case is not a concern since MVP will not autofill select, which are mainly applied for country/region)

Supporting <select> is actually in MVP scope.

After consulting Juwei, she said that we can simply ignore the status of <select>s and re-enable Form Autofill feature if other <input>s are empty. Though I still think we should take the highlight state into account -- not the emptiness.

Comment 8

[Steve Chung [:steveck]]

Comment on attachment 8851956 [details]
Bug 1350264 - Allow users to re-enable form autofill feature after all populated fields were cleared manually,

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/124204/diff/2-3/

Comment 9

[Luke Chang [:lchang] (inactive)]

Comment on attachment 8851956 [details]
Bug 1350264 - Allow users to re-enable form autofill feature after all populated fields were cleared manually,

https://reviewboard.mozilla.org/r/124204/#review153880

Looks good!

Comment 10

[Steve Chung [:steveck]]

Thanks!

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/78fc69bbf3bb
Allow users to re-enable form autofill feature after all populated fields were cleared manually, r=lchang

Comment 12

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/78fc69bbf3bb