Closed
Bug 758979
Opened 12 years ago
Closed 12 years ago
convert mailnews/base/src/*.js to Services.jsm and MailServices.js
Categories
(MailNews Core :: Backend, defect)
MailNews Core
Backend
Tracking
(Not tracked)
RESOLVED
FIXED
Thunderbird 16.0
People
(Reporter: aceman, Assigned: aceman)
References
Details
Attachments
(1 file, 1 obsolete file)
(deleted),
patch
|
mconley
:
review+
iannbugzilla
:
review+
|
Details | Diff | Splinter Review |
msgAsyncPrompter.js: this._threadManager = Cc["@mozilla.org/thread-manager;1"]
nsMailNewsCommandLineHandler.js: let ioService = Cc["@mozilla.org/network/io-service;1"]
nsMailNewsCommandLineHandler.js: let windowMediator = Cc["@mozilla.org/appshell/window-mediator;1"]
nsMailNewsCommandLineHandler.js: let windowWatcher = Cc["@mozilla.org/embedcomp/window-watcher;1"]
virtualFolderWrapper.js: Cc["@mozilla.org/messenger/account-manager;1"]
virtualFolderWrapper.js: Cc["@mozilla.org/messenger/services/filters;1"]
I wonder why these .js files are even here under /src. They should probably be in /content. Would it be OK to move them?
Comment 1•12 years ago
|
||
(In reply to :aceman from comment #0)
> I wonder why these .js files are even here under /src. They should probably
> be in /content. Would it be OK to move them?
No, they are JS modules (loaded by Components.utils.import) or JS components (loaded by XPCOM infrastructure per their manifest definitions). If you look in Makefile.in, you will see they are listed under EXTRA_COMPONENTS or EXTRA_JS_MODULES (or PP variants).
Files in /content go there because they are scripts loaded into the global scope by 'script' tags.
Attachment #627580 -
Flags: review?(mbanner)
Comment 4•12 years ago
|
||
(In reply to :aceman from comment #2)
> Thanks. So why not mailnews/base/util?
Attempts at consistency, even if we lack the dependency reasons that might have motivated the C++ directory structure. For example virtualFolderWrapper.js deals with virtual folders, and nsMsgSearchDBView.cpp and nsMsgXFVirtualFolderDBView.cpp which implement the actual virtual folder logic live in the src/ directory, so it's more appropriate than util/ which is more a home for data-types/catch-all fallback.
Comment 5•12 years ago
|
||
Comment on attachment 627580 [details] [diff] [review]
patch
I'm busy this week, so passing review on.
Attachment #627580 -
Flags: review?(mbanner) → review?(mconley)
Comment 6•12 years ago
|
||
Comment on attachment 627580 [details] [diff] [review]
patch
Review of attachment 627580 [details] [diff] [review]:
-----------------------------------------------------------------
::: mailnews/base/src/msgAsyncPrompter.js
@@ +84,5 @@
> };
>
> function msgAsyncPrompter() {
> this._pendingPrompts = {};
> + this._threadManager = Services.tm;
Is there even a point of having this be aliased? We might as well just use Services.tm everywhere, instead of _threadManager.
Maybe for external callers, but I couldn't find any via http://mxr.mozilla.org/comm-central/search?string=_threadManager&find=&findi=&filter=^[^\0]*%24&hitlimit=&tree=comm-central .
Comment 8•12 years ago
|
||
The underscore indicates that this should only be used internally. Anybody reaching in and attempting to manipulate something starting with an underscore are playing a dangerous game.
So I think you're safe to just replace it with Services.tm.
OK.
Attachment #627580 -
Attachment is obsolete: true
Attachment #627580 -
Flags: review?(mconley)
Attachment #631510 -
Flags: review?(mconley)
Comment 10•12 years ago
|
||
Comment on attachment 631510 [details] [diff] [review]
patch v2
Looks good to me - thanks for your work,
-Mike
Attachment #631510 -
Flags: review?(mconley) → review+
Attachment #631510 -
Flags: review?(iann_bugzilla)
Attachment #631510 -
Flags: review?(iann_bugzilla) → review+
Keywords: checkin-needed
Comment 11•12 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Flags: in-testsuite-
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 16.0
You need to log in
before you can comment on or make changes to this bug.
Description
•