281847 - Linux: clicking mailto: link and "send link" fail to bring up mail compose

Status: RESOLVED FIXED

(Thunderbird :: General, defect)

Description

[sairuh (rarely reading bugmail)]

found using 2005021006-trunk tbird bits on linux fc2. I'm able to set
thunderbird as my default mail client, but accessing a mailto: link or "send
link" command in my browser (firefox), fails to launch a mail compose window.

1. install (decompress) today's tbird build and launch it.

2. it asks me to make it the default mail client (I click OK).

3. in firefox, click a mailto: link or select File > Send Link.

expected: mail compose window should launch.

actual results: no mail compose appears. in the shell from which I launched
tbird, I see this error:

/home/sairuh/thunderbird-bits/2005021006-trunk/thunderbird/mozilla-xremote-client:
Error: Failed to send command: 509 internal error

I didn't see other JS console errors.

note: if tbird is already set as the default browser, but not launched, clicking
mailto: or doing "send link" *does* launch tbird --but a mail compose window
still doesn't open.

Comment 1

[sairuh (rarely reading bugmail)]

hrm, I'm seeing this with 2005020904-trunk's tbird build as well.

Tracy / Scott, could this be a regression on Firefox's part? I'm not sure, since
the error is appearing in the shell for tbird, but...

Comment 2

[Scott MacGregor]

cc'ing some xremote gurus who may know of recent xremote changes that could have
introduced this behavior for Thunderbird. The only recent xremote changes I saw
had to do with supporting calendar and that didn't look like it would cause
xremote for Thunderbird to stop working.

Comment 3

[David :Bienvenu]

changing summary

Comment 4

[Daniel de Wildt]

Duplicate of bug 227786?

Comment 5

[Scott MacGregor]

you guys still seeing this problem?

Comment 6

[Adam Guthrie]

(In reply to comment #5)
> you guys still seeing this problem?

version 1.6a1 (20050824)

Yep.

Comment 7

[Andreas Otte]

I see this too. It works fine between firefox 1.0.6 and thunderbird 1.0.6, but
trunk and 1.8 branch firefox do not bring up thunderbird when clicking on mail
links and do not linkify email addresses in bugzilla's bugs-sent page. To me
this looks like a firefox problem with the exthandler.

Comment 8

[Andreas Otte]

I just figured it out (thanks to a blog entry by gerv). I had to set a user pref
in user.js "network.protocol-handler.app.mailto" to "/[your path]/thunderbird".
It seems while older firefox (1.0.x) recognized my KDE settngs, newer versions
(1.5) do not.

Comment 9

[Adam Guthrie]

*** Bug 309933 has been marked as a duplicate of this bug. ***

Comment 10

[Tuukka Tolvanen (sp3000)]

According to the gnome default apps config applet, tb registers as default mail
app with the command line
    .../thunderbird "%s"
("..." being the path). With that, clicking on the testcase link in firefox, or
File->Send Link in firefox, or running
    thunderbird mailto:[e-mail]
from the command line, only focuses the tb main window.

If I change the gnome default mail app command line to
    .../thunderbird -compose "%s"
or run
    thunderbird -compose mailto:[e-mail]
from the command line, the compose window is brought up as expacted. So it
appears that tb should either register as default mail app with the -compose
option, or not require the -compose option in order to bring up a compose window
when a mailto: argument is provided.

Comment 11

[Adam Guthrie]

*** Bug 310001 has been marked as a duplicate of this bug. ***

Comment 12

[Scott MacGregor]

this is already scheduled for 1.1

Comment 13

[Scott MacGregor]

Tuuka, do you have access to a linux build which you could try some potential
patches if I put some together?

I see where the gnome integration code isn't including -compose when registering
for mailto urls in the gnome registry. I can put up a patch if you can test it.

Comment 14

[Scott MacGregor]

Actually, upon further inspection, it looks like 1.0 wrote the same values into
the registry. None of this code has changed.

And sure enough, running 1.0.x, I can call

thunderbird.exe mailto:[e-mail]

and we properly bring up a compose window instead of the main mail 3 pane
window. The same command line on a trunk / 1.5 build brings up the mail 3 pane
instead of the compose window.

Benjamin, I wonder if this regression is from Bug 276588 which was the new
command line handler stuff which landed after 1.0.

Comment 15

[Benjamin Smedberg]

This is due precisely to the comment at

http://lxr.mozilla.org/mozilla/source/mail/components/nsMailDefaultHandler.js#216

On further inspection, I think that we can use uriloader (the eventloop won't
get tricked, because the window will be open already, even if it isn't set up
completely).

Comment 16

[Tuukka Tolvanen (sp3000)]

Attachment: patch1: nsMsgComposeService::Handle

here's a patch I cooked up for nsMsgComposeService::Handle, that being the
first fn that caught my eye in the bug 276588 patches. I wouldn't know if
nsMailDefaultHandler::handle is a better place for this, though.

Comment 17

[Benjamin Smedberg]

Comment on attachment 197546 [details] [diff] [review]
patch1: nsMsgComposeService::Handle

We don't want to handle non-flag arguments (bare URIs) in nsMsgComposeService.
the maildefaulthandler is the correct place for this code.

Comment 18

[Benjamin Smedberg]

Attachment: Handle URIs happily

This is entirely untested. Tuukka, can you apply and test this patch?

Comment 19

[Tuukka Tolvanen (sp3000)]

Comment on attachment 197548 [details] [diff] [review]
Handle URIs happily

>-    wwatch.openWindow(null, "chrome://messenger/content/", "_blank",
>-                      "chrome,dialog=no,all", argstring);
>+      wwatch.openWindow(null, "chrome://messenger/content/", "_blank",
>+			"chrome,dialog=no,all", argstring);
>+    }

tabs!

      while (i < count) {
...
	++i;
      }
      if (i < count) {
	uri = cmdLine.getArgument(i);
...
      }

no uri from this logic.

	if (uri.match.(/^mailto:/)) {

dot typo kills the handler dead here.

	  do {
	    ++i;
	    var testarg = cmdLine.getArgument(++i);
	    if (testarg.match(/^-/))
	      break;

	    uri += " " + testarg;
	  } while (1);

increments twice per loop and past the end, kills handler

>+      openURI(uri);

openURI can't handle a string

function mayOpenURI(uri)
...
function openURI(uri)
{
  if (!mayOpenURI())

uri undefined in mayOpenURI, dies

with those fixed, works good. tested:
$ thunderbird -compose mailto:yay for spaces '<hello@world>'
$ thunderbird mailto:yay for spaces '<hello@world>'
$ thunderbird -compose
$ thunderbird
$ thunderbird mailto:yay for spaces '<hello@world>' -compose

Comment 20

[Benjamin Smedberg]

Tuukka, can you make the updated patch, since you already fixed it in your tree?

Comment 21

[Tuukka Tolvanen (sp3000)]

Attachment: patch2.1

yeah, cvs-mirror was just being laggy.

Comment 22

[Benjamin Smedberg]

Comment on attachment 197585 [details] [diff] [review]
patch2.1

excellent

Comment 23

[Scott MacGregor]

Comment on attachment 197585 [details] [diff] [review]
patch2.1

thanks guys. 

approving for 1.8b5.

Comment 24

[Tuukka Tolvanen (sp3000)]

Attachment: patch3: wwatch.openWindow

ack. seems I neglected to test with thunderbird not running -- the OpenURI bit
runs through successfully but does nothing, app exits successfully, no compose
window.

The equivalent wwatch.openWindow js to what composeservice c++ handler compose
window launching uses works both when running or not. (should the xxxbsmedberg
nsIURILoader.openURI comment be restored?)

Comment 25

[Tuukka Tolvanen (sp3000)]

Comment on attachment 197585 [details] [diff] [review]
patch2.1

...but messengercompose.xul wouldn't really work for non-mailto: URIs, would
it? I think I'm out of clues on this one now :)

Comment 26

[Benjamin Smedberg]

Comment on attachment 197629 [details] [diff] [review]
patch3: wwatch.openWindow

right, we should continue to support news: and other URIs

Comment 27

[Benjamin Smedberg]

Attachment: Use a loadgroup to manage the lastwindowclosingsurvivalarea, rev. 1

This patch depends on the appstartup changes in bug 310076, and is therefore
not acceptable for the 1.8 branch. I'll try to hack together something for the
branch (probably hard-coding a mailto case). Boris/Darin, I'm mainly interested
in making sure the uriloader/loadgroup/requestlistener bits of this patch make
sense: the other logic has already been reviewed.

Comment 28

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 197851 [details] [diff] [review]
Use a loadgroup to manage the lastwindowclosingsurvivalarea, rev. 1

Yeah, this should work...   Remove that tab in the GetInterface method, though?
;)

It might be worth filing a bug (cc me, darin, biesi) to talk about whether we
should have some sort of better setup for this.

Comment 29

[Darin Fisher]

Comment on attachment 197851 [details] [diff] [review]
Use a loadgroup to manage the lastwindowclosingsurvivalarea, rev. 1

sr=darin

Comment 30

[Benjamin Smedberg]

Attachment: Branch version hacks mailto specifically, rev. 1

Comment 31

[Scott MacGregor]

Comment on attachment 197993 [details] [diff] [review]
Branch version hacks mailto specifically, rev. 1

I'll be testing this on Windows and Mac today to make sure it didn't regress
anything. Tuukka, can you review and give this a spin on Linux?

Comment 32

[Scott MacGregor]

Benjamin, let's go ahead and get this checked into the trunk and the branch,
we'll get tuuka's testing feedback and more from others who can help if we get
this in sooner rather than later. Thanks again for the patch.

Comment 33

[Tuukka Tolvanen (sp3000)]

Comment on attachment 197993 [details] [diff] [review]
Branch version hacks mailto specifically, rev. 1

a few issues: thunderbird with no args does not open any window; run with one
uri arg, opens up to 3 windows. Works to the extent that all args do open the
expected window whether in a new or existing process (mailto: and news: URIs
tested)

>+        if (!curarg.match(/^-/))
>+	  break;

tab

>-    if (!uri && cmdLine.state != nsICommandLine.STATE_INITIAL_LAUNCH) {
>-      try {
>-        var wmed = Components.classes["@mozilla.org/appshell/window-mediator;1"]
>-                             .getService(nsIWindowMediator);
>+    if (cmdLine.state != nsICommandLine.STATE_INITIAL_LAUNCH) {
>+      if (uri) {
>+        openURI(cmdLine.resolveURI(uri));
>+      }
>+      else {
...
>       }

should return here

>     }

Comment 34

[Tuukka Tolvanen (sp3000)]

>+    if (uri.match(/^mailto:/)) {

fails with no uri; should be if (uri && uri.match(/^mailto:/)) {

With the missing return and that fix, looks good, except for a superfluous
untargeted main window with a news: newsgroup uri in a new process; news:
message-id uri seems to fail in an existing process, but I suppose that's unrelated.

Comment 35

[Benjamin Smedberg]

Attachment: Final branch patch for checkin

Comment 36

[Benjamin Smedberg]

OK, the branch version has been checked in. I have *not* checked in anything on
trunk, waiting for the more intrusive patch from bug 310076 so that we can fix
this "right" using uriloader in all cases.

Comment 37

[Benjamin Smedberg]

Fixed on trunk with the full uriloader patch.