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)
:
Mentors:
Depends on:
Blocks: 433316
  Show dependency treegraph
 
Reported: 2008-12-16 05:14 PST by Mark Banner (:standard8)
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)
no flags Details | Diff | Review
The fix v2 (7.37 KB, patch)
2008-12-17 03:17 PST, Mark Banner (:standard8)
neil: review+
mozilla: superreview+
Details | Diff | Review
Add protocol handlers to packages files (7.37 KB, patch)
2009-01-08 02:39 PST, Mark Banner (:standard8)
no flags Details | Diff | Review
Add protocol handlers to packages files (1.19 KB, patch)
2009-01-08 02:47 PST, Mark Banner (:standard8)
neil: review+
mozilla: review+
Details | Diff | Review

Description Mark Banner (:standard8) 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) 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) 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) 2008-12-29 10:08:37 PST
Patch checked in: http://hg.mozilla.org/comm-central/rev/b8799ccd6a51
Comment 5 Mark Banner (:standard8) 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) 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) 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.