Last Comment Bug 852690 - Remaining conversion to mailServices.js in /mail/ and /mailnews/
: Remaining conversion to mailServices.js in /mail/ and /mailnews/
Status: RESOLVED FIXED
:
Product: Thunderbird
Classification: Client Software
Component: General (show other bugs)
: Trunk
: All All
: -- enhancement (vote)
: Thunderbird 22.0
Assigned To: Sebastian H. [:aryx][:archaeopteryx]
:
Mentors:
: 821619 (view as bug list)
Depends on: 856350
Blocks: 720358
  Show dependency treegraph
 
Reported: 2013-03-19 13:16 PDT by Sebastian H. [:aryx][:archaeopteryx]
Modified: 2013-04-01 10:47 PDT (History)
7 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
accounts, v1 (33.71 KB, patch)
2013-03-19 13:16 PDT, Sebastian H. [:aryx][:archaeopteryx]
mconley: review+
Details | Diff | Splinter Review
compose, v1 (49.29 KB, patch)
2013-03-19 13:19 PDT, Sebastian H. [:aryx][:archaeopteryx]
mconley: review+
Details | Diff | Splinter Review
im, v1 (8.21 KB, patch)
2013-03-19 13:20 PDT, Sebastian H. [:aryx][:archaeopteryx]
florian: review+
Details | Diff | Splinter Review
import, v1 (7.62 KB, patch)
2013-03-19 13:22 PDT, Sebastian H. [:aryx][:archaeopteryx]
standard8: review+
Details | Diff | Splinter Review
/mailnews/test/, v1 (21.58 KB, patch)
2013-03-19 13:24 PDT, Sebastian H. [:aryx][:archaeopteryx]
standard8: review+
Details | Diff | Splinter Review
mdn, v1 (2.27 KB, patch)
2013-03-19 13:26 PDT, Sebastian H. [:aryx][:archaeopteryx]
mconley: review+
Details | Diff | Splinter Review
mime, v1 (11.24 KB, patch)
2013-03-19 13:27 PDT, Sebastian H. [:aryx][:archaeopteryx]
Pidgeot18: review+
Details | Diff | Splinter Review
newsblog, v1 (1.74 KB, patch)
2013-03-19 13:28 PDT, Sebastian H. [:aryx][:archaeopteryx]
standard8: review+
Details | Diff | Splinter Review
news, v1 (19.95 KB, patch)
2013-03-19 13:29 PDT, Sebastian H. [:aryx][:archaeopteryx]
Pidgeot18: review+
Details | Diff | Splinter Review
spam filter, v1 (18.59 KB, patch)
2013-03-19 13:32 PDT, Sebastian H. [:aryx][:archaeopteryx]
rkent: review+
Details | Diff | Splinter Review
im, v2, r=florian [checkin: comment 11] (8.49 KB, patch)
2013-03-20 07:47 PDT, Sebastian H. [:aryx][:archaeopteryx]
no flags Details | Diff | Splinter Review
spam filter, v2, r=rkent [checkin: comment 14] (18.86 KB, patch)
2013-03-21 05:35 PDT, Sebastian H. [:aryx][:archaeopteryx]
no flags Details | Diff | Splinter Review
accounts, v2, r=mconley [checkin: comment 28] (34.00 KB, patch)
2013-03-25 11:52 PDT, Sebastian H. [:aryx][:archaeopteryx]
no flags Details | Diff | Splinter Review
compose, v2, r=mconley [checkin: comment 28] (52.57 KB, patch)
2013-03-25 11:53 PDT, Sebastian H. [:aryx][:archaeopteryx]
no flags Details | Diff | Splinter Review
import, v2, r=Standard8 [checkin: comment 28] (7.89 KB, patch)
2013-03-25 11:54 PDT, Sebastian H. [:aryx][:archaeopteryx]
no flags Details | Diff | Splinter Review
/mailnews/test/, v2, r=Standard8 [checkin: comment 28] (21.86 KB, patch)
2013-03-25 11:55 PDT, Sebastian H. [:aryx][:archaeopteryx]
no flags Details | Diff | Splinter Review
mdn, v2, r=mconley [checkin: comment 28] (2.56 KB, patch)
2013-03-25 11:55 PDT, Sebastian H. [:aryx][:archaeopteryx]
no flags Details | Diff | Splinter Review
mime, v2, r=jcranmer [checkin: comment 28] (11.51 KB, patch)
2013-03-25 11:56 PDT, Sebastian H. [:aryx][:archaeopteryx]
no flags Details | Diff | Splinter Review
newsblog, v2, r=Standard8 [checkin: comment 28] (2.01 KB, patch)
2013-03-25 11:58 PDT, Sebastian H. [:aryx][:archaeopteryx]
no flags Details | Diff | Splinter Review
news, v2, r=jcranmer [checkin: comment 28] (20.22 KB, patch)
2013-03-25 11:59 PDT, Sebastian H. [:aryx][:archaeopteryx]
no flags Details | Diff | Splinter Review
addressbook, v2 (9.82 KB, patch)
2013-03-27 11:30 PDT, Sebastian H. [:aryx][:archaeopteryx]
standard8: review+
Details | Diff | Splinter Review
gloda, v2 (27.36 KB, patch)
2013-03-27 11:44 PDT, Sebastian H. [:aryx][:archaeopteryx]
standard8: review+
Details | Diff | Splinter Review
imap, v2 (49.75 KB, patch)
2013-03-27 11:50 PDT, Sebastian H. [:aryx][:archaeopteryx]
standard8: review+
Details | Diff | Splinter Review
internals, v2 (73.19 KB, patch)
2013-03-27 11:57 PDT, Sebastian H. [:aryx][:archaeopteryx]
standard8: review+
Details | Diff | Splinter Review
local, v2 (30.95 KB, patch)
2013-03-27 12:04 PDT, Sebastian H. [:aryx][:archaeopteryx]
standard8: review+
Details | Diff | Splinter Review
message views, v2 (83.52 KB, patch)
2013-03-27 12:27 PDT, Sebastian H. [:aryx][:archaeopteryx]
standard8: review+
Details | Diff | Splinter Review
search, v2 (32.91 KB, patch)
2013-03-27 12:31 PDT, Sebastian H. [:aryx][:archaeopteryx]
standard8: review+
Details | Diff | Splinter Review
addressbook, v3, r=Standard8 [checkin: comment 43] (9.91 KB, patch)
2013-03-29 03:10 PDT, Sebastian H. [:aryx][:archaeopteryx]
no flags Details | Diff | Splinter Review
gloda, v3, r=Standard8 [checkin: comment 43] (27.45 KB, patch)
2013-03-29 03:10 PDT, Sebastian H. [:aryx][:archaeopteryx]
no flags Details | Diff | Splinter Review
imap, v3, r=Standard8 [checkin: comment 43] (49.83 KB, patch)
2013-03-29 03:11 PDT, Sebastian H. [:aryx][:archaeopteryx]
no flags Details | Diff | Splinter Review
internals, v3, r=Standard8 [checkin: comment 43] (73.27 KB, patch)
2013-03-29 03:12 PDT, Sebastian H. [:aryx][:archaeopteryx]
no flags Details | Diff | Splinter Review
local, v3, r=Standard8 [checkin: comment 43] (31.03 KB, patch)
2013-03-29 03:12 PDT, Sebastian H. [:aryx][:archaeopteryx]
no flags Details | Diff | Splinter Review
message views, v3, r=Standard8 [checkin: comment 43] (83.61 KB, patch)
2013-03-29 03:13 PDT, Sebastian H. [:aryx][:archaeopteryx]
no flags Details | Diff | Splinter Review
search, v3, r=Standard8 [checkin: comment 43] (32.99 KB, patch)
2013-03-29 03:14 PDT, Sebastian H. [:aryx][:archaeopteryx]
no flags Details | Diff | Splinter Review

Description Sebastian H. [:aryx][:archaeopteryx] 2013-03-19 13:16:30 PDT
Created attachment 726864 [details] [diff] [review]
accounts, v1

Using mailServices.js will clean up the readability of our code a bit, and should avoid the costs the getService call each time we want a service.

Some patches are blocked by bug 824150 and I won't request review for them until their dependencies in that bug got checked in.

There was a successful Try run one month ago, but tbpl doesn't show anymore the results: https://tbpl.mozilla.org/?tree=Thunderbird-Try&showall=1&rev=e150a67bfeff
Comment 1 Sebastian H. [:aryx][:archaeopteryx] 2013-03-19 13:19:02 PDT
Created attachment 726865 [details] [diff] [review]
compose, v1
Comment 2 Sebastian H. [:aryx][:archaeopteryx] 2013-03-19 13:20:51 PDT
Created attachment 726868 [details] [diff] [review]
im, v1
Comment 3 Sebastian H. [:aryx][:archaeopteryx] 2013-03-19 13:22:44 PDT
Created attachment 726869 [details] [diff] [review]
import, v1
Comment 4 Sebastian H. [:aryx][:archaeopteryx] 2013-03-19 13:24:17 PDT
Created attachment 726872 [details] [diff] [review]
/mailnews/test/, v1
Comment 5 Sebastian H. [:aryx][:archaeopteryx] 2013-03-19 13:26:39 PDT
Created attachment 726873 [details] [diff] [review]
mdn, v1
Comment 6 Sebastian H. [:aryx][:archaeopteryx] 2013-03-19 13:27:42 PDT
Created attachment 726875 [details] [diff] [review]
mime, v1
Comment 7 Sebastian H. [:aryx][:archaeopteryx] 2013-03-19 13:28:51 PDT
Created attachment 726877 [details] [diff] [review]
newsblog, v1
Comment 8 Sebastian H. [:aryx][:archaeopteryx] 2013-03-19 13:29:52 PDT
Created attachment 726878 [details] [diff] [review]
news, v1
Comment 9 Sebastian H. [:aryx][:archaeopteryx] 2013-03-19 13:32:45 PDT
Created attachment 726880 [details] [diff] [review]
spam filter, v1
Comment 10 Sebastian H. [:aryx][:archaeopteryx] 2013-03-20 07:47:41 PDT
Created attachment 727202 [details] [diff] [review]
im, v2, r=florian [checkin: comment 11]
Comment 11 Ryan VanderMeulen [:RyanVM] 2013-03-20 10:14:49 PDT
Comment on attachment 727202 [details] [diff] [review]
im, v2, r=florian [checkin: comment 11]

https://hg.mozilla.org/comm-central/rev/8a50d60dd7a0
Comment 12 Kent James (:rkent) 2013-03-20 12:06:29 PDT
Comment on attachment 726880 [details] [diff] [review]
spam filter, v1

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

Looks good, thanks for the patch.
Comment 13 Sebastian H. [:aryx][:archaeopteryx] 2013-03-21 05:35:41 PDT
Created attachment 727637 [details] [diff] [review]
spam filter, v2, r=rkent [checkin: comment 14]
Comment 14 Ryan VanderMeulen [:RyanVM] 2013-03-21 06:53:38 PDT
Comment on attachment 727637 [details] [diff] [review]
spam filter, v2, r=rkent [checkin: comment 14]

https://hg.mozilla.org/comm-central/rev/75432cb9b21a
Comment 15 Joshua Cranmer [:jcranmer] 2013-03-21 15:32:51 PDT
Comment on attachment 726875 [details] [diff] [review]
mime, v1

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

You are just trying to bitrot all of my patches aren't you?

Then again, I suppose it would be my fault for rewriting all of these tests..
Comment 16 Joshua Cranmer [:jcranmer] 2013-03-21 15:36:55 PDT
Comment on attachment 726878 [details] [diff] [review]
news, v1

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

It's too bad that the method mailServices uses to generate their getters can't be used to specify having multiple interfaces.
Comment 17 Mike Conley (:mconley) - (Needinfo me!) 2013-03-23 12:09:55 PDT
Comment on attachment 726864 [details] [diff] [review]
accounts, v1

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

r=me with the following changes. Thanks!

::: mailnews/base/prefs/content/AccountWizard.js
@@ +403,2 @@
>    // Create an account.
> +  var account = MailServices.accounts.createAccount();

let, not var

@@ +411,2 @@
>      // Create an identity.
> +    var identity = MailServices.accounts.createIdentity();

let, not var

@@ +488,5 @@
>         */
>        if (destIdentity.attachSignature)
>        {
>            var sigFileName = accountData.signatureFileName;
> +          var sigFile = MailServices.mailSession.getDataFilesDir("messenger");

let, not var

@@ +497,5 @@
>        if (accountData.smtp.hostname && !destIdentity.smtpServerKey)
>        {
>            // hostname + no key => create a new SMTP server.
>  
> +          var smtpServer = MailServices.smtp.createServer();

let, not var

@@ +619,5 @@
>        copiesAndFoldersServer = server;
>      }
>      else 
>      {
> +      if (!MailServices.accounts.localFoldersServer) 

Please trim trailing whitespace.

@@ +775,5 @@
>      var skipPanelsPrefStr = "mail.identity." + identity.key + ".wizardSkipPanels";
>      accountData.wizardSkipPanels = Services.prefs.getCharPref(skipPanelsPrefStr);
>  
>      if (identity.smtpServerKey) {
> +      var smtpServer = MailServices.smtp.getServerByKey(identity.smtpServerKey);

let, not var

::: mailnews/base/prefs/content/accountcreation/createInBackend.js
@@ +20,2 @@
>    // incoming server
> +  var inServer = MailServices.accounts.createIncomingServer(

let, not var

@@ +121,5 @@
>    }
>  
>    // identity
>    // TODO accounts without identity?
> +  var identity = MailServices.accounts.createIdentity();

let, not var

@@ +139,5 @@
>    // Note: Setting incomingServer will cause the AccountManager to refresh
>    // itself, which could be a problem if we came from it and we haven't set
>    // the identity (see bug 521955), so make sure everything else on the
>    // account is set up before you set the incomingServer.
> +  var account = MailServices.accounts.createAccount();

let, not var

@@ +244,5 @@
>   */
>  function checkOutgoingServerAlreadyExists(config)
>  {
>    assert(config instanceof AccountConfig);
> +  var smtpServers = MailServices.smtp.servers;

let, not var

::: mailnews/base/prefs/content/accountcreation/emailWizard.js
@@ +199,5 @@
>  
>      // Populate SMTP server dropdown with already configured SMTP servers from
>      // other accounts.
>      var menulist = e("outgoing_hostname");
> +    var smtpServers = MailServices.smtp.servers;

let, not var

::: mailnews/base/prefs/content/accountcreation/verifyConfig.js
@@ +49,5 @@
>      return;
>    }
>  
>    // incoming server
>    var inServer =

let, not var
Comment 18 Mike Conley (:mconley) - (Needinfo me!) 2013-03-23 12:11:33 PDT
Comment on attachment 726873 [details] [diff] [review]
mdn, v1

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

That was easy. :) r=me.
Comment 19 Mike Conley (:mconley) - (Needinfo me!) 2013-03-23 12:20:53 PDT
Comment on attachment 726865 [details] [diff] [review]
compose, v1

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

This looks really good - I'm just concerned that we don't need the gMimeHeaderParser global in addressingWidgetOverlay.js. Can you try to get rid of that in a follow-up?

::: mail/components/compose/content/MsgComposeCommands.js
@@ +2273,5 @@
>  
>    // " <>" is an empty identity, and most likely not valid
>    if (!params.identity || params.identity.identityName == " <>") {
>      // no pre selected identity, so use the default account
> +    var identities = MailServices.accounts.defaultAccount.identities;

let, not var

@@ +2729,5 @@
>  
>      // Check if the user tries to send a message to a newsgroup through a mail
>      // account.
>      var currentAccountKey = getCurrentAccountKey();
> +    var account = MailServices.accounts.getAccount(currentAccountKey);

let, not var

@@ +3093,4 @@
>      var emailAddresses = {};
>      var names = {};
>      var fullNames = {};
> +    var numAddresses = MailServices.headerParser.parseHeadersWithArray(aAddressesToAdd, emailAddresses, names, fullNames);

let, not var

::: mail/components/compose/content/addressingWidgetOverlay.js
@@ +94,5 @@
>      var reply_Sep = "";
>      var ng_Sep = "";
>      var follow_Sep = "";
>  
> +    gMimeHeaderParser = MailServices.headerParser;

Why do we need this alias?

@@ +157,5 @@
>  
>  function CompFields2Recipients(msgCompFields)
>  {
>    if (msgCompFields) {
> +    gMimeHeaderParser = MailServices.headerParser;

Again, do we need this?

@@ +1035,4 @@
>    var addresses = {};
>    var names = {};
>    var fullNames = {};
> +  var numAddresses = MailServices.headerParser.parseHeadersWithArray(strippedAddresses, addresses, names, fullNames);

let, not var

::: mailnews/compose/test/unit/head_compose.js
@@ +37,2 @@
>    // Set up the identity
> +  var identity = MailServices.accounts.createIdentity();

let, not var

::: mailnews/compose/test/unit/test_bug474774.js
@@ +178,2 @@
>  
> +  var account = MailServices.accounts.createAccount();

let, not var for these two lines.

::: mailnews/compose/test/unit/test_sendBackground.js
@@ +103,2 @@
>  
> +  var account = MailServices.accounts.createAccount();

let, not var for these two.

::: mailnews/compose/test/unit/test_sendMessageLater.js
@@ +185,2 @@
>  
> +  var account = MailServices.accounts.createAccount();

let, not var for these two

::: mailnews/compose/test/unit/test_sendMessageLater2.js
@@ +229,2 @@
>  
> +  var account = MailServices.accounts.createAccount();

let, not var for these two

::: mailnews/compose/test/unit/test_sendMessageLater3.js
@@ +132,2 @@
>  
> +  var account = MailServices.accounts.createAccount();

let, not var for these two

::: mail/test/mozmill/message-header/test-reply-to-list-from-address-selection.js
@@ +68,5 @@
>    return aFolder.msgDatabase.getMsgHdrForMessageID(msgId);
>  }
>  
>  var addIdentitiesAndFolder = function() {
> +  var identity2 = MailServices.accounts.createIdentity();

let, not var for each of these changes.
Comment 20 Sebastian H. [:aryx][:archaeopteryx] 2013-03-25 11:52:49 PDT
Created attachment 729106 [details] [diff] [review]
accounts, v2, r=mconley [checkin: comment 28]
Comment 21 Sebastian H. [:aryx][:archaeopteryx] 2013-03-25 11:53:34 PDT
Created attachment 729107 [details] [diff] [review]
compose, v2, r=mconley [checkin: comment 28]
Comment 22 Sebastian H. [:aryx][:archaeopteryx] 2013-03-25 11:54:17 PDT
Created attachment 729108 [details] [diff] [review]
import, v2, r=Standard8 [checkin: comment 28]
Comment 23 Sebastian H. [:aryx][:archaeopteryx] 2013-03-25 11:55:03 PDT
Created attachment 729109 [details] [diff] [review]
/mailnews/test/, v2, r=Standard8 [checkin: comment 28]
Comment 24 Sebastian H. [:aryx][:archaeopteryx] 2013-03-25 11:55:44 PDT
Created attachment 729110 [details] [diff] [review]
mdn, v2, r=mconley [checkin: comment 28]
Comment 25 Sebastian H. [:aryx][:archaeopteryx] 2013-03-25 11:56:49 PDT
Created attachment 729111 [details] [diff] [review]
mime, v2, r=jcranmer [checkin: comment 28]
Comment 26 Sebastian H. [:aryx][:archaeopteryx] 2013-03-25 11:58:37 PDT
Created attachment 729113 [details] [diff] [review]
newsblog, v2, r=Standard8 [checkin: comment 28]
Comment 27 Sebastian H. [:aryx][:archaeopteryx] 2013-03-25 11:59:17 PDT
Created attachment 729116 [details] [diff] [review]
news, v2, r=jcranmer [checkin: comment 28]
Comment 29 Sebastian H. [:aryx][:archaeopteryx] 2013-03-27 11:30:36 PDT
Created attachment 730249 [details] [diff] [review]
addressbook, v2
Comment 30 Sebastian H. [:aryx][:archaeopteryx] 2013-03-27 11:44:59 PDT
Created attachment 730254 [details] [diff] [review]
gloda, v2
Comment 31 Sebastian H. [:aryx][:archaeopteryx] 2013-03-27 11:50:26 PDT
Created attachment 730258 [details] [diff] [review]
imap, v2
Comment 32 Sebastian H. [:aryx][:archaeopteryx] 2013-03-27 11:57:57 PDT
Created attachment 730266 [details] [diff] [review]
internals, v2
Comment 33 Sebastian H. [:aryx][:archaeopteryx] 2013-03-27 12:04:09 PDT
Created attachment 730269 [details] [diff] [review]
local, v2
Comment 34 Sebastian H. [:aryx][:archaeopteryx] 2013-03-27 12:27:45 PDT
Created attachment 730281 [details] [diff] [review]
message views, v2
Comment 35 Sebastian H. [:aryx][:archaeopteryx] 2013-03-27 12:31:33 PDT
Created attachment 730283 [details] [diff] [review]
search, v2
Comment 36 Sebastian H. [:aryx][:archaeopteryx] 2013-03-29 03:10:10 PDT
Created attachment 731092 [details] [diff] [review]
addressbook, v3, r=Standard8 [checkin: comment 43]
Comment 37 Sebastian H. [:aryx][:archaeopteryx] 2013-03-29 03:10:45 PDT
Created attachment 731093 [details] [diff] [review]
gloda, v3, r=Standard8 [checkin: comment 43]
Comment 38 Sebastian H. [:aryx][:archaeopteryx] 2013-03-29 03:11:24 PDT
Created attachment 731094 [details] [diff] [review]
imap, v3, r=Standard8 [checkin: comment 43]
Comment 39 Sebastian H. [:aryx][:archaeopteryx] 2013-03-29 03:12:02 PDT
Created attachment 731095 [details] [diff] [review]
internals, v3, r=Standard8 [checkin: comment 43]
Comment 40 Sebastian H. [:aryx][:archaeopteryx] 2013-03-29 03:12:51 PDT
Created attachment 731097 [details] [diff] [review]
local, v3, r=Standard8 [checkin: comment 43]
Comment 41 Sebastian H. [:aryx][:archaeopteryx] 2013-03-29 03:13:28 PDT
Created attachment 731098 [details] [diff] [review]
message views, v3, r=Standard8 [checkin: comment 43]
Comment 42 Sebastian H. [:aryx][:archaeopteryx] 2013-03-29 03:14:05 PDT
Created attachment 731100 [details] [diff] [review]
search, v3, r=Standard8 [checkin: comment 43]
Comment 44 Sebastian H. [:aryx][:archaeopteryx] 2013-04-01 10:47:30 PDT
*** Bug 821619 has been marked as a duplicate of this bug. ***

Note You need to log in before you can comment on or make changes to this bug.