Open
Bug 1137316
Opened 10 years ago
Updated 2 years ago
Lazify the PluralForm.jsm imports
Categories
(MailNews Core :: Internationalization, enhancement)
MailNews Core
Internationalization
Tracking
(Not tracked)
ASSIGNED
People
(Reporter: aryx, Assigned: aryx)
Details
Attachments
(5 files)
11.15 KB,
patch
|
Fallen
:
review+
|
Details | Diff | Splinter Review |
2.42 KB,
patch
|
aleth
:
review+
|
Details | Diff | Splinter Review |
3.48 KB,
patch
|
mkmelin
:
review+
neil
:
review+
|
Details | Diff | Splinter Review |
19.34 KB,
patch
|
mkmelin
:
review+
|
Details | Diff | Splinter Review |
5.36 KB,
patch
|
neil
:
review+
|
Details | Diff | Splinter Review |
Similar to bug 996448 in mozilla-central, this bug is about lazily loading PluralForm.jsm
Attachment #8569972 -
Flags: review?(philipp)
Assignee | ||
Comment 1•10 years ago
|
||
Assignee | ||
Comment 2•10 years ago
|
||
Attachment #8569975 -
Flags: review?(neil)
Attachment #8569975 -
Flags: review?(mkmelin+mozilla)
Assignee | ||
Comment 3•10 years ago
|
||
Attachment #8569977 -
Flags: review?(mkmelin+mozilla)
Assignee | ||
Comment 4•10 years ago
|
||
Attachment #8569978 -
Flags: review?(neil)
Updated•10 years ago
|
Attachment #8569977 -
Flags: review?(mkmelin+mozilla) → review+
Comment 5•10 years ago
|
||
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|
Attachment #8569972 -
Flags: review?(philipp) → review+
Updated•10 years ago
|
Attachment #8569975 -
Flags: review?(mkmelin+mozilla) → review+
Comment 6•10 years ago
|
||
Comment on attachment 8569978 [details] [diff] [review] suite, v1 Nit: missed one in navigator.js
Attachment #8569978 -
Flags: review?(neil) → review+
Updated•10 years ago
|
Attachment #8569975 -
Flags: review?(neil) → review+
Comment 7•10 years ago
|
||
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?
Assignee | ||
Comment 8•10 years ago
|
||
(In reply to neil@parkwaycc.co.uk 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•10 years ago
|
||
(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.
Updated•10 years ago
|
Attachment #8569974 -
Flags: review?(aleth) → review+
Updated•2 years ago
|
Severity: normal → S3
You need to log in
before you can comment on or make changes to this bug.
Description
•