217328 - Send Link ... incorrectly cuts url after first variable

Status: VERIFIED FIXED

(SeaMonkey :: UI Design, defect)

Description

[Jesper Hertel]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.4) Gecko/20030624
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.4) Gecko/20030624

When using the "send link" function in urls like the above, the link is cut
before the &, giving an incorrect URL in the email message.

Reproducible: Always

Steps to Reproduce:
1. Go to the mentioned url.
2. Select File | Send link...
3. Look at the url in the mail.

Actual Results:  
The url in the mail is missing "&short_desc_type=allwords".

Expected Results:  
The url in the mail should include "&short_desc_type=allwords".

Related bugs (in other products):

Bug 181090 (I copied the bug text from this one)
Bug 216170

Comment 1

[Hermann Schwab]

wfm Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.5b) Gecko/20030826

opened URL in a new tab, sent it to my POP3 Account, opened the received link in
a new tab, identical.

Comment 2

[Hermann Schwab]

wfm Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.4) Gecko/20030624

my mail is configured to send plain text, what is your configuration?

Comment 3

[Jesper Hertel]

I use Outlook 2000 as e-mail client. The e-mail message that I get from Send
link is in RTF format. But I don't think that this is the problem. 

Whether I choose HTML, RTF or plain text as default for new messages, I always
get RTF messages when they are created with a mailto: url or with File | Send
link from Mozilla (which probably uses mailto?). 

So File | Send link gives me an RTF mail with the URL cut off at the same place
(and the space in the URL is also not escaped as %20). 

When I use Send link on the given URL (that is,
http://mecha.mozilla.org/webtools/bugzilla/buglist.cgi?short_desc=send%20link&short_desc_type=allwords):

I get this body:
"http://mecha.mozilla.org/webtools/bugzilla/buglist.cgi?short_desc=send link".

When I choose Start | Run and enter this url:

  mailto:[e-mail]?subject=Comments&body=This%20is%20the%20body

I get this body: "This is the body".


This url:

  mailto:[e-mail]?subject=Comments&body=This%2520is%2520the%2520body

gives this body: "This%20is%20the%20body".


This url:
 mailto:?subject=Bug%20List&body=http://mecha.mozilla.org/webtools/bugzilla/buglist.cgi?short_desc=send%20link&short_desc_type=allwords

gives this body:
"http://mecha.mozilla.org/webtools/bugzilla/buglist.cgi?short_desc=send link"

It seems like the "&" character in the http url (just before "short_desc_type")
is not properly escaped by Mozilla.

This url:

mailto:?subject=Bug%20List&body=http://mecha.mozilla.org/webtools/bugzilla/buglist.cgi?short_desc=send%2520link%26short_desc_type=allwords

gives this body:
"http://mecha.mozilla.org/webtools/bugzilla/buglist.cgi?short_desc=send%20link&short_desc_type=allwords"
which is exactly what Send link should give when used on the given URL.

So, in the http URL part of the mailto URL, space should be escaped as %2520
(double escape), and & (in the http URL part) should be escaped as %26. 

There may be other characters that should be escaped, but for the given URL,
doing these escapes seems to do the trick.

Comment 4

[Hermann Schwab]

related, fixed 2002-11-21, but Product: Camino, not Mozilla:
Bug 181090 send link incorrectly cuts url after first variable

Comment 5

[Jesper Hertel]

In short, it looks like the http URL part of the mailto URL is only single
escaped now, but it should be double escaped.

I do not know the source code, but somewhere is must say something like

mailto_url = "mailto:?subject=" + quote(subject) + "&body=" + quote(url)

which should be

mailto_url = "mailto:?subject=" + quote(subject) + "&body=" + quote(quote(url))

expressed in pseudocode (actually Python here :O) ), where quote() is a function
that url-encodes the argument (e.g. quote(" ") -> "%20").

Comment 6

[Jesper Hertel]

I asked Simon Fraser, who fixed the similar bug in Camino
(http://bugzilla.mozilla.org/show_bug.cgi?id=181090#c2), but he said Camino has
its own Send Link code and didn't think his solution would be of much help in
Mozilla.

Comment 7

[Jon Henry]

Can anyone still reproduce this bug in the browser suite (SeaMonkey)? I tried
with the 20030917 build and things worked as expected. However, Firebird and
Thunderbird do not behave correctly, and leave off the
"&short_desc_type=allwords" as described in this bug. If this is really now
confined only to Firebird, it is a dupe of bug 216170. Anyone who has any input
one way or the other, please speak up. Thanks.

Comment 8

[Jesper Hertel]

I am unsure what you mean by "the browser suite (SeaMonkey)". Is it Mozilla? I
use Mozilla 1.4 without the mail client, that is, browser only, to be able to
use Outlook as mail client, so I don't understand the "suite" thing.

Could you give me (and others listening maybe) a URL to the version I should
try, so I am sure I am checking the right version of the right software?

Comment 9

[Jon Henry]

-> XP Apps

Sorry if that was confusing, I am indeed referring to SeaMonkey when I say
"browser suite".

Anyway, I did some more testing and I can now confirm that this problem does
happen in all situations except when sending a link from the SeaMonkey browser
to the SeaMonkey mail client. When sending a link from Firebird or SeaMonkey
browser, all mail programs except SeaMonkey mail (including Outlook and
Thunderbird), it will leave off the "&short_desc_type=allwords". Strangely, a
link sent between SeaMonkey browser and mail *does* come out correctly.

Comment 10

[Jesper Hertel]

The problem is still there in Mozilla 1.5 RC2 ("Mozilla/5.0 (Windows; U; Windows
NT 5.0; en-US; rv:1.5) Gecko/20030925") - of course, since it hasn't been solved
yet.

Comment 11

[Jesper Hertel]

I tried searching the source code and found something in
http://lxr.mozilla.org/mozilla/source/browser/base/content/browser.js#4173 :

4170 function sendLink(url, title)
4171 {
4172   // generate a mailto url based on the url and the url's title
4173   var mailtoUrl = url ? "mailto:?body=" + url + "&subject=" + escape(title)
: "mailto:";
4174 
4175   var ioService =
Components.classes["@mozilla.org/network/io-service;1"].getService(Components.interfaces.nsIIOService);
4176   var uri = ioService.newURI(mailtoUrl, null, null);
4177 
4178   // now pass this url to the operating system
4179   launchExternalUrl(uri); 
4180 }


I don't know if this function is the code that is actually used for Send Link,
but in any case there is a bug in line 4173: The url is not escaped. The line
probably should be:

4173   var mailtoUrl = url ? "mailto:?body=" + escape(url) + "&subject=" +
escape(title) : "mailto:";

I don't know what the escape() function does (the title doesn't say that much
about which kind of escaping), but in this case it should url-escape the string
in the "right" way, and it seems like this is what is intended with escape(title).

Comment 12

[Jon Henry]

Good catch Jesper, I'd bet that's part of the problem. The "browser" directory
is code specific to Firebird, but there's similar code in SeaMonkey at
http://lxr.mozilla.org/seamonkey/source/xpfe/browser/resources/content/mailNavigatorOverlay.xul#105

I just noticed I didn't assign this bug to the right people when confirming it,
sorry about that, I'll do it now.

Comment 13

[Jesper Hertel]

Ah, thank you! 

I have the problem in my external mail application, Outlook 2000, so I guess it
must be in the function openExternalMailer() at
http://lxr.mozilla.org/seamonkey/source/xpfe/browser/resources/content/mailNavigatorOverlay.xul#66


Line 71 says:

  var mailto = url ? "mailto:?body="+url+"&subject="+escape(title) : "mailto:";

which probably should be:

  var mailto = url ? "mailto:?body="+escape(url)+"&subject="+escape(title) :
"mailto:";

Comment 14

[Jesper Hertel]

In my Mozilla 1.5 RC2 I tried changing the file \Program
Files\mozilla.org\Mozilla15\chrome\comm.jar\content\navigator\mailNavigatorOverlay.xul
with the change in line 71 from comment 13, and it works perfectly now.

Line 71 should be:

var mailto = url ? "mailto:?body="+escape(url)+"&subject="+escape(title) :
"mailto:";

Could someone make that change in CVS? Then I guess this bug would be fixed.

I am sorry I cannot find out how to make a patch file, some day I will learn it.
Hope my human description is okay.

Comment 15

[R.J. Keller]

Attachment: Patch (see comment #13)

[e-mail] requested that I post a patch that makes the changes noted
in comment #13, so here it is.

Comment 16

[Hermann Schwab]

Requesting blocking 1.4.x
This bug has a one-line patch, needs review, is important for users of Outlook.

Index: browser/resources/content/mailNavigatorOverlay.xul
===================================================================
RCS file: /cvsroot/mozilla/xpfe/browser/resources/content/mailNavigatorOverlay.xul,v
retrieving revision 1.4
diff -u -r1.4 mailNavigatorOverlay.xul
--- browser/resources/content/mailNavigatorOverlay.xul	30 May 2003 05:08:36
-0000	1.4
+++ browser/resources/content/mailNavigatorOverlay.xul	5 Oct 2003 00:17:34 -0000
@@ -68,7 +68,7 @@
                                     
.getService(Components.interfaces.nsIExternalProtocolService);
       var ioService = Components.classes["@mozilla.org/network/io-service;1"]
                                 .getService(Components.interfaces.nsIIOService);
-      var mailto = url ? "mailto:?body="+url+"&subject="+escape(title) : "mailto:";
+      var mailto = url ? "mailto:?body="+escape(url)+"&subject="+escape(title)
: "mailto:";
       var uri = ioService.newURI(mailto, null, null);
 
       extProtocolSvc.loadUrl(uri);

Comment 17

[[e-mail]]

Comment on attachment 132649 [details] [diff] [review]
Patch (see comment #13)

/me wonders who timeless planned to ask for superreview

Comment 18

[Jon Henry]

Thanks for the quick r/sr...could someone with the appropriate powers please
check this into CVS? Is it too late in the game to get approval for the
1.5/1.4.1 branches?

Also, am I right in thinking that this will only fix SeaMonkey and not Firebird,
since it's in the toolkit code? If so, I'll go ahead and file one in the
Firebird product assuming there isn't one already.

Comment 19

[R.J. Keller]

Patch checked in.

Comment 20

[Asa Dotzler [:asa]]

Comment on attachment 132649 [details] [diff] [review]
Patch (see comment #13)

1.5 shipped. removing obsolete request.

Comment 21

[Jesper Hertel]

What about version 1.6a? Will the patch automatically be a part of that, or how
does this work? The flags for Patch 132649 only says "approval1.4.2?" but
doesn't mention 1.6.

Comment 22

[Jon Henry]

Verifying fix.

You only need approval to check code in at certain times, such as when a release
is close to shipping and only low-risk or critical patches are being accepted.
When the fix for this bug was checked in, it went straight to 1.6a by default,
since that's the version the "trunk" is currently at. So basically 1.6a, which
is the version of the current nightly builds, will have this fix and 1.5 and
earlier releases will not. Hence the request for approval for the next 1.4.2
release.

Comment 23

[Jesper Hertel]

Thank you, Jon, for the fine explanation.

Comment 24

[Mike Kaply [:mkaply]]

Comment on attachment 132649 [details] [diff] [review]
Patch (see comment #13)

a=mkaply for 1.4.2

Comment 25

[Mike Kaply [:mkaply]]

Anyone interested in putting this on the 1.4 branch and marking fixed1.4.2?

Comment 26

[Mike Kaply [:mkaply]]

I checked this on the branch.