Closed Bug 1364977 Opened 7 years ago Closed 7 years ago

e-mail compose in SeaMonkey crashes [@ InvalidArrayIndex_CRASH | nsTArray_Impl<T>::ElementAt | nsCommandManager::CommandStatusChanged ]


(SeaMonkey :: MailNews: Composition, defect, P3)

SeaMonkey 2.47 Branch


(seamonkey2.48 fixed, seamonkey2.49esr fixed, seamonkey2.50 wontfix, seamonkey2.51 fixed, seamonkey2.52 fixed)

Tracking Status
seamonkey2.48 --- fixed
seamonkey2.49esr --- fixed
seamonkey2.50 --- wontfix
seamonkey2.51 --- fixed
seamonkey2.52 --- fixed


(Reporter: frg, Assigned: frg)



(Keywords: crash, regression)

Crash Data


(2 files, 3 obsolete files)

+++ This bug was initially created as a clone of Bug #1308923 +++

User agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:51.0) Gecko/20100101 Firefox/51.0 SeaMonkey/2.48a2
Build identifier: 20161006025928

1. open new message compose windows with Ctrl+M

New window appears and than SeaMonkey crashes.

MOZ_CRASH Reason	ElementAt(aIndex = 2, aLength = 2)

I am creating a new bug for SeaMonkey only because it also happens in Thunderbird with different code in place.  The cause for this must be different.

In SeaMonkey it happens in:

The obs_documentCreated observer is removed while observing said topic. This causes a crash in embedding/components/commandhandler/nsCommandManager.cpp because the observer array size is not supposed to change there. It does not happen on all systems.
Attached patch 1364977-messagecrash.patch (obsolete) — Splinter Review
I am not so familiar with commandObservers. I am assuming that each editor has its own CommandManager and can have different observers. If this is not the case the patch might need changes. Tested with several windows and it worked. No errors reported in the console.
Attachment #8867793 - Flags: review?(iann_bugzilla)
Blocks: SM2.48
Comment on attachment 8867793 [details] [diff] [review]

Is it worth revisiting the some of the suggestions from bug 468740?
Attached patch 1364977-messagecrash-V2.patch (obsolete) — Splinter Review
Not removing the observer in the observer causes bug 468740 again. Per Neil in Bug 468740 Comment 4 using the openURL function from toolkit's contentAreaUtils.js could be used. I opted to take the OpenSelectedAttachment function from TBs OpenSelectedAttachment MsgComposeCommands.js which more or less does the same. Just Reformatted it.

Instead of the call to this could be used:

> let channel = NetUtil.newChannel({
>   uri,
>   loadUsingSystemPrincipal: true
> });

But it uses the same exact call to newChannelFromURI2 internally and for clarity I actually prefer the TB code.
Attachment #8867793 - Attachment is obsolete: true
Attachment #8867793 - Flags: review?(iann_bugzilla)
Attachment #8869848 - Flags: review?(iann_bugzilla)
Comment on attachment 8869848 [details] [diff] [review]

> function OpenSelectedAttachment()
> {
>+  let bucket = document.getElementById("attachmentBucket");
>+  if (bucket.selectedItems.length == 1) {
>+    let attachmentUrl = bucket.getSelectedItem(0).attachment.url;
>+    let messagePrefix = /^mailbox-message:|^imap-message:|^news-message:/i;
>+    if (messagePrefix.test(attachmentUrl)) {
>+      // We must be dealing with a forwarded attachment, treat this special.
Nit: specially
>+      let msgHdr = gMessenger.messageServiceFromURI(attachmentUrl).messageURIToMsgHdr(attachmentUrl);
I'd prefer to stick with gMessenger.msgHdrFromURI(attachmentUrl) here.

Attachment #8869848 - Flags: review?(iann_bugzilla) → review+
NIT fixed. First thought the different message header processing was a recent change but it seems to be so for ages. Only did go back to 1.9.1.

[Approval Request Comment]
Regression caused by (bug #): 
User impact if declined: SeaMonkey might crash when trying to compose an email.
Testing completed (on m-c, etc.): comm-esr52 to comm-central
Risk to taking this patch (and alternatives if risky): low risk and no alternative.
String changes made by this patch: none

c-r approval is meant for 2.48
Attachment #8869848 - Attachment is obsolete: true
Attachment #8874359 - Flags: review+
Attachment #8874359 - Flags: approval-comm-release?
Attachment #8874359 - Flags: approval-comm-esr52?
Attachment #8874359 - Flags: approval-comm-beta?
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → seamonkey2.52
Version: Trunk → SeaMonkey 2.47 Branch
Comment on attachment 8874359 [details] [diff] [review]

a=me for the requested repos
Attachment #8874359 - Flags: approval-comm-release?
Attachment #8874359 - Flags: approval-comm-release+
Attachment #8874359 - Flags: approval-comm-esr52?
Attachment #8874359 - Flags: approval-comm-esr52+
Attachment #8874359 - Flags: approval-comm-beta?
Attachment #8874359 - Flags: approval-comm-beta+
So is this also needed in Thunderbird? Is bug 1308923 the TB version?
As FRG observed in bug 1308923 comment #28, we never remove the observer added with this call:
            addCommandObserver(gMsgEditorCreationObserver, "obs_documentCreated");

For good house-keeping we could remove it in ComposeUnload().

We already have the nsAttachmentOpener stuff from the SM patch.
Attachment #8875483 - Flags: review?(acelists)
Attachment #8875483 - Attachment is obsolete: true
Attachment #8875483 - Flags: review?(acelists)
Attachment #8875485 - Flags: review?(acelists)
Comment on attachment 8875485 [details] [diff] [review]
1364977.patch - TB patch for the same issue.

Review of attachment 8875485 [details] [diff] [review]:

Thanks. So does this fix bug 1308923 or is that different?
Attachment #8875485 - Flags: review?(acelists) → review+
aceman. Not sure but I think not. The problem here was that it was removed during notification processing so the array used in the calling code changed. That is also why I spun off this bug. There must be other causes for this but probably only add-on related.
Landed for Thunderbird, won't do uplifts:

Sorry for hijacking your bug ;-)
No longer blocks: 2.51BulkMalfunctions
You need to log in before you can comment on or make changes to this bug.