+++ This bug was initially created as a clone of Bug #736571 +++ Today's trunk build, using the plain-text editor (don't know if it matters). 1. Select Message > Forward As > Attachment, composition window opens. 2. Attachment is in the attachment pane and looks normal. 3. Try to send or save as draft. 4. See the error message that "subject.eml" couldn't be attached. Looks like another fall-out from the "attachment in cloud" patch, but I may be wrong in that assumption (i.e., didn't try the respective earlier nightly). Thanks to L.A.R. Grizzly for reporting this bug on MozillaZine.

Here's a link to my post on Mozillazine if interested: http://forums.mozillazine.org/viewtopic.php?p=11831885#p11831885

Works fine for me on the very latest trunk build.

yeah, works fine here too with daily build.

So, what might make the difference... I can reproduce with several messages from my Local Folders, with or without a Subject being present. My default composition mode is plain text. That's a profile migrated from 10.0 and earlier, i.e., that wasn't a fresh profile. I get the following in the Error Console after dismissing the error dialog: Error: GenericSendMessage FAILED: [Exception... "Component returned failure code: 0x8055311a [nsIMsgCompose.SendMsg]" nsresult: "0x8055311a (<unknown>)" location: "JS frame :: chrome://messenger/content/messengercompose/MsgComposeCommands.js :: GenericSendMessage :: line 2823" data: no] Source File: chrome://messenger/content/messengercompose/MsgComposeCommands.js Line: 2826 No add-ons other than Test Pilot (enabled or disabled doesn't make a difference).

Composition mode doesn't seem to make a dif With or without .eml Same error message here, using file>>send later, rather than actually sending. Testing with: Mozilla/5.0 (Windows NT 5.1; rv:14.0) Gecko/20120314 Thunderbird/14.0a1 ID:20120314030025 Solid failure, regardless of content.

Testing Thunderbird/14.0a1 BuildID=20120317030033 on Windows 7 (64 bit) here. I have neither Cloud nor IM set up, but haven't disabled it either.

This only seems to apply to Windows with POP3 messages.

I didn't test on Linux or directly with an unsynchronized IMAP message, so let's declare it Windows-only for now. Maybe that's some drive-letter path issue?

Checked with trunk builds on Win-XP. (Problem-A) Local mail folder, Subject: ABC. Internal url of attached mail in composition window. (#3247 = Offset) > mailbox-message://x@x.x.x/Inbox/Mail-Forward-Test#3247 Save As Draft faild with following alert. > Save Draft Error > Unable to save your message as draft. > There was an error attaching ABC.eml. Please check if you have access to the file. > [ OK ] After click OK, same exception as comment #4 was shown at Error Console. (Line No. depends on build) Regression Window of (Problem-A) : No problem. 2012-03-07-03-00-33-comm-central/thunderbird-13.0a1.en-US.win32.zip Mozilla/5.0 (Windows NT 5.1; rv:13.0) Gecko/20120307 Thunderbird/13.0a1 Problem started to occur. 2012-03-08-03-00-48-comm-central/thunderbird-13.0a1.en-US.win32.zip Mozilla/5.0 (Windows NT 5.1; rv:13.0) Gecko/20120308 Thunderbird/13.0a1 comm-central - pushlog, From: 2012-03-07 02:00:00 To: 2012-03-08 04:00:00 http://hg.mozilla.org/comm-central/pushloghtml?startdate=2012-03-07+02%3A00%3A00&enddate=2012-03-08+04%3A00%3A00 (Problem-B) IMAP mail folder, Subject: ABC. (both offline-use=On and offlin-use=Off) Internal url of attached mail in composition window. (#12125 = UID) > imap-message://yatter.one%40gmail.com@imap.gmail.com/INBOX#12125 Save As Draft was executed normally. But following error was reported to Error Console. > Timestamp: 2012/03/21 09:44:12 > Error: bundle is not defined > Source File: chrome://messenger/content/messengercompose/MsgComposeCommands.js Line: 4337 > (Line No. depends on build) "dialog after save" is enabled at Copies&Folders in my prefs. Problem-B occurs in 20120307 build. So different regresson from Problem-A.

(In reply to WADA from comment #9) > Regression Window of (Problem-A) : > No problem. > 2012-03-07-03-00-33-comm-central/thunderbird-13.0a1.en-US.win32.zip > Mozilla/5.0 (Windows NT 5.1; rv:13.0) Gecko/20120307 Thunderbird/13.0a1 > Problem started to occur. > 2012-03-08-03-00-48-comm-central/thunderbird-13.0a1.en-US.win32.zip > Mozilla/5.0 (Windows NT 5.1; rv:13.0) Gecko/20120308 Thunderbird/13.0a1 Thanks WADA, that likely eliminates BigFiles as the source of this regression. On the other hand, nothing in the pushlog catches my eyes as a possible cause...

this is backing out part of Bug 186724 - SetSpec fails because nsMailboxUrl::ParseUrl() fails with a file:///test url, because fileURL->GetFile(getter_AddRefs(fileURLFile)) fails with a non-existent file. This failure is harmless, and lets forward as attachment work again.

Comment on attachment 608448 [details] [diff] [review] proposed fix > nsCOMPtr<nsIURI> aURL; > rv = messageService->GetUrlForUri(uri.get(), getter_AddRefs(aURL), nsnull); >+ // we ignore errors with SetSpec because attached eml files seem >+ // to cause issues. >+ if (aURL) >+ (void) aURL->SetSpec(nsDependentCString(uri.get())); > > rv = NS_NewInputStreamChannel(getter_AddRefs(m_converter_channel), aURL, nsnull); Is there any chance you can explain what this code is doing? In particular, why is it using GetUrlForUri but then trying to change the spec back? Also, nsDependentCString(.get()) is a really silly thing to do ;-)

(In reply to neil@parkwaycc.co.uk from comment #12) > Comment on attachment 608448 [details] [diff] [review] > proposed fix > > > nsCOMPtr<nsIURI> aURL; > > rv = messageService->GetUrlForUri(uri.get(), getter_AddRefs(aURL), nsnull); > >+ // we ignore errors with SetSpec because attached eml files seem > >+ // to cause issues. > >+ if (aURL) > >+ (void) aURL->SetSpec(nsDependentCString(uri.get())); > > > > rv = NS_NewInputStreamChannel(getter_AddRefs(m_converter_channel), aURL, nsnull); > Is there any chance you can explain what this code is doing? In particular, > why is it using GetUrlForUri but then trying to change the spec back? > the spec can be significantly different after GetUrlForUri is done with it, since nsMailboxService::PrepareMessageUrl can munge it. That function leaves originalSpec set to the original uri. I'm not sure what code downstream cares. I can try without it, but that's even scarier than the initial error handling change was :-(

Comment on attachment 608448 [details] [diff] [review] proposed fix Review of attachment 608448 [details] [diff] [review]: ----------------------------------------------------------------- ::: mailnews/compose/src/nsMsgAttachmentHandler.cpp @@ +589,5 @@ > rv = messageService->GetUrlForUri(uri.get(), getter_AddRefs(aURL), nsnull); > + // we ignore errors with SetSpec because attached eml files seem > + // to cause issues. > + if (aURL) > + (void) aURL->SetSpec(nsDependentCString(uri.get())); Comment should be inside the if.

(In reply to Serge Gautherie (:sgautherie) from comment #14) > Comment on attachment 608448 [details] [diff] [review] > proposed fix > > Review of attachment 608448 [details] [diff] [review]: > ----------------------------------------------------------------- > > ::: mailnews/compose/src/nsMsgAttachmentHandler.cpp > @@ +589,5 @@ > > rv = messageService->GetUrlForUri(uri.get(), getter_AddRefs(aURL), nsnull); > > + // we ignore errors with SetSpec because attached eml files seem > > + // to cause issues. > > + if (aURL) > > + (void) aURL->SetSpec(nsDependentCString(uri.get())); > > Comment should be inside the if. I think that's less readable, and ugly, so, no.

(In reply to David :Bienvenu from comment #11) > SetSpec fails because > nsMailboxUrl::ParseUrl() fails with a file:///test url, because > fileURL->GetFile(getter_AddRefs(fileURLFile)) fails with a non-existent > file. Should the comment in the patch be less vague? Should we special-case .eml files? Could an automatic test be added?

The first question to answer is Neil's - perhaps we don't have to change the spec back, since failure to do so doesn't cause forward as attachment to fail. We can do a mozmill test that forwards as attachment, and then attempts to save a local draft, to reproduce this issue.

so, FTR, we do have a mozmill test that covers this.

the SetSpec call was added by the patch in bug 161488, which had to do with decoding s/mime messages before forwarding. I suspect we can just remove the setspec call since forwarding as attachment works w/o it. I'll have to see if I can find an encrypted message to make sure we're not regressing bug 161488.

I think we can get rid of the setspec call entirely.

Comment on attachment 609522 [details] [diff] [review] remove setspec code completely > rv = messageService->GetUrlForUri(uri.get(), getter_AddRefs(aURL), nsnull); [I wonder why kaie never checked this rv]

fixed on trunk - http://hg.mozilla.org/comm-central/rev/5bff4f51bfed

Comment on attachment 609522 [details] [diff] [review] remove setspec code completely [Approval Request Comment] Regression caused by (bug #): User impact if declined: Testing completed (on c-c, etc.): Risk to taking this patch (and alternatives if risky):

(In reply to neil@parkwaycc.co.uk from bug 736789 comment #22) > > rv = messageService->GetUrlForUri(uri.get(), getter_AddRefs(aURL), nsnull); > [I wonder why kaie never checked this rv] I filed Bug 739616.