439292 - Context menu for mailto: links should include commands like "Open in new tab" (when handled by a web app)

Status: RESOLVED FIXED

(Firefox :: General, enhancement)

Description

[Nochum Sossonko [:Natch]]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9) Gecko/2008052906 Firefox/3.0
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9) Gecko/2008052906 Firefox/3.0

The context menu for regular links includes an option to open in a new tab/window, since mailto is now handled in the browser, there should be a similar option.

Reproducible: Always

Steps to Reproduce:
1.
2.
3.

Comment 1

[Nochum Sossonko [:Natch]]

Attachment: Patch for nsContextmenu

Here's a patch that's tested and works.

Comment 2

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 335590 [details] [diff] [review]
Patch for nsContextmenu

>diff --git a/browser/base/content/nsContextMenu.js b/browser/base/content/nsContextMenu.js

>   initOpenItems: function CM_initOpenItems() {
>+    var extProtocolSvc = Components.classes["@mozilla.org/uriloader/external-protocol-service;1"]
>+                                   .getService(Components.interfaces.nsIExternalProtocolService);
>+    var isMailtoInternal;
>+    if (extProtocolSvc) {

This null check is unnecessary, getService will either return an object or throw. You can just call getProtocolHandlerInfo directly on the result of getService and assign directly to mailtoHandler (removing the extProtocolSvc variable entirely). You can also use the Cc/Ci shortcuts here as with other getService calls in this file.

>+      var mailtoHandler = extProtocolSvc.getProtocolHandlerInfo("mailto");
>+      isMailtoInternal = (mailtoHandler.preferredAction == 2 &&

Can use Ci.nsIHandlerInfo.useHelperApp here, rather than "2".

>+                         (mailtoHandler.preferredApplicationHandler instanceof Ci.nsIWebHandlerApp));
>+    }

Otherwise this looks good, thanks for the patch! Attach a patch addressing those comments and I'll r+.

Comment 3

[Nochum Sossonko [:Natch]]

Attachment: Addresses comment 2

Comment 4

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 335631 [details] [diff] [review]
Addresses comment 2

Actually, I noticed one last thing: you can avoid doing all that work to retrieve isMailtoInternal if this.onMailtoLink is false. I'll fix that and check it in.

Comment 5

[:Gavin Sharp [email: [e-mail]]]

Attachment: updated patch

Comment 6

[:Gavin Sharp [email: [e-mail]]]

Hmm, with this latest patch I still get the "Open in new tab" options if I select "Always ask".

Comment 7

[:Gavin Sharp [email: [e-mail]]]

Attachment: updated updated patch

A check for !alwaysAskBeforeHandling fixes that.

Comment 8

[:Gavin Sharp [email: [e-mail]]]

Fixed for Firefox 3.1
http://hg.mozilla.org/mozilla-central/index.cgi/rev/5c132b2d3848

Thanks again for the patch!

Comment 9

[Nochum Sossonko [:Natch]]

(In reply to comment #7)
> Created an attachment (id=335639) [details]
> updated updated patch
> 
> A check for !alwaysAskBeforeHandling fixes that.


That shouldn't be necessary as preferredAction should not be == useHelperApp. It worked in my patch. Anyhow... thanks for the help.

Comment 10

[:Gavin Sharp [email: [e-mail]]]

(In reply to comment #9)
> That shouldn't be necessary as preferredAction should not be == useHelperApp.
> It worked in my patch. Anyhow... thanks for the help.

preferredAction isn't updated when askBeforeHandling is set, so it is needed (at least for me, on Mac). You need to switch to a web handler (so that preferredAction gets set), and then switch to "Always ask" for it to be a problem, though, so that might explain why you didn't see that problem in testing.