1420169 - filledRecordGUID will be set to null while auto-filling if there's a section has been populated in the same form

Status: RESOLVED FIXED

(Toolkit :: Form Autofill, defect, P3)

Description

[Ray Lin[:ralin]]

By chance found this bug when implementing Bug 1415073. The onChangeHandler[0] would affect the later filled section as it could not distinguish the source of  the "input" events, between by user or by form auto-fill, then erases the filledRecordGUID.


[0] https://searchfox.org/mozilla-central/rev/33c90c196bc405e628bc868a4f4ba29b992478c0/browser/extensions/formautofill/FormAutofillHandler.jsm#936-948

Comment 1

[Ray Lin[:ralin]]

Attachment: Bug 1420169 - Move the ownership of input handler from form to section.

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

Comment 2

[Luke Chang [:lchang] (inactive)]

Comment on attachment 8931552 [details]
Bug 1420169 - Move the ownership of input handler from form to section.

https://reviewboard.mozilla.org/r/202680/#review208384

::: browser/extensions/formautofill/FormAutofillHandler.jsm:96
(Diff revision 1)
>      };
>  
>      this._validDetails = Array.of(...(this.address.fieldDetails),
>                                    ...(this.creditCard.fieldDetails));
> +
> +    this._onInputHandler = ({target, isTrusted}) => {

Can this be implemented in `handleEvent()`?

::: browser/extensions/formautofill/FormAutofillHandler.jsm:937
(Diff revision 1)
>     * @param {HTMLElement} focusedInput
>     *        A focused input element needed to determine the address or credit
>     *        card field.
>     */
>    async autofillFormFields(profile, focusedInput) {
>      let noFilledSections = !this.hasFilledSection();

nit: (unrelated to this patch) Can we rename it to something like `noFilledSectionsPreviously`?

Comment 3

[Luke Chang [:lchang] (inactive)]

Comment on attachment 8931552 [details]
Bug 1420169 - Move the ownership of input handler from form to section.

https://reviewboard.mozilla.org/r/202680/#review208388

Comment 4

[Ray Lin[:ralin]]

Comment on attachment 8931552 [details]
Bug 1420169 - Move the ownership of input handler from form to section.

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

Comment 5

[Ray Lin[:ralin]]

Comment on attachment 8931552 [details]
Bug 1420169 - Move the ownership of input handler from form to section.

https://reviewboard.mozilla.org/r/202680/#review208384

Thanks for reviewing, the issues are fixed.

> Can this be implemented in `handleEvent()`?

moved to handleEvent

> nit: (unrelated to this patch) Can we rename it to something like `noFilledSectionsPreviously`?

renamed

Comment 6

[Luke Chang [:lchang] (inactive)]

Comment on attachment 8931552 [details]
Bug 1420169 - Move the ownership of input handler from form to section.

https://reviewboard.mozilla.org/r/202680/#review208424

Thanks.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2e174bb5580c
Move the ownership of input handler from form to section. r=lchang

Comment 8

[Eliza Balazs [:ebalazs_]]

https://hg.mozilla.org/mozilla-central/rev/2e174bb5580c