[Form Autofill] Listen for form submissions in autofill code

RESOLVED FIXED in Firefox 55

Status

()

Toolkit
Form Manager
RESOLVED FIXED
a month ago
12 days ago

People

(Reporter: steveck, Assigned: steveck)

Tracking

(Blocks: 1 bug)

unspecified
mozilla55
Points:
---

Firefox Tracking Flags

(firefox55 fixed)

Details

(Whiteboard: [form autofill:M2])

MozReview Requests

()

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

Attachments

(1 attachment)

(Assignee)

Description

a month ago
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
(Assignee)

Updated

a month ago
Flags: needinfo?(MattN+bmo)
Comment hidden (mozreview-request)
(Assignee)

Updated

27 days ago
Assignee: nobody → schung
Flags: needinfo?(MattN+bmo)

Updated

22 days ago
Whiteboard: [form autofill:M2]
Comment hidden (mozreview-request)
(Assignee)

Comment 3

14 days ago
mozreview-review
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
Comment hidden (mozreview-request)
(Assignee)

Comment 6

13 days ago
Thanks!
Keywords: checkin-needed

Comment 7

13 days ago
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
Last Resolved: 12 days ago
status-firefox55: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
You need to log in before you can comment on or make changes to this bug.