366482 - "Edit as new" for opened eml files results in a broken compose window

Status: RESOLVED FIXED

(MailNews Core :: Composition, defect)

Description

[Henrik Skupin [:whimboo][⌚️UTC+1]]

version 2 beta 1 (20070108) win32

This is similar to bug 307672 which is for SeaMonkey. I can get the same exception and can reproduce it each time. I think this could help us to find also a solution for SeaMonkey. What you have to do:

First start Thunderbird, open the testcase and double click the eml attachment which will be displayed in a new window. Inside this window choose "Message | Edit message as new" which results in the error message "An error occurred while creating a message compose window. Please try again." The compose window comes up for a very short time and is closed immediately. Close the message window with the opened message because we don't need it anymore.

Now you only have to open a new compose window or try to reply a message. Here I get a js alert "The URL is not valid and cannot be loaded.". The window opens but it is not styled and produces error messages and exceptions. If you can't see the errors for the first time close the current compose window and try to open a new one again.

Error: [Exception... "Component returned failure code: 0x80004003 (NS_ERROR_INVALID_POINTER) [nsIMsgCompose.initEditor]"  nsresult: "0x80004003 (NS_ERROR_INVALID_POINTER)"  location: "JS frame :: chrome://messenger/content/messengercompose/MsgComposeCommands.js :: InitEditor :: line 3547"  data: no]
Source File: chrome://messenger/content/messengercompose/MsgComposeCommands.js
Line: 3547

Error: gMsgCompose has no properties
Source File: chrome://messenger/content/messengercompose/MsgComposeCommands.js
Line: 2461

The funniest part is, that I'm unable to get a 'L' inside the address field or the subject. If you reply a message and the subject contains some 'l' letters they will be converted to ','. Further any new lines from text inside the body will be converted to '>l'.

The compose window is not usable anymore until you restart Thunderbird.

Comment 1

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Attachment: Open eml file will break the compose window

Comment 2

[Mike Cowperthwaite]

I think the 'gMsgCompose has no properties' error is from something else; that occurs when you close the window.

I didn't get that, but I did get the NS_ERROR_INVALID_POINTER problem when trying to Edit As New into a cached compose window.  No error appears in the Error Console if you try to Edit As New into a new compose window.

Also, that error only occurs (in my early testing, at least) when the cached compose window is plain text.  If it's HTML, I only see the messagebox with "Error creating compose window" in it.

Comment 3

[Mike Cowperthwaite]

Reproduced with TB 3a1-0108.

In Seamonkey 1.5a, I get a different message in the error console:
==========
Error: [Exception... "Component returned failure code: 0x80040154 (NS_ERROR_FACTORY_NOT_REGISTERED) [nsIMsgComposeService.InitCompose]"  nsresult: "0x80040154 (NS_ERROR_FACTORY_NOT_REGISTERED)"  location: "JS frame :: chrome://messenger/content/messengercompose/MsgComposeCommands.js :: ComposeStartup :: line 1315"  data: no]
Source File: chrome://messenger/content/messengercompose/MsgComposeCommands.js
Line: 1315
==========

Comment 4

[Henrik Skupin [:whimboo][⌚️UTC+1]]

I see the same exception on some situation but not every time. Clicking multiple times on the eml attachment just raises an "mailbox is not a registered protocol" error! *sigh*

Comment 5

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Attachment: screenshot of broken compose window

Thats the reply window after the eml attachment was opened:

- all 'l' letters are visible as ','
- no cursor is available
- not styled correctly
- body is empty

Comment 6

[Bruno 'Aqualon' Escherl]

The testcase in Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.2pre)
Gecko/20061223 SeaMonkey/1.1 shows the following error in -console (command
line):

EX: = [Exception... "Component returned failure code: 0x80004005
(NS_ERROR_FAILURE)
[nsIMsgComposeService.InitCompose]"  nsresult: "0x80004005 (NS_ERROR_FAILURE)"
location: "JS frame ::
chrome://messenger/content/messengercompose/MsgComposeCommands.js ::
ComposeStartup :: line 1303"  data: no]

When mail.compose.max_recycled_windows is set to 1, I get the error in -console
and error console and a Window with greyed out subject and to fields (like the
one described in bug 307672). At the next try to Edit as New I get the "An
error occurred while creating a message compose window. Please try again."
error dialog like Henrik in Thunderbird. With mail.compose.max_recycled_windows
set to 0 I get the error dialog at first try.

The difference to Thunderbird is that composing a new mail works fine in
SeaMonkey after those errors.

Comment 7

[Bruno 'Aqualon' Escherl]

(In reply to comment #6)

> When mail.compose.max_recycled_windows is set to 1, I get the error in -console
> and error console...
That's wrong, I only see it in -console at first try, error console only shows it, when the error dialog appears.

(In reply to comment #3)

> In Seamonkey 1.5a, I get a different message in the error console:
> ==========
> Error: [Exception... "Component returned failure code: 0x80040154
> (NS_ERROR_FACTORY_NOT_REGISTERED) [nsIMsgComposeService.InitCompose]" 
> nsresult: "0x80040154 (NS_ERROR_FACTORY_NOT_REGISTERED)"  location: "JS frame
> :: chrome://messenger/content/messengercompose/MsgComposeCommands.js ::
> ComposeStartup :: line 1315"  data: no]
> Source File: chrome://messenger/content/messengercompose/MsgComposeCommands.js
> Line: 1315
> ==========

I see this too in suiterunner, the code where it occurs is the same like in 1.1: gMsgCompose = sMsgComposeService.InitCompose(window, params);

Comment 8

[Henrik Skupin [:whimboo][⌚️UTC+1]]

As I described in comment 0 this bug is reported explicitly for Thunderbird and not SeaMonkey while this is covered in bug 307672. MsgComposeCommands.js is forked so will get a different behavior in SeaMonkey.

http://landfill.mozilla.org/mxr-test/mozilla/find?string=MsgComposeCommands.js

If you like to create a new bug for SeaMonkey feel free but I don't think its good to watch both products at the same time. The results are too different.

Comment 9

[Ian Neal]

I think the problem is nothing to do with the front end but the back end.

In nsMsgCompose::CreateMessage http://lxr.mozilla.org/seamonkey/source/mailnews/compose/src/nsMsgCompose.cpp#1729
because the originalMsgURI used to contain &type=application/x-message-display (which was removed and &realtype=message/rfc822 appended at the end), rv = GetMsgDBHdrFromURI(uri, getter_AddRefs(msgHdr)); returns a failure which means CreateMessage fails.

We should probably remove the NS_ENSURE_SUCCESS, check for a failure and return NS_OK or remove the NS_ENSURE_SUCCESS, check for a success as well as msgHdr and at the end of the function (about line 1983) return NS_OK or something along those lines. I would say the latter because the former would probably cause leakage and there may be multiple uris passed through to be dealt with.

Comment 10

[Ian Neal]

Attachment: Add origMsgHdr to stream patch v0.1 (comm-central checkin: Comment 19, cvs checkin: Comment 20)

This patch:
* Passes msgHdr into msgComposeService.OpenComposeWindow removing need to refetch it
* Passes msgHdr into LoadDraftOrTemplate so it can be added to the stream
* Passes msgHdr into CreateTheComposeWindow so it can be added to the compose params

Comment 11

[Ian Neal]

Just as a note this patch resolves the issue in comment 9 as mOrigMsgHdr is now defined, so there is no attempt to try a fetch the msgHdr. Whether there is a way of making sure that when CreateMessage is called that it is always defined and then we are able to removed the fetching part completely is a different matter.

Comment 12

[David :Bienvenu]

Ian - thx for tackling this.

Is adding Requires mailnews to cthandlers this needed for this bug?

+		  mailnews \

An alternate way of fixing this might be to change the mimedrft code to get the msg window from the msg url, a la this code:

http://mxr.mozilla.org/mailnews/source/mailnews/mime/src/mimecms.cpp#451

get the dummy msg hdr from there, and set the orig msg hdr from that, if we can't get it from the original msg uri.

But I'm OK with your fix as well. Just let me know about the cthandler part of the patch...

Comment 13

[David :Bienvenu]

Comment on attachment 266817 [details] [diff] [review]
Add origMsgHdr to stream patch v0.1 (comm-central checkin: Comment 19, cvs checkin: Comment 20)

r=bienvenu, pending the question about the makefile.

Comment 14

[Ian Neal]

(In reply to comment #12)
> Ian - thx for tackling this.
> 
> Is adding Requires mailnews to cthandlers this needed for this bug?
> 
> +                 mailnews \
> 
Without the above, when building you get the following:
In file included from ../../../../dist/include/mime/nsIMimeStreamConverter.h:26,
                 from ../../../../dist/include/mime/modlmime.h:48,
                 from ../../../../dist/include/mime/mimehdrs.h:41,
                 from ../../../../dist/include/mime/mimei.h:231,
                 from ../../../../dist/include/mime/mimecth.h:48,
                 from ../../../../dist/include/mime/nsIMimeContentTypeHandler.h:59,
                 from /mozdev/bug366482/mozilla/mailnews/mime/cthandlers/glue/nsMimeContentTypeHandler.h:54,
                 from /mozdev/bug366482/mozilla/mailnews/mime/cthandlers/glue/nsMimeContentTypeHandler.cpp:43:
../../../../dist/include/msgbase/nsIMsgHdr.h:14:28: error: MailNewsTypes2.h: No such file or directory
../../../../dist/include/msgbase/nsIMsgHdr.h:21:27: error: MailNewsTypes.h: No such file or directory
../../../../dist/include/msgbase/nsIMsgHdr.h:71: error: ‘nsMsgPriorityValue’ has not been declared
../../../../dist/include/msgbase/nsIMsgHdr.h:72: error: ‘nsMsgPriorityValue’ has not been declared
../../../../dist/include/msgbase/nsIMsgHdr.h:88: error: ‘nsMsgKey’ has not been declared
../../../../dist/include/msgbase/nsIMsgHdr.h:89: error: ‘nsMsgKey’ has not been declared
../../../../dist/include/msgbase/nsIMsgHdr.h:92: error: ‘nsMsgKey’ has not been declared
../../../../dist/include/msgbase/nsIMsgHdr.h:93: error: ‘nsMsgKey’ has not been declared
../../../../dist/include/msgbase/nsIMsgHdr.h:96: error: ‘nsMsgKey’ has not been declared
../../../../dist/include/msgbase/nsIMsgHdr.h:97: error: ‘nsMsgKey’ has not been declared
../../../../dist/include/msgbase/nsIMsgHdr.h:184: error: ‘nsMsgLabelValue’ has not been declared
../../../../dist/include/msgbase/nsIMsgHdr.h:185: error: ‘nsMsgLabelValue’ has not been declared
gmake[7]: *** [nsMimeContentTypeHandler.o] Error 1
gmake[7]: Leaving directory `/mozdev/bug366482/mozilla/suite-opt/mailnews/mime/cthandlers/glue'

I am looking at the alternate suggestion at the moment too.

Comment 15

[Worcester12345]

Similar, but on branch:

Bug 392585 – Unable to "edit as new" .EML files attached in an e-mail

Comment 16

[[e-mail]]

Comment on attachment 266817 [details] [diff] [review]
Add origMsgHdr to stream patch v0.1 (comm-central checkin: Comment 19, cvs checkin: Comment 20)

I think the idl change might give Calendar a headache?

Comment 17

[Michiel van Leeuwen (email: mvl+moz@)]

Comment on attachment 266817 [details] [diff] [review]
Add origMsgHdr to stream patch v0.1 (comm-central checkin: Comment 19, cvs checkin: Comment 20)

Daniel, can you look at this? You know the code that calls the compose window from lightning much better then I do...

Comment 18

[Daniel Boelzle [:dbo]]

Comment on attachment 266817 [details] [diff] [review]
Add origMsgHdr to stream patch v0.1 (comm-central checkin: Comment 19, cvs checkin: Comment 20)

looks good for trunk calendar; r=dbo

Comment 19

[Ian Neal]

Comment on attachment 266817 [details] [diff] [review]
Add origMsgHdr to stream patch v0.1 (comm-central checkin: Comment 19, cvs checkin: Comment 20)

http://hg.mozilla.org/comm-central/index.cgi/rev/0137db13395c

Comment 20

[Ian Neal]

Comment on attachment 266817 [details] [diff] [review]
Add origMsgHdr to stream patch v0.1 (comm-central checkin: Comment 19, cvs checkin: Comment 20)

Checking in
sun-calendar-event-dialog.js; new revision: 1.83; previous revision: 1.82
done

Comment 21

[Ian Neal]

http://hg.mozilla.org/comm-central/index.cgi/rev/3f93f937cce7 to fix missing comma from mail/base/content/mailOverlay.xul in pushed versus reviewed patch

Comment 22

[Joe Sabash [:JoeS1]]

Close,but no "cuppie doll" on this one.

The compose window does in fact open, but inline image url's are munged and are not available in the new composition.
IE:<img alt=""
 src="file:///C:/saveas/test%20eml%20image.eml?fetchCompleteMessage=true&amp;part=1.2&amp;filename=tb16a1.gif"
 height="79" width="290">
Does not display in the compose window on a saved eml.
So the eml format is still worthless for saving inline image content.

Tested with:
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1a2pre) Gecko/20080823161750 Shredder/3.0b1pre ID:20080823161750

Comment 23

[Ian Neal]

Could you attach an example?

Comment 24

[Mike Cowperthwaite]

I don't think comment 22 is related to this bug; it looks like bug 351109 to me.

Comment 25

[Joe Sabash [:JoeS1]]

Attachment: eml with inline image

Comment 26

[Joe Sabash [:JoeS1]]

(In reply to comment #24)
> I don't think comment 22 is related to this bug; it looks like bug 351109 to
> me.
> 
Probably so Mike.
I usually can edit around those munged img url's but no luck on this one.
Good to see you posting again.