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

RESOLVED FIXED in Firefox 59

Status

()

Toolkit
Form Autofill
P3
normal
RESOLVED FIXED
2 months ago
2 months ago

People

(Reporter: ralin, Assigned: ralin)

Tracking

(Blocks: 1 bug)

unspecified
mozilla59
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox59 fixed)

Details

(Whiteboard: [form autofill:V2])

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment)

(Assignee)

Description

2 months ago
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 hidden (mozreview-request)

Comment 2

2 months ago
mozreview-review
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

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

https://reviewboard.mozilla.org/r/202680/#review208388
Attachment #8931552 - Flags: review?(lchang)
Comment hidden (mozreview-request)
(Assignee)

Comment 5

2 months ago
mozreview-review-reply
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

2 months ago
mozreview-review
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.
Attachment #8931552 - Flags: review?(lchang) → review+

Comment 7

2 months ago
Pushed by ralin@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/2e174bb5580c
Move the ownership of input handler from form to section. r=lchang

Comment 8

2 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/2e174bb5580c
Status: ASSIGNED → RESOLVED
Last Resolved: 2 months ago
status-firefox59: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla59

Updated

2 months ago
Blocks: 1411877
You need to log in before you can comment on or make changes to this bug.