The default bug view has changed. See this FAQ.

SMTP protocol needs protocol handlers for toolkit password manager.

RESOLVED FIXED in Thunderbird 3.0b2

Status

MailNews Core
Networking: SMTP
RESOLVED FIXED
8 years ago
8 years ago

People

(Reporter: standard8, Assigned: standard8)

Tracking

Trunk
Thunderbird 3.0b2
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 2 obsolete attachments)

(Assignee)

Description

8 years ago
Created attachment 353202 [details] [diff] [review]
The fix

The toolkit password manager assumes that for the passwords saved in it, it can call IOService.newURI on it.

The SMTP protocol currently doesn't have any protocols registered for it, this means the IOService returns a simple URI, which doesn't do everything we need and hence the toolkit password manager doesn't work for it. The patch I'm attaching registers the SMTP and SMTPS protocols. Although we're not currently going to be using SMTPS I don't think it hurts to have it registered if we decide to use it later.

The only downside I can see to having these registered is that smtp:// links in emails will get linked but they won't do anything when clicked. I haven't found a way to disable them getting linked - I guess there probably isn't a way.

This patch also has basic tests to check we get the protocol ok, and the basic url is working as we expect. If you were to build with the toolkit password manager you'd also find that test_smtpPassword.js (which tests using passwords from the current wallet file) passes with this patch.
Attachment #353202 - Flags: superreview?(dmose)
Attachment #353202 - Flags: review?(neil)

Comment 1

8 years ago
Comment on attachment 353202 [details] [diff] [review]
The fix

>+    classDescription: aProtocol + " Protocol Handler",
Care to throw in a .toUpperCase()?

>+      nsIProtocolHandler.URI_NOAUTH |
I don't think this one fits, it means "no user or host"...

>+function nsSMTPProtocolHandler() {}
>+
>+nsSMTPProtocolHandler.prototype =
>+  makeProtocolHandler("smtp",
>+                      Components.interfaces.nsISmtpUrl.DEFAULT_SMTP_PORT,
>+                      "b14c2b67-8680-4c11-8d63-9403c7d4f757");
>+
>+function nsSMTPSProtocolHandler() {}
>+
>+nsSMTPSProtocolHandler.prototype =
>+  makeProtocolHandler("smtps",
>+                      Components.interfaces.nsISmtpUrl.DEFAULT_SMTPS_PORT,
>+                      "057d0997-9e3a-411e-b4ee-2602f53fe05f");
>+
>+function NSGetModule(compMgr, fileSpec) {
>+  return XPCOMUtils.generateModule([nsSMTPProtocolHandler,
>+                                    nsSMTPSProtocolHandler]);
>+}
Ugh, generateModule is really lame here :-(
(Assignee)

Comment 2

8 years ago
(In reply to comment #1)
> Ugh, generateModule is really lame here :-(

Well I've changed it slightly but I don't see a better way :-(
(Assignee)

Comment 3

8 years ago
Created attachment 353396 [details] [diff] [review]
The fix v2

Revised patch to address Neil's comments.
Attachment #353202 - Attachment is obsolete: true
Attachment #353396 - Flags: superreview?(dmose)
Attachment #353396 - Flags: review?(neil)
Attachment #353202 - Flags: superreview?(dmose)
Attachment #353202 - Flags: review?(neil)

Updated

8 years ago
Attachment #353396 - Flags: review?(neil) → review+
(Assignee)

Updated

8 years ago
Attachment #353396 - Flags: superreview?(dmose) → superreview?(bienvenu)

Updated

8 years ago
Attachment #353396 - Flags: superreview?(bienvenu) → superreview+
(Assignee)

Comment 4

8 years ago
Patch checked in: http://hg.mozilla.org/comm-central/rev/b8799ccd6a51
Status: NEW → RESOLVED
Last Resolved: 8 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.1b3
(Assignee)

Comment 5

8 years ago
Created attachment 355932 [details] [diff] [review]
Add protocol handlers to packages files

Thankfully I took a look at the build logs today and realised we were missing nsSMTPProtocolHandler.js from the packages files. Otherwise we could have had a few broken passwords...
Attachment #355932 - Flags: review?(neil)
Attachment #355932 - Flags: review?(bienvenu)
(Assignee)

Comment 6

8 years ago
Created attachment 355933 [details] [diff] [review]
Add protocol handlers to packages files

Correct patch this time...
Attachment #355932 - Attachment is obsolete: true
Attachment #355933 - Flags: review?(neil)
Attachment #355933 - Flags: review?(bienvenu)
Attachment #355932 - Flags: review?(neil)
Attachment #355932 - Flags: review?(bienvenu)

Updated

8 years ago
Attachment #355933 - Flags: review?(bienvenu) → review+

Updated

8 years ago
Attachment #355933 - Flags: review?(neil) → review+
(Assignee)

Comment 7

8 years ago
Patch checked in: http://hg.mozilla.org/comm-central/rev/8848a00c189c
Product: Core → MailNews Core
(Assignee)

Updated

8 years ago
Target Milestone: mozilla1.9.1b3 → Thunderbird 3.0b2
You need to log in before you can comment on or make changes to this bug.