373954 - Cannot send any attachment files which are attached in original message when I forward a mail as inline

Status: VERIFIED FIXED

(MailNews Core :: Internationalization, defect)

Description

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

If there are these conditions, I cannot send a forwarding mail.

1. the forwarded message is inline.
2. the forwarded mail has attachment file.
3. if the user name of Windows has non-ASCII characters.

I think that this is a regression of bug 278161. Now we can see the assertion at that time.
http://lxr.mozilla.org/mozilla1.8/source/mailnews/compose/src/nsMsgSend.cpp#2328

Because in mime_draft_process_attachments, 
http://lxr.mozilla.org/mozilla1.8/source/mailnews/mime/src/mimedrft.cpp#548
tmpFile->orig_url has native encoded URI spec.

Comment 1

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Patch rv1.0

This patch fixes this bug, but I don't know whether the risk is low.

Comment 2

[Scott MacGregor]

Thanks for jumping in here Masayuki,

One quick comment, I think you can use:

NS_GetURLSpecFromFile http://mxr.mozilla.org/seamonkey/source/netwerk/base/public/nsNetUtil.h#676

instead of this: 


+    nsCOMPtr<nsIIOService> ios =
+      do_GetService("@mozilla.org/network/io-service;1");
+    if (!ios)
+      return MIME_OUT_OF_MEMORY;
+    nsCOMPtr<nsIProtocolHandler> ph;
+    ios->GetProtocolHandler("file", getter_AddRefs(ph));
+    if (!ph)
+      return MIME_OUT_OF_MEMORY;
+    nsCOMPtr<nsIFileProtocolHandler> fph = do_QueryInterface(ph);
+    nsCAutoString fileURL;
+    fph->GetURLSpecFromFile(tmpFile, fileURL);

Comment 3

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Patch rv1.1

Thank you Scott for your advice.

Comment 4

[David :Bienvenu]

Comment on attachment 258599 [details] [diff] [review]
Patch rv1.1

thx for the patch!

two nits:

you don't need this:

+      nsCOMPtr<nsIFile> tmpFile = do_QueryInterface(lf);


nsILocalFile inherits from nsIFile, so you can use lf directly.

and

no need for the braces here:

+      {
+        nsMimeNewURI(getter_AddRefs(newAttachment->orig_url),
+                     fileURL.get(), nsnull);
+      }

r/sr=bienvenu, with those two nits...

Comment 5

[David :Bienvenu]

I did step through this code for the ascii case and it still works fine.

Comment 6

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Patch rv1.2

Comment 7

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

checked-in to trunk, thanks.

Comment 8

[Scott MacGregor]

Comment on attachment 258657 [details] [diff] [review]
Patch rv1.2

Thank you Masayuki.

Comment 9

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

checked-in to branch too.

Comment 10

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

-> v. on trunk and 1.8branch