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

RESOLVED FIXED in Thunderbird 24.0

Status

Thunderbird
General
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: aryx, Assigned: aryx)

Tracking

(Blocks: 2 bugs)

Trunk
Thunderbird 24.0
Dependency tree / graph
Bug Flags:
in-testsuite +

Thunderbird Tracking Flags

(thunderbird24+ fixed)

Details

Attachments

(2 attachments, 2 obsolete attachments)

Created attachment 739464 [details] [diff] [review]
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
Attachment #739464 - Flags: review?(mconley)
Summary: Fxi leftovers from switch to Services.jsm and mailServices.j → Fix leftovers from switch to Services.jsm and mailServices.js

Comment 1

4 years ago
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?
(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 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,
Attachment #739464 - Flags: review?(mconley) → review+
Created attachment 751491 [details] [diff] [review]
patch, v3, r=mconley [checked in, comment 5]
Attachment #739464 - Attachment is obsolete: true
Keywords: checkin-needed
https://hg.mozilla.org/comm-central/rev/8fc54bbfaa0b
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Flags: in-testsuite+
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 24.0

Updated

4 years ago
Blocks: 720358, 720356

Comment 6

4 years ago
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;
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Created attachment 759980 [details] [diff] [review]
fix smtpServer falls, v1
Attachment #759980 - Flags: review?(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
Attachment #759980 - Flags: review?(mconley) → review+

Comment 9

4 years ago
Don't forget to get this into TB24.
tracking-thunderbird24: --- → ?
Created attachment 772803 [details] [diff] [review]
fix smtpServer calls, v2, r=mconley [checked in, comment 11]
Attachment #759980 - Attachment is obsolete: true
Keywords: checkin-needed
Whiteboard: [check in patch 'fix smtpServer calls' into comm-central]
https://hg.mozilla.org/comm-central/rev/65c5bfb05484
Status: REOPENED → RESOLVED
Last Resolved: 4 years ago4 years ago
Resolution: --- → FIXED
Note: please make a clear statement why we should track it, having to dig through comments isn't easy.
tracking-thunderbird24: ? → +
Keywords: checkin-needed
Whiteboard: [check in patch 'fix smtpServer calls' into comm-central]

Updated

4 years ago
Attachment #751491 - Attachment description: patch, v3, r=mconley → patch, v3, r=mconley [checked in, comment 5]

Comment 13

4 years ago
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
Attachment #772803 - Attachment description: fix smtpServer calls, v2, r=mconley [ready-for-checkin] → fix smtpServer calls, v2, r=mconley [checked in, comment 11]
Attachment #772803 - Flags: approval-comm-aurora?
Attachment #772803 - Flags: approval-comm-aurora? → approval-comm-aurora+
https://hg.mozilla.org/releases/comm-aurora/rev/0adb0914b405
status-thunderbird24: --- → fixed
You need to log in before you can comment on or make changes to this bug.