Closed Bug 1382635 Opened 2 years ago Closed 2 years ago

FormDataListener shouldn't listen for change events

Categories

(Firefox :: Session Restore, enhancement)

enhancement
Not set

Tracking

()

RESOLVED FIXED
Firefox 56
Tracking Status
firefox56 --- fixed

People

(Reporter: ttaubert, Assigned: ttaubert)

References

Details

Attachments

(1 file)

As already mentioned by Ehsan in bug 1373672 comment #0, the FormDataListener listens for both change and input events, based on a decision made long before we imported the add-on code into Firefox.

AFAICT whenever the user interacts with a form element we dispatch the "input" event properly, I couldn't find a counterexample. In the past (Firefox 2 and before) we might have had inconsistent behavior.
Assignee: nobody → ttaubert
Status: NEW → ASSIGNED
I removed browser_form_restore_events_sample.html completely because that actually doesn't really seem to test anything. We send fake events from the content script and those are of course exactly the ones we expect.
Attachment #8888268 - Flags: review?(mdeboer)
Blocks: 1373672
Comment on attachment 8888268 [details] [diff] [review]
0002-Bug-1382635-FormDataListener-shouldn-t-listen-for-ch.patch

Review of attachment 8888268 [details] [diff] [review]:
-----------------------------------------------------------------

I'm intrigued...very curious to see what the impact will be on various benchmarks, because I think the 'change' event is mostly fired when 'tabbing' out of a form control, which will now no longer fire a collect() and therefore reduce possible jank in the most important phase: form field navigation.
Thanks for this change!
Attachment #8888268 - Flags: review?(mdeboer) → review+
Pushed by ttaubert@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/d1340f39e016
FormDataListener shouldn't listen for change events r=mikedeboer
https://hg.mozilla.org/mozilla-central/rev/d1340f39e016
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 56
You need to log in before you can comment on or make changes to this bug.