1137316 - Lazify the PluralForm.jsm imports

Status: ASSIGNED

(MailNews Core :: Internationalization, enhancement)

Description

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Attachment: calendar, v1

Similar to bug 996448 in mozilla-central, this bug is about lazily loading PluralForm.jsm

Comment 1

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Attachment: chat/im, v1

Comment 2

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Attachment: mailnews, v1

Comment 3

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Attachment: mail, v1

Comment 4

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Attachment: suite, v1

Comment 5

[Philipp Kewisch [:Fallen][📱📆]]

Comment on attachment 8569972 [details] [diff] [review]
calendar, v1

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

r=philipp with this comment considered:

::: calendar/base/content/calendar-task-tree.xml
@@ +117,5 @@
>          Components.utils.import("resource://gre/modules/Services.jsm");
>          Components.utils.import("resource://calendar/modules/calItemUtils.jsm");
>          Components.utils.import("resource://gre/modules/XPCOMUtils.jsm");
> +        XPCOMUtils.defineLazyModuleGetter(this, "PluralForm",
> +                                          "resource://gre/modules/PluralForm.jsm");

passing |this| here will define the getter on the binding instance. I think you will have to use Components.getGlobalForObject(this) instead, or maybe just pass |window|

Comment 6

[[e-mail]]

Comment on attachment 8569978 [details] [diff] [review]
suite, v1

Nit: missed one in navigator.js

Comment 7

[aleth [:aleth]]

Comment on attachment 8569974 [details] [diff] [review]
chat/im, v1

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

::: mail/components/im/modules/chatNotifications.jsm
@@ +11,2 @@
>  Cu.import("resource://gre/modules/Timer.jsm");
> +Cu.import("resource://gre/modules/XPCOMUtils.jsm");

I don't think you need this import, as imServices already imports it?

Comment 8

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

(In reply to [e-mail] from comment #6)
> Nit: missed one in navigator.js
That's part of
> __defineGetter__("PluralForm", function() {
>   Components.utils.import("resource://gre/modules/PluralForm.jsm");
>   return this.PluralForm;
> });
and only used at https://dxr.mozilla.org/comm-central/source/suite/common/bookmarks/browser-places.js#176

Doesn't this get only loaded when it's called?

(In reply to aleth [:aleth] from comment #7)
> ::: mail/components/im/modules/chatNotifications.jsm
> @@ +11,2 @@
> >  Cu.import("resource://gre/modules/Timer.jsm");
> > +Cu.import("resource://gre/modules/XPCOMUtils.jsm");
> 
> I don't think you need this import, as imServices already imports it?
But it doesn't export it (it is itself a module). Or did you mean im/content/extensions.js?

Comment 9

[aleth [:aleth]]

(In reply to Archaeopteryx [:aryx] from comment #8)
> (In reply to aleth [:aleth] from comment #7)
> > ::: mail/components/im/modules/chatNotifications.jsm
> > @@ +11,2 @@
> > >  Cu.import("resource://gre/modules/Timer.jsm");
> > > +Cu.import("resource://gre/modules/XPCOMUtils.jsm");
> > 
> > I don't think you need this import, as imServices already imports it?
> But it doesn't export it (it is itself a module). Or did you mean
> im/content/extensions.js?

Ah, you're right.