Closed Bug 281847 Opened 15 years ago Closed 14 years ago

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

Categories

(Thunderbird :: General, defect, major)

x86
Linux
defect
Not set
major

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird1.1

People

(Reporter: bugzilla, Assigned: benjamin)

References

()

Details

(Keywords: fixed1.8, regression)

Attachments

(2 files, 5 obsolete files)

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.
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...
Flags: blocking-aviary1.1?
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.
Flags: blocking-aviary1.1?
Target Milestone: --- → Thunderbird1.1
changing summary
Summary: clicking mailto: link and "send link" fail to bring up mail compose → Linux: clicking mailto: link and "send link" fail to bring up mail compose
Duplicate of bug 227786?
you guys still seeing this problem?
(In reply to comment #5)
> you guys still seeing this problem?

version 1.6a1 (20050824)

Yep.
Depends on: 227786
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.
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.
*** Bug 309933 has been marked as a duplicate of this bug. ***
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:foopy@test.this
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:foopy@test.this
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.
Flags: blocking1.8b5?
*** Bug 310001 has been marked as a duplicate of this bug. ***
this is already scheduled for 1.1
Flags: blocking1.8b5?
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.
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:foo@foo.com

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.
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).
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.
Attachment #197546 - Flags: superreview?(mscott)
Attachment #197546 - Flags: review?(benjamin)
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.
Attachment #197546 - Flags: superreview?(mscott)
Attachment #197546 - Flags: review?(benjamin)
Attachment #197546 - Flags: review-
Attached patch Handle URIs happily (obsolete) — Splinter Review
This is entirely untested. Tuukka, can you apply and test this patch?
Assignee: mscott → benjamin
Attachment #197546 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #197548 - Flags: superreview?(mscott)
Attachment #197548 - Flags: review?(tuukka.tolvanen)
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
Attachment #197548 - Attachment is obsolete: true
Attachment #197548 - Flags: superreview?(mscott)
Attachment #197548 - Flags: review?(tuukka.tolvanen)
Attachment #197548 - Flags: review-
Tuukka, can you make the updated patch, since you already fixed it in your tree?
Attached patch patch2.1 (obsolete) — Splinter Review
yeah, cvs-mirror was just being laggy.
Attachment #197585 - Flags: superreview?(mscott)
Attachment #197585 - Flags: review?(benjamin)
Comment on attachment 197585 [details] [diff] [review]
patch2.1

excellent
Attachment #197585 - Flags: review?(benjamin) → review+
Assignee: benjamin → tuukka.tolvanen
Status: ASSIGNED → NEW
Status: NEW → ASSIGNED
Flags: blocking1.8b5?
Whiteboard: has patch, needs review (mscott)
Comment on attachment 197585 [details] [diff] [review]
patch2.1

thanks guys. 

approving for 1.8b5.
Attachment #197585 - Flags: superreview?(mscott)
Attachment #197585 - Flags: superreview+
Attachment #197585 - Flags: approval1.8b5+
Flags: blocking1.8b5? → blocking1.8b5+
Whiteboard: has patch, needs review (mscott)
Attached patch patch3: wwatch.openWindow (obsolete) — Splinter Review
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?)
Attachment #197585 - Attachment is obsolete: true
Attachment #197629 - Flags: superreview?(mscott)
Attachment #197629 - Flags: review?(benjamin)
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 :)
Attachment #197585 - Attachment is obsolete: false
Attachment #197585 - Attachment is obsolete: true
Attachment #197585 - Flags: approval1.8b5+
Comment on attachment 197629 [details] [diff] [review]
patch3: wwatch.openWindow

right, we should continue to support news: and other URIs
Attachment #197629 - Flags: superreview?(mscott)
Attachment #197629 - Flags: review?(benjamin)
Attachment #197629 - Flags: review-
Depends on: 310076
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.
Assignee: tuukka.tolvanen → benjamin
Attachment #197851 - Flags: superreview?(darin)
Attachment #197851 - Flags: review?(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.
Attachment #197851 - Flags: review?(bzbarsky) → review+
Comment on attachment 197851 [details] [diff] [review]
Use a loadgroup to manage the lastwindowclosingsurvivalarea, rev. 1

sr=darin
Attachment #197851 - Flags: superreview?(darin) → superreview+
Attachment #197993 - Flags: superreview?(mscott)
Attachment #197993 - Flags: review?(tuukka.tolvanen)
Attachment #197629 - Attachment is obsolete: true
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?
Attachment #197993 - Flags: superreview?(mscott)
Attachment #197993 - Flags: superreview+
Attachment #197993 - Flags: approval1.8b5+
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 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

>     }
Attachment #197993 - Flags: review?(tuukka.tolvanen) → review-
>+    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.
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.
Keywords: fixed1.8
Attachment #197993 - Attachment is obsolete: true
Fixed on trunk with the full uriloader patch.
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.