Closed Bug 1348193 Opened 7 years ago Closed 7 years ago

[Form Autofill] Listen for form submissions in autofill code

Categories

(Toolkit :: Form Manager, enhancement)

enhancement
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla55
Tracking Status
firefox55 --- fixed

People

(Reporter: steveck, Assigned: steveck)

References

Details

(Whiteboard: [form autofill:M2])

Attachments

(1 file)

Since formlike component we created might not based on real form element, listening the submit event will not work for non-form element. Hi MattN, is this earlysubmit observer[1] the right part that we should reference to? 

[1] https://dxr.mozilla.org/mozilla-central/rev/ff04d410e74b69acfab17ef7e73e7397602d5a68/toolkit/components/passwordmgr/LoginManagerContent.jsm#46-146
Flags: needinfo?(MattN+bmo)
Assignee: nobody → schung
Flags: needinfo?(MattN+bmo)
Whiteboard: [form autofill:M2]
Comment on attachment 8853346 [details]
Bug 1348193 - Handle submit action with earlyformsubmit.

https://reviewboard.mozilla.org/r/125456/#review132478

::: browser/extensions/formautofill/test/unit/test_onFormSubmitted.js:22
(Diff revision 2)
> +
> +  let form = MOCK_DOC.getElementById("form1");
> +  FormAutofillContent.identifyAutofillFields(MOCK_DOC);
> +  sinon.spy(FormAutofillContent, "_onFormSubmit");
> +
> +  do_check_eq(FormAutofillContent.notify(form), true);

I tried to invoke a submit event and verify *_onFormSubit* is called while receiving the event, but it seems unable to get the submit event no matter calling form.submit() or clicking the submit button. Not sure if there's any limitation in the xpcshell test...
Comment on attachment 8853346 [details]
Bug 1348193 - Handle submit action with earlyformsubmit.

https://reviewboard.mozilla.org/r/125456/#review132718

(In reply to Steve Chung [:steveck] from comment #0)
> Since formlike component we created might not based on real form element,
> listening the submit event will not work for non-form element. 

earlyformsubmit isn't for non-form formlikes either but doesn't require holding form references and allows us to retrieve field values before the page modifies them on a submit event which can lead to better results in some cases. To handle all form/non-form "submissions" I think ideally we should move the submission logic from LoginManagerContent.jsm to FormLike.jsm but we can do that later.

::: browser/extensions/formautofill/FormAutofillContent.jsm:282
(Diff revision 2)
>      this.savedFieldNames =
>        Services.cpmm.initialProcessData.autofillSavedFieldNames || new Set();
>    },
>  
> +  _onFormSubmit(handler) {
> +    // TODO: Handle form submit event for profile saving and metrics

Can you add the bug number (990219) here? I've been letting things go recently with the intention of catching up at the end of M1 but generally we should have bug numbers for any TODO comments.
Attachment #8853346 - Flags: review?(MattN+bmo) → review+
Status: NEW → ASSIGNED
Summary: [Form Autofill] Handle submit action for the formLike component that is not based on form element → [Form Autofill] Listen for form submissions in autofill code
Thanks!
Keywords: checkin-needed
Pushed by ihsiao@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/a8ccc3f53976
Handle submit action with earlyformsubmit. r=MattN
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/a8ccc3f53976
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
Blocks: 1360079
Flags: qe-verify-
You need to log in before you can comment on or make changes to this bug.