863617 - Fix leftovers from switch to Services.jsm and mailServices.js

Status: RESOLVED FIXED

(Thunderbird :: General, defect)

Description

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

Attachment: patch, v2

aceman found some code which can be converted to Services.jsm / mailServices.js. Successful Thunderbird-Try run: https://tbpl.mozilla.org/?tree=Thunderbird-Try&showall=1&rev=00293112bed4

Comment 1

[:aceman]

OK, this seems to cover some of the remaining users, but after applying the patch I still see these occurrences:

mailnews:
news/test/unit/test_uriParser.js:  let nntpService = Cc["@mozilla.org/messenger/nntpservice;1"]
news/test/unit/test_getNewsMessage.js:    var nntpService = Cc["@mozilla.org/messenger/nntpservice;1"]
compose/test/unit/test_nsSmtpService1.js:const SmtpServiceContractID = "@mozilla.org/messengercompose/smtp;1";
extensions/bayesian-spam-filter/test/unit/test_traits.js:  Cc["@mozilla.org/messenger/filter-plugin;1?name=bayesianfilter"]
extensions/bayesian-spam-filter/test/unit/test_traits.js:        Cc["@mozilla.org/messenger/filter-plugin;1?name=bayesianfilter"]
extensions/bayesian-spam-filter/test/unit/test_msgCorpus.js:  Cc["@mozilla.org/messenger/filter-plugin;1?name=bayesianfilter"]

mail:
base/content/plugins.js:                                   "@mozilla.org/xre/app-info;1",
base/content/aboutDialog.js:                                     "@mozilla.org/extensions/blocklist;1",
base/test/unit/test_windows_font_migration.js:  let sysInfo = Cc["@mozilla.org/system-info;1"]

(Ignoring mozmill itself and testpilot.) Are these intentionally left out or inconvertible?

Comment 2

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

(In reply to :aceman from comment #1)
> OK, this seems to cover some of the remaining users, but after applying the
> patch I still see these occurrences:
> 
> mailnews:
> news/test/unit/test_uriParser.js:  let nntpService =
> Cc["@mozilla.org/messenger/nntpservice;1"]
> news/test/unit/test_getNewsMessage.js:    var nntpService =
> Cc["@mozilla.org/messenger/nntpservice;1"]
These are tests for the news interface itself, so they shouldn't be changed.

> compose/test/unit/test_nsSmtpService1.js:const SmtpServiceContractID =
> "@mozilla.org/messengercompose/smtp;1";
This is a test for the smtp interface itself, so it shouldn't be changed.

> extensions/bayesian-spam-filter/test/unit/test_traits.js: 
> Cc["@mozilla.org/messenger/filter-plugin;1?name=bayesianfilter"]
> extensions/bayesian-spam-filter/test/unit/test_traits.js:       
> Cc["@mozilla.org/messenger/filter-plugin;1?name=bayesianfilter"]
> extensions/bayesian-spam-filter/test/unit/test_msgCorpus.js: 
> Cc["@mozilla.org/messenger/filter-plugin;1?name=bayesianfilter"]
These are tests for the junk interface itself, so they shouldn't be changed.


> mail:
> base/content/plugins.js:                                  
> "@mozilla.org/xre/app-info;1",
This calls nsICrashReporter which is not covered by Services.jsm

> base/content/aboutDialog.js:                                    
> "@mozilla.org/extensions/blocklist;1",
blocklist has been added to Services.jsm after this bug has been added and is already covered by bug 864838.

> base/test/unit/test_windows_font_migration.js:  let sysInfo =
> Cc["@mozilla.org/system-info;1"]
This calls nsIWriteablePropertyBag2 which is not covered by Services.jsm

> (Ignoring mozmill itself and testpilot.) Are these intentionally left out or
> inconvertible?
mozmill: As far as I know mozmill had been used before for Firefox, so I regarded it as shared code because the main repository should get the patches and Thunderbird get them downstream.

testpilot: Gregg Lind (tp developer) and squib (ported tp to tb) agreed last week that tp shouldn't ship anymore in its current state. As far as I know the new version 2 is a rewrite, so patching version 1 is a waste of time.

Comment 3

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

Comment on attachment 739464 [details] [diff] [review]
patch, v2

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

Looks good. r=me with my nit fixed. Thanks Sebastian!

::: mail/base/content/phishingDetector.js
@@ +269,5 @@
>        var dialogMsg = bundle.getFormattedString("confirmPhishingUrl", 
>                          [brandShortName, unobscuredHostNameValue], 2);
>  
>        const nsIPS = Components.interfaces.nsIPromptService;
> +      return !Services.prompt.confirmEx(window, titleMsg, dialogMsg, nsIPS.STD_YES_NO_BUTTONS + nsIPS.BUTTON_POS_1_DEFAULT, 

This looks like it can be broken up more neatly:

return !Services.prompt.confirmEx(window, titleMsg, dialogMsg,
                                  nsIPS.STD_YES_NO_BUTTONS +
                                  nsIPS.BUTTON_POS_1_DEFAULT,
                                  "", "", "", "", {});

Also note the trimmed whitespace after BUTTON_POS_1_DEFAULT,

Comment 4

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

Attachment: patch, v3, r=mconley [checked in, comment 5]

Comment 5

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/comm-central/rev/8fc54bbfaa0b

Comment 6

[:aceman]

Looks like these still need update... smtpService got removed :(
/mailnews/base/prefs/content/aw-done.js
    line 128 -- var smtpServer = parent.smtpService.defaultServer;

/mailnews/base/prefs/content/aw-outgoing.js
    line 48 -- if (parent.smtpService.defaultServer && !smtpCreateNewServer) {
    line 49 -- smtpServer = parent.smtpService.defaultServer;
    line 85 -- var smtpServer = parent.smtpService.defaultServer;

Comment 7

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

Attachment: fix smtpServer falls, v1

Comment 8

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

Comment on attachment 759980 [details] [diff] [review]
fix smtpServer falls, v1

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

r=me with the vars switched to lets. We don't like vars. :)

::: mailnews/base/prefs/content/aw-done.js
@@ +126,5 @@
>          setDivTextFromForm("server.type", incomingServerType.toUpperCase());
>  
>          var smtpServerName="";
>          if (pageData.server && pageData.server.smtphostname) {
> +          var smtpServer = MailServices.smtp.defaultServer;

Please change var to let

::: mailnews/base/prefs/content/aw-outgoing.js
@@ +82,5 @@
>      if (boxToShow)
>        boxToShow.removeAttribute("hidden");
>  
>      var smtpNameInput = document.getElementById("smtpusername");
> +    var smtpServer = MailServices.smtp.defaultServer;

let, not var

Comment 9

[:aceman]

Don't forget to get this into TB24.

Comment 10

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

Attachment: fix smtpServer calls, v2, r=mconley [checked in, comment 11]

Comment 11

[Mark Banner (:standard8)]

https://hg.mozilla.org/comm-central/rev/65c5bfb05484

Comment 12

[Mark Banner (:standard8)]

Note: please make a clear statement why we should track it, having to dig through comments isn't easy.

Comment 13

[:aceman]

Comment on attachment 772803 [details] [diff] [review]
fix smtpServer calls, v2, r=mconley [checked in, comment 11]

[Approval Request Comment]
Regression caused by (bug #): one of the mailServices conversion bugs reachable from bug 720358.
User impact if declined: Account wizard possibly not finishing properly
Testing completed (on c-c, etc.): TB24
Risk to taking this patch (and alternatives if risky): probably none

Comment 14

[Mark Banner (:standard8)]

https://hg.mozilla.org/releases/comm-aurora/rev/0adb0914b405