[Form Autofill] Listen for form submissions in autofill code

RESOLVED FIXED in Firefox 55

Status

()

Toolkit
Form Manager
RESOLVED FIXED
3 months ago
9 hours ago

People

(Reporter: steveck, Assigned: steveck)

Tracking

unspecified
mozilla55
Points:
---
Dependency tree / graph
Bug Flags:
qe-verify -

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

3 months 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

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

Updated

3 months ago
Assignee: nobody → schung
Flags: needinfo?(MattN+bmo)

Updated

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

Comment 3

3 months 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

3 months ago
Thanks!
Keywords: checkin-needed

Comment 7

3 months 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: 3 months ago
status-firefox55: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
(Assignee)

Updated

2 months ago
Blocks: 1360079

Updated

9 hours ago
Flags: qe-verify-
You need to log in before you can comment on or make changes to this bug.