852690 - Remaining conversion to mailServices.js in /mail/ and /mailnews/

Status: RESOLVED FIXED

(Thunderbird :: General, enhancement)

Description

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

Attachment: 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 Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Attachment: compose, v1

Comment 2

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

Attachment: im, v1

Comment 3

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

Attachment: import, v1

Comment 4

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

Attachment: /mailnews/test/, v1

Comment 5

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

Attachment: mdn, v1

Comment 6

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

Attachment: mime, v1

Comment 7

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

Attachment: newsblog, v1

Comment 8

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

Attachment: news, v1

Comment 9

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

Attachment: spam filter, v1

Comment 10

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

Attachment: im, v2, r=florian [checkin: comment 11]

Comment 11

[Ryan VanderMeulen [:RyanVM]]

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)]

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 Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Attachment: spam filter, v2, r=rkent [checkin: comment 14]

Comment 14

[Ryan VanderMeulen [:RyanVM]]

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]]

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]]

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) (:⚙️)]

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) (:⚙️)]

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) (:⚙️)]

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 Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Attachment: accounts, v2, r=mconley [checkin: comment 28]

Comment 21

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

Attachment: compose, v2, r=mconley [checkin: comment 28]

Comment 22

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

Attachment: import, v2, r=Standard8 [checkin: comment 28]

Comment 23

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

Attachment: /mailnews/test/, v2, r=Standard8 [checkin: comment 28]

Comment 24

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

Attachment: mdn, v2, r=mconley [checkin: comment 28]

Comment 25

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

Attachment: mime, v2, r=jcranmer [checkin: comment 28]

Comment 26

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

Attachment: newsblog, v2, r=Standard8 [checkin: comment 28]

Comment 27

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

Attachment: news, v2, r=jcranmer [checkin: comment 28]

Comment 28

[Mark Banner (:standard8)]

https://hg.mozilla.org/comm-central/rev/2e45eea93d3e
https://hg.mozilla.org/comm-central/rev/45dc9315bd58
https://hg.mozilla.org/comm-central/rev/4da58068e80c
https://hg.mozilla.org/comm-central/rev/de20b3bf7162
https://hg.mozilla.org/comm-central/rev/ff8167d02656
https://hg.mozilla.org/comm-central/rev/8add84037357
https://hg.mozilla.org/comm-central/rev/90c4c9c880bf
https://hg.mozilla.org/comm-central/rev/0d9c225b2a8b

Comment 29

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

Attachment: addressbook, v2

Comment 30

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

Attachment: gloda, v2

Comment 31

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

Attachment: imap, v2

Comment 32

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

Attachment: internals, v2

Comment 33

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

Attachment: local, v2

Comment 34

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

Attachment: message views, v2

Comment 35

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

Attachment: search, v2

Comment 36

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

Attachment: addressbook, v3, r=Standard8 [checkin: comment 43]

Comment 37

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

Attachment: gloda, v3, r=Standard8 [checkin: comment 43]

Comment 38

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

Attachment: imap, v3, r=Standard8 [checkin: comment 43]

Comment 39

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

Attachment: internals, v3, r=Standard8 [checkin: comment 43]

Comment 40

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

Attachment: local, v3, r=Standard8 [checkin: comment 43]

Comment 41

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

Attachment: message views, v3, r=Standard8 [checkin: comment 43]

Comment 42

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

Attachment: search, v3, r=Standard8 [checkin: comment 43]

Comment 43

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/comm-central/rev/cb0afd1720f9
https://hg.mozilla.org/comm-central/rev/d202d72fc4c1
https://hg.mozilla.org/comm-central/rev/3a6a45f4bf24
https://hg.mozilla.org/comm-central/rev/f65892db66d3
https://hg.mozilla.org/comm-central/rev/5455891552e4
https://hg.mozilla.org/comm-central/rev/e650ba2dd552
https://hg.mozilla.org/comm-central/rev/0b0ff12d6f09