Seen on version 3 alpha 1 (20070304) in safe mode. On TB 2 version 2.0pre (20070305) ctrl-w and then save causes crash - Bug 372705 reproducible: always 1. create blank message (don't need anything in address, body or subject) 2. ctrl-w 3. save, cancel or "don't save" in response to dialog 4. ctrl-w actual results: no dialog in response to ctrl-w of step 4. "save" results in js error, but does save the draft Error: gMsgCompose.getExternalSendListener is not a function Source File: chrome://messenger/content/messengercompose/MsgComposeCommands.js Line: 1056 only way to get rid of compose window after initial ctrl-w is send the message or click X.

but if you have a blank message, cntrl-w shouldn't even bring up a dialog. Do you have a sig file or something? And after step 3, the window should close, so a subsequent ctrl-w shouldn't do anything...

happens same in two imap accounts - one has no signature, the other does.

I tried this with a nightly build from this morning, and it worked fine. Wayne, do you have any extensions installed?

confirming to get on the radar

updated by one day to latest nightly version 3 alpha 1 (20070305), safe mode startup, test profile, one extension in the test profile MR Tech local (plus talkback of course). and no js errors after startup more interesting behavior - ctrl-s, then attemting ctrl-w shows same behavior, ctrl-w doesn't work

Scott says he crashes with a null editor. No idea why I don't, but here's a patch.

file | save as (draft or template) also breaks ctrl-w

and file|close

regressed between 20070224 and 20070228, so caused by bug 312275

still get the js error and other symptoms on trunk from this tinderbox build http://ftp.mozilla.org/pub/mozilla.org/thunderbird/tinderbox-builds/tb-win32-tbox-trunk/ thunderbird-3.0a1.en-US.win32.installer.exe 05-Mar-2007 18:48 6.7M didn't test 2.0 for the crasher

This also affects SM (see bug 372168). I hit this anytime I try to close a message window (except for clicking the "x"). attachment 256449 [details] [diff] [review] removed getExternalSendListener() from the nsIMsgCompose interface (and removed the method implementation), but it's still being called from MsgComposeCommands.js http://lxr.mozilla.org/seamonkey/search?string=getExternalSendListener

Andrew, that's true of the trunk, but not the 2.0 branch...

AFAICT from comments, there is a crash bug and a JS exception bug... perhaps they're not related? (and from the summary and comment 0, this is the JS exception bug although a crash patch is here). I'm not seeing a crash on branch or trunk, and I don't see the exception on the branch.

I think comment 11 and comment 13 have the truth of it, so far.

correct - I just wanted to ensure anyone reading the bug would know the crash fix didn't handle the js error. And I don't think anyone determined the origin of the crash regression.

I do have a trunk patch for this - just need to test it. It's not related to the crash, and it was caused by bug 312275

the same fix will work for SM, I think. I verified that with this patch, nsMsgCompose::OnSendNotPerformed is called. The js assertion about sMsgComposeService still happens - that one baffles me.

Port TB patch, per comment 17. [Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9a3pre) Gecko/20070306 SeaMonkey/1.5a] (nightly) (W2Ksp4) Fixes both bug 372168 and bug 372216, which I'll mark as duplicates.

(In reply to comment #17) > The js assertion about sMsgComposeService still happens - that one baffles me. [Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9a3pre) Gecko/20070306 SeaMonkey/1.5a] (nightly) (W2Ksp4) I see this "new" (= after applying the JS patch) error with SM too. 1. Ctrl+M to open a first Compose window. 2. Ctrl+M to open a second Compose window. 3. Alt+F4 to close either of the window. 4. Alt+F4 to close the remaining window. 4r. {{ Error: sMsgComposeService is not defined Source File: chrome://messenger/content/messengercompose/MsgComposeCommands.js Line: 1085 }} This latter bug may have been around for some time, and may not be related to the current bug !? ... Oh well, this looks much like bug 160942.!.

Comment on attachment 257602 [details] [diff] [review] (Cv1-SM) <MsgComposeCommands.js> As I mentioned in bug 312275 comment 17 this would be easier if nsIMsgCompose inherited from nsIMsgSendListener ...

Comment on attachment 257602 [details] [diff] [review] (Cv1-SM) <MsgComposeCommands.js> > if (gMsgCompose) > { >+ var externalListener = gMsgCompose.QueryInterface(Components.interfaces.nsIMsgSendListener); > if (externalListener) >+ externalListener.onSendNotPerformed(null, Components.results.NS_ERROR_ABORT); > } This isn't the correct way to test for an interface (although gMsgCompose always implements nsIMsgSendListener so it doesn't make any difference). Instead you should write if (gMsgCompose instanceof Components.interfaces.nsIMsgSendListener) gMsgCompose.onSendNotPerformed(null, Components.results.NS_ERROR_ABORT);

(In reply to comment #22) > (From update of attachment 257602 [details] [diff] [review]) > As I mentioned in bug 312275 comment 17 this would be easier if nsIMsgCompose > inherited from nsIMsgSendListener ... (I wouldn't know how to do that: I'll wait and see for David...) (In reply to comment #23) > Instead you should write > if (gMsgCompose instanceof Components.interfaces.nsIMsgSendListener) > gMsgCompose.onSendNotPerformed(null, Components.results.NS_ERROR_ABORT); David, do you agree for TB too !?

Instead of getting the externalSendListener from the gMsgCompose object and sending onSendNotPerfomed directly like: var externalListener = gMsgCompose.getExternalSendListener(); if (externalListener) { externalListener.onSendNotPerformed(null, Components.results.NS_ERROR_ABORT); } you can just call: gMsgCompose.OnSendNotPerformed(messageId, Components.results.NS_ERROR_ABORT); This will notify all registered nsIMsgSendListeners, exactly the same way you do in JavaScript, but then iterating over all listeners. The implementation of nsIMsgCompose has new methods for registering and removing nsIMsgSendListeners: AddMsgSendListener and RemoveMsgSendListener. Do try to send the messageId as the first parameter of this call, since listeners may be interested in that information. Oh, I see now that I'm saying the same as Neil, but then more verbose... sorry. I think it's already implemented as part of bug 312275 and nsMsgCompose does implement nsIMsgSendListener. I can't see this reflected in the idl file though, so that might be the only thing that has to be changed? Maybe nsMsgCompose.h too, to delete the NS_DECL_NSIMSGCOMPOSE?

(In reply to comment #25) >I think it's already implemented as part of bug 312275 and nsMsgCompose does >implement nsIMsgSendListener. I can't see this reflected in the idl file >though, so that might be the only thing that has to be changed? Maybe >nsMsgCompose.h too, to delete the NS_DECL_NSIMSGCOMPOSE? nsMsgCompose implements nsIMsgSendListener, but nsIMsgCompose does not extend nsIMsgSendListener, which is why we can't directly call onSendNotPerformed.

Yes. I think that's what I meant with "I can't see this reflected in the idl file". It has to state something like: interface nsIMsgCompose : nsIMsgSendListener instead of interface nsIMsgCompose : nsISupports I presume the javascript objects are generated from the IDL and that makes calling it impossible, while nsMsgCompse does expose the correct methods.

(In reply to comment #27) >I presume the javascript objects are generated from the IDL and that makes >calling it impossible, while nsMsgCompose does expose the correct methods. It's not impossible, you just have the extra step of getting the secondary IDL associated with the gMsgCompose object so that the extra methods are exposed.

This is my attempt to fix the IDL and nsMsgCompose.h. How does the javascript generation thingy work? Do I have to do something to make it work?

(In reply to comment #29) >How does the javascript generation thingy work? With this patch, it is automatic, and you can just call: gMsgCompose.onSendNotPerformed(messageId, Components.results.NS_ERROR_ABORT); [note lower ^ case in JS]

Comment on attachment 257696 [details] [diff] [review] patch for nsIMsgCompose and nsMsgCompose [Checkin: Comment 32] I can confirm that this patch avoids a call to QueryInterface in the UI.

Comment on attachment 257696 [details] [diff] [review] patch for nsIMsgCompose and nsMsgCompose [Checkin: Comment 32] I landed Matthijs' patch. This bug is FIXED now, right?

Hmm, actually I still see the bug.

(In reply to comment #33) >Hmm, actually I still see the bug. I don't think suite has updated MsgComposeCommands.js to reflect that we now call onSendNotPerformed on gMsgCompose directly.

I'm actually sceptical about nsIMsgCompose extending nsIMsgSendListener. I think nsMsgCompose should fire events about it's state when appropriate to interested components. Other components should not tell nsMsgCompose to fire events. In other words when a nsMsgCompose window is closed (or aborted?) it should fire the onSendNotPerformed event. nsMsgCompose knows best when to fire what event. This would also prevent API misuse. On the other hand, when it's not broken, don't fix it...

ctrl+w still fails for me also and same js error. TB version 3 alpha 1 (20070326)

Cv1-SM, with comment 25 suggestion(s), and one space nit. [Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9a4pre) Gecko/20070326 SeaMonkey/1.5a] (nightly) (W2Ksp4) Matthijs, Neil: As it is, I kept |null| because |messageId| is undefined at both lines...

Comment on attachment 259729 [details] [diff] [review] (Cv2-SM) <MsgComposeCommands.js> [Checkin: See comment 39] r+sr=me without the whitespace change.

Comment on attachment 259729 [details] [diff] [review] (Cv2-SM) <MsgComposeCommands.js> [Checkin: See comment 39] Checkin: { 2007-03-27 03:13 neil%parkwaycc.co.uk mozilla/mailnews/compose/resources/content/MsgComposeCommands.js 1.394 } without the whitespace change.

Bv1-TB, with comment 25 suggestion(s). (== Port Cv2-SM patch.) [Mozilla Thunderbird, version 3 alpha 1 (20070326)] (nightly) (W2Ksp4)

[Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9a4pre) Gecko/20070327 SeaMonkey/1.5a] (nightly) (W2Ksp4) V.Fixed, SM part.

checked in Bv2-TB Checking in mail/components/compose/content/MsgComposeCommands.js; /cvsroot/mozilla/mail/components/compose/content/MsgComposeCommands.js,v <-- MsgComposeCommands.js new revision: 1.110; previous revision: 1.109

v fixed on trunk - ctrl+w closes correctly. No js error except the already known Error: sMsgComposeService is not defined Source File: chrome://messenger/content/messengercompose/MsgComposeCommands.js Line: 1055

Comment on attachment 257386 [details] [diff] [review] proposed fix for crash [Checkin: Comment 44 & 47] Checkin: { 1.503 bienvenu%nventure.com 2007-03-05 10:26 } NB: This patch was for bug 372705, actually.

[Mozilla Thunderbird, version 3 alpha 1 (20070330)] (nightly) (W2Ksp4) V.Fixed. (The other/"new" error is bug 160942.)

(In reply to comment #35) > I'm actually sceptical about nsIMsgCompose extending nsIMsgSendListener. > > On the other hand, when it's not broken, don't fix it... You might want to open a followup bug for this !?

Comment on attachment 257386 [details] [diff] [review] proposed fix for crash [Checkin: Comment 44 & 47] Checkin: { 1.460.2.32 bienvenu%nventure.com 2007-03-05 10:25 }

I'm using Thunderbird 2.0.0.0 (20070326) on WinXP sp2. I receive the following error: Error: sMsgComposeService is not defined Source File: chrome://messenger/content/messengercompose/MsgComposeCommands.js Line: 1064 This might be fixed in the newer builds, but I'm reporting this because I couldn't find any bug report that mentioned line 1064 specifically. That line contains: if (sMsgComposeService.isCachedWindow(window)) { To reproduce: 1) Click on the "Write" toolbar button. 2) Close the composition window by clicking the X at the top-right.