804004 - Convert rest of Account manager files to Services.jsm and mailServices.js

Status: RESOLVED FIXED

(MailNews Core :: Account Manager, defect)

Description

[:aceman]

am-addressing.js:    gPrefInt = Components.classes["@mozilla.org/preferences-service;1"]
am-copies.js:var gPrefBranch = Components.classes["@mozilla.org/preferences-service;1"].getService(Components.interfaces.nsIPrefBranch);
am-identity-edit.js:    var accountManager = Components.classes["@mozilla.org/messenger/account-manager;1"]
am-identity-edit.js:    var promptService = Components.classes["@mozilla.org/embedcomp/prompt-service;1"]
am-identity-edit.js:  var smtpService = Components.classes["@mozilla.org/messengercompose/smtp;1"]
am-offline.js:    var prefService = Components.classes["@mozilla.org/preferences-service;1"];
am-server-advanced.js:    gAccountManager = Components.classes["@mozilla.org/messenger/account-manager;1"].getService(Components.interfaces.nsIMsgAccountManager);
am-server-advanced.js:        Components.classes["@mozilla.org/embedcomp/prompt-service;1"].
am-server.js:  gObserver= Components.classes["@mozilla.org/observer-service;1"].

Comment 1

[:aceman]

Attachment: patch

This intentionally skips am-offline.js as that is covered in bug 755885.

Comment 2

[Ian Neal]

Comment on attachment 673722 [details] [diff] [review]
patch

>+++ b/mailnews/base/prefs/content/am-addressing.js
>+Components.utils.import("resource://gre/modules/Services.jsm");
am-addressingOverlay.xul loads both am-addressing.js and amUtils.js.
As amUtils.js already imports Services.jsm you don't strictly need to import it here.

>+++ b/mailnews/base/prefs/content/am-identity-edit.js
>+Components.utils.import("resource:///modules/mailServices.js");
>+Components.utils.import("resource://gre/modules/Services.jsm");
am-identity-edit.xul and am-main.xul both loads both am-prefs.js.
As am-prefs.js already imports Services.jsm you don't strictly need to import it here.

>+++ b/mailnews/base/prefs/content/am-server-advanced.js

>       case "2":
>         picker = document.getElementById("deferedServerFolderPicker");
picker doesn't seem to be used, so you could remove it.

>+++ b/mailnews/base/prefs/content/am-server.js
>+Components.utils.import("resource://gre/modules/Services.jsm");
am-server.xul loads amUtils.js.
As amUtils.js already imports Services.jsm you don't strictly need to import it here.

There appears to be an orphaned gPrefBranch here:
http://mxr.mozilla.org/comm-central/source/mailnews/base/content/msgAccountCentral.js#280

Does that need fixing up the same time as the rest of this?

Comment 3

[:aceman]

Technically, Account central is not covered by this bug and the gPrefBranch is defined somewhere else (not in am-*.js). It is still defined even after this patch. But yes, I can replace it for simplicity (so we do not wonder where it comes from).

I'll rather leave the 'not strictly needed imports' for now. The inclusion of amUtils.js may change in the future and then suddenly they will be missing. The imports indicate what modules are used in which file. In the future I may make a pass over all the files and optimize the imports: my idea would be to import them only into AccountManages.js and then call them via top.document.Services or something like that. Would that be better? Or is that slower again?

Comment 4

[:aceman]

Attachment: patch v2

Comment 5

[Mike Conley (:mconley) (:⚙️)]

Comment on attachment 674311 [details] [diff] [review]
patch v2

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

This looks pretty good - have you run the accounts tests?

::: mailnews/base/prefs/content/am-addressing.js
@@ +53,3 @@
>    }
>  
> +  if (gIdentity && Services.prefs.prefIsLocked("mail.identity." + gIdentity.key + ".directoryServer")) {

Is this under 80 chars? If not, please break it up.

::: mailnews/base/prefs/content/am-server-advanced.js
@@ +23,5 @@
>  }
>  
>  function getLocalFoldersAccount()
>  {
> +  var localFoldersServer = MailServices.accounts.localFoldersServer;

let, not var

@@ +128,3 @@
>          var server = document.getElementById("deferedServerFolderPicker")
>                               .selectedItem._folder.server;
> +        var account = MailServices.accounts.FindAccountForServer(server);

let, not var

::: mailnews/base/prefs/content/am-server.js
@@ -6,3 @@
>  var gServer;
> -var gObserver;
> -const Ci = Components.interfaces;

Why are you dropping Ci?

Comment 6

[:aceman]

(In reply to Mike Conley (:mconley) from comment #5)
> ::: mailnews/base/prefs/content/am-server.js
> @@ -6,3 @@
> >  var gServer;
> > -var gObserver;
> > -const Ci = Components.interfaces;
> 
> Why are you dropping Ci?

It is unused after the patch. Should I leave it with the few occurences it has?

Comment 7

[:aceman]

Attachment: patch v3

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/comm-central/rev/7863603e2121