|Submitter||Diff||Changes||Open Issues||Last Updated|
|Error loading review requests:|
59 bytes, text/x-review-board-request
|Details | Review|
While working on bug 1339892, I noticed that datetimepopup.xml is being executed during browser startup. When I changed the API of mozIntl it shows an error about mozIntl API not working. It seems to me that since there's no datetimepopup in the main UI, that's a missed opportunity to lazy initialize it on the first call, instead of at the browser startup.
:smaug, :scottwu, you guys should know more if I'm right.
You are right that currently the datetimepopup binding is attached during browser startup. Thanks for bringing this to my attention because I didn't really think about the performance implication until now. I could change it so that the binding is not attached until it is first used. Though I wonder if there's a standard approach?
:scottwu, is it ready for review?
I think this is the solution I can come up with now. Let me ask :mconley if he could take a look.
Comment on attachment 8843192 [details] Bug 1343707 - Make datetimepopup.xml binding attach lazily. https://reviewboard.mozilla.org/r/116970/#review121528 This might even give us a little bit of a win on the tpaint and ts_paint talos tests. Thanks! ::: commit-message-e91de:1 (Diff revision 1) > +Bug 1343707 - datetimepopup.xml binding is delayed until needed We should probably make this commit message clearer. Specifically, we should say what we're changing, perhaps like: "Bug 1343707 - Make datetimepopup.xml binding attach lazily. r?mconley" ::: toolkit/content/widgets/datetimepopup.xml:32 (Diff revision 1) > const mozIntl = Components.classes["@mozilla.org/mozintl;1"] > .getService(Components.interfaces.mozIMozIntl); > mozIntl.addGetCalendarInfo(this.l10n); > mozIntl.addGetDisplayNames(this.l10n); > + // Notify DateTimePickerHelper.jsm that binding is done. > + this.dispatchEvent(new CustomEvent("DateTimePickerBindingDone")); Nit: I wonder if "DateTimePickerBindingReady" or "DateTimePickerBindingAttached" is a more accurate event name. "Done" is a little ambiguous, imo.
Thanks for the suggestions Mike! I've modified the commit message and changed the event name to "DateTimePickerBindingReady".
Pushed by email@example.com: https://hg.mozilla.org/integration/autoland/rev/f0816e8fc07a Make datetimepopup.xml binding attach lazily. r=mconley