Last Comment Bug 469807 - SMTP protocol needs protocol handlers for toolkit password manager.
: SMTP protocol needs protocol handlers for toolkit password manager.
Status: RESOLVED FIXED
:
Product: MailNews Core
Classification: Components
Component: Networking: SMTP (show other bugs)
: Trunk
: All All
: -- normal (vote)
: Thunderbird 3.0b2
Assigned To: Mark Banner (:standard8, limited time in Dec)
:
:
Mentors:
Depends on:
Blocks: 433316
  Show dependency treegraph
 
Reported: 2008-12-16 05:14 PST by Mark Banner (:standard8, limited time in Dec)
Modified: 2009-02-02 07:00 PST (History)
4 users (show)
standard8: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
The fix (7.49 KB, patch)
2008-12-16 05:14 PST, Mark Banner (:standard8, limited time in Dec)
no flags Details | Diff | Splinter Review
The fix v2 (7.37 KB, patch)
2008-12-17 03:17 PST, Mark Banner (:standard8, limited time in Dec)
neil: review+
mozilla: superreview+
Details | Diff | Splinter Review
Add protocol handlers to packages files (7.37 KB, patch)
2009-01-08 02:39 PST, Mark Banner (:standard8, limited time in Dec)
no flags Details | Diff | Splinter Review
Add protocol handlers to packages files (1.19 KB, patch)
2009-01-08 02:47 PST, Mark Banner (:standard8, limited time in Dec)
neil: review+
mozilla: review+
Details | Diff | Splinter Review

Description Mark Banner (:standard8, limited time in Dec) 2008-12-16 05:14:08 PST
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.
Comment 1 neil@parkwaycc.co.uk 2008-12-16 16:04:55 PST
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 :-(
Comment 2 Mark Banner (:standard8, limited time in Dec) 2008-12-17 03:16:11 PST
(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 :-(
Comment 3 Mark Banner (:standard8, limited time in Dec) 2008-12-17 03:17:17 PST
Created attachment 353396 [details] [diff] [review]
The fix v2

Revised patch to address Neil's comments.
Comment 4 Mark Banner (:standard8, limited time in Dec) 2008-12-29 10:08:37 PST
Patch checked in: http://hg.mozilla.org/comm-central/rev/b8799ccd6a51
Comment 5 Mark Banner (:standard8, limited time in Dec) 2009-01-08 02:39:37 PST
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...
Comment 6 Mark Banner (:standard8, limited time in Dec) 2009-01-08 02:47:21 PST
Created attachment 355933 [details] [diff] [review]
Add protocol handlers to packages files

Correct patch this time...
Comment 7 Mark Banner (:standard8, limited time in Dec) 2009-01-08 11:45:23 PST
Patch checked in: http://hg.mozilla.org/comm-central/rev/8848a00c189c

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