Closed Bug 1350264 Opened 3 years ago Closed 2 years ago

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

Categories

(Toolkit :: Form Manager, enhancement, P3)

53 Branch
enhancement

Tracking

()

RESOLVED FIXED
mozilla56
Tracking Status
firefox56 --- fixed

People

(Reporter: ralin, Assigned: steveck)

References

(Blocks 1 open bug)

Details

(Whiteboard: [form autofill:M3])

User Story

As a Firefox user, I want to be able to populate profile data if the form is empty. Even if the form has been populated, I expect form autofill works as-is after I clear all filled fields manually.

Attachments

(1 file)

We should re-activate profile data if user cleared all populated fields manually.
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.
Attachment #8851956 - Flags: review?(lchang)
Assignee: nobody → schung
Status: NEW → ASSIGNED
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?
Attachment #8851956 - Flags: review?(lchang)
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.
Attachment #8851956 - Flags: review?(MattN+bmo)
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.
(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.
Whiteboard: [form autofill:MVP] → [form autofill:M3]
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!
Attachment #8851956 - Flags: review?(lchang) → review+
Thanks!
Keywords: checkin-needed
Pushed by lchang@mozilla.com:
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
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/78fc69bbf3bb
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
You need to log in before you can comment on or make changes to this bug.