The default bug view has changed. See this FAQ.

datetimepopup.xml should not be initialized during browser startup

RESOLVED FIXED in Firefox 55



24 days ago
10 days ago


(Reporter: gandalf, Assigned: scottwu)



Firefox Tracking Flags

(firefox55 fixed)


MozReview Requests


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


(1 attachment)



24 days ago
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.

Comment 1

24 days ago
:smaug, :scottwu, you guys should know more if I'm right.
Flags: needinfo?(scwwu)
Flags: needinfo?(bugs)

Comment 2

23 days ago
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?
Flags: needinfo?(scwwu)
Comment hidden (mozreview-request)

Comment 4

16 days ago
:scottwu, is it ready for review?
Flags: needinfo?(scwwu)

Comment 5

13 days ago
I think this is the solution I can come up with now. Let me ask :mconley if he could take a look.
Assignee: nobody → scwwu
Flags: needinfo?(scwwu)


13 days ago
Attachment #8843192 - Flags: review?(mconley)
Comment on attachment 8843192 [details]
Bug 1343707 - Make datetimepopup.xml binding attach lazily.

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[";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.
Attachment #8843192 - Flags: review?(mconley) → review+
Comment hidden (mozreview-request)

Comment 8

10 days ago
Thanks for the suggestions Mike! I've modified the commit message and changed the event name to "DateTimePickerBindingReady".
Flags: needinfo?(bugs)
Keywords: checkin-needed

Comment 9

10 days ago
Pushed by
Make datetimepopup.xml binding attach lazily. r=mconley
Keywords: checkin-needed

Comment 10

10 days ago
Last Resolved: 10 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.