Crash when sending after copy/paste with Spell Check enabled

VERIFIED FIXED

Status

MailNews Core
Composition
P1
normal
VERIFIED FIXED
17 years ago
10 years ago

People

(Reporter: marina, Assigned: kinmoz)

Tracking

({crash})

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [rtm++] reviewed patch)

Attachments

(1 attachment)

(Reporter)

Description

17 years ago
***** observed with 2000-10-11-08 branch build *****
Steps to reproduce:
- compose new mail;
- highlight text in the Subject field;
- copy/paste several times into the body (paste is not working properly in
today's build, see bug # 56135)
- click Send button and before it'll do spell check (if this option is checked)
it'll crash
//this is a random crash but happens often enough to ignore, attaching a stack
(Reporter)

Comment 1

17 years ago
 Call Stack:     (Signature = 0x029118dd 808c0135)
     0x029118dd
     ViewportFrame::Destroy
[d:\builds\seamonkey\mozilla\layout\html\base\src\nsViewportFrame.cpp, line 144]
     FrameManager::~FrameManager
[d:\builds\seamonkey\mozilla\layout\html\base\src\nsFrameManager.cpp, line 405]
     FrameManager::`scalar deleting destructor'
     FrameManager::Release
[d:\builds\seamonkey\mozilla\layout\html\base\src\nsFrameManager.cpp, line 387]
     PresShell::~PresShell
[d:\builds\seamonkey\mozilla\layout\html\base\src\nsPresShell.cpp, line 1287]
     PresShell::`scalar deleting destructor'
     nsTextInputListener::Release
[d:\builds\seamonkey\mozilla\layout\html\forms\src\nsGfxTextControlFrame2.cpp,
line 225]
     nsCOMPtr_base::~nsCOMPtr_base
[d:\builds\seamonkey\mozilla\xpcom\base\nsCOMPtr.cpp, line 50]
     nsTextServicesDocument::~nsTextServicesDocument
[d:\builds\seamonkey\mozilla\editor\txtsvc\src\nsTextServicesDocument.cpp, line 202]
     nsTextServicesDocument::`scalar deleting destructor'
     nsTextServicesDocument::Release
[d:\builds\seamonkey\mozilla\editor\txtsvc\src\nsTextServicesDocument.cpp, line 211]
     nsCOMPtr_base::~nsCOMPtr_base
[d:\builds\seamonkey\mozilla\xpcom\base\nsCOMPtr.cpp, line 50]
     spellchecker.dll + 0x10b4 (0x036c10b4)
     js_FinalizeObject [d:\builds\seamonkey\mozilla\js\src\jsobj.c, line 1603]
     gc_finalize_phase [d:\builds\seamonkey\mozilla\js\src\jsgc.c, line 915]
     js_GC [d:\builds\seamonkey\mozilla\js\src\jsgc.c, line 1180]
     js_ForceGC [d:\builds\seamonkey\mozilla\js\src\jsgc.c, line 872]
     js_DestroyContext [d:\builds\seamonkey\mozilla\js\src\jscntxt.c, line 220]
     JS_DestroyContext [d:\builds\seamonkey\mozilla\js\src\jsapi.c, line 833]
     nsJSContext::~nsJSContext
[d:\builds\seamonkey\mozilla\dom\src\base\nsJSEnvironment.cpp, line 369]
     nsJSContext::`scalar deleting destructor'
     nsJSContext::Release
[d:\builds\seamonkey\mozilla\dom\src\base\nsJSEnvironment.cpp, line 374]
     nsCOMPtr_base::assign_with_AddRef
[d:\builds\seamonkey\mozilla\xpcom\base\nsCOMPtr.cpp, line 59]
     nsWebShell::Destroy
[d:\builds\seamonkey\mozilla\docshell\base\nsWebShell.cpp, line 1393]
     nsXULWindow::Destroy
[d:\builds\seamonkey\mozilla\xpfe\appshell\src\nsXULWindow.cpp, line 325]
     nsWebShell::Destroy
[d:\builds\seamonkey\mozilla\docshell\base\nsWebShell.cpp, line 1393]
     nsXULWindow::Destroy
[d:\builds\seamonkey\mozilla\xpfe\appshell\src\nsXULWindow.cpp, line 325]
     nsWebShellWindow::Destroy
[d:\builds\seamonkey\mozilla\xpfe\appshell\src\nsWebShellWindow.cpp, line 1779]
     nsChromeTreeOwner::Destroy
[d:\builds\seamonkey\mozilla\xpfe\appshell\src\nsChromeTreeOwner.cpp, line 217]
     nsMsgCompose::CloseWindow
[d:\builds\seamonkey\mozilla\mailnews\compose\src\nsMsgCompose.cpp, line 843]
     nsMsgComposeAndSend::NotifyListenersOnStopCopy
[d:\builds\seamonkey\mozilla\mailnews\compose\src\nsMsgSend.cpp, line 3421]
     CopyListener::OnStopCopy
[d:\builds\seamonkey\mozilla\mailnews\compose\src\nsMsgCopy.cpp, line 142]
     nsMsgCopyService::ClearRequest
[d:\builds\seamonkey\mozilla\mailnews\base\src\nsMsgCopyService.cpp, line 160]
     nsMsgCopyService::NotifyCompletion
[d:\builds\seamonkey\mozilla\mailnews\base\src\nsMsgCopyService.cpp, line 436]
     nsImapMailFolder::ClearCopyState
[d:\builds\seamonkey\mozilla\mailnews\imap\src\nsImapMailFolder.cpp, line 4342]
     nsImapMailFolder::OnStopRunningUrl
[d:\builds\seamonkey\mozilla\mailnews\imap\src\nsImapMailFolder.cpp, line 3426]
     nsUrlListenerManager::BroadcastChange
[d:\builds\seamonkey\mozilla\mailnews\base\src\nsUrlListenerManager.cpp, line 84]
     nsUrlListenerManager::OnStopRunningUrl
[d:\builds\seamonkey\mozilla\mailnews\base\src\nsUrlListenerManager.cpp, line 113]
     nsMsgMailNewsUrl::SetUrlState
[d:\builds\seamonkey\mozilla\mailnews\base\util\nsMsgMailNewsUrl.cpp, line 107]
     nsImapMailFolder::SetUrlState
[d:\builds\seamonkey\mozilla\mailnews\imap\src\nsImapMailFolder.cpp, line 3946]
     SetUrlStateProxyEvent::HandleEvent
[d:\builds\seamonkey\mozilla\mailnews\imap\src\nsImapProxyEvent.cpp, line 1313]
     nsImapEvent::imap_event_handler
[d:\builds\seamonkey\mozilla\mailnews\imap\src\nsImapProxyEvent.cpp, line 76]
     PL_HandleEvent [d:\builds\seamonkey\mozilla\xpcom\threads\plevent.c, line 581]
     PL_ProcessPendingEvents
[d:\builds\seamonkey\mozilla\xpcom\threads\plevent.c, line 517]
     _md_EventReceiverProc [d:\builds\seamonkey\mozilla\xpcom\threads\plevent.c,
line 1051]
<../images/spacer.gif>

Comment 2

17 years ago
I can reproduce this. Will check other platforms.
QA Contact: esther → laurel
Summary: Crash when sending after copy/paste → Crash when sending after copy/paste with Spell Check enabled

Comment 3

17 years ago
I reproduced on Linux rh6.0, but only after turning OFF the copy to Sent folder
pref. If I turn the copy to Sent folder back on, it seems to delay a bit and I
always see the spell check dialog appear. Don't know if that's an absolute
requirement to hit this...

Updated

17 years ago
OS: Windows NT → All
Hardware: PC → All

Comment 4

17 years ago
Adding rtm keyword for PDT consideration.
I would think a lot of folks might hit this in the real user world.
Keywords: crash, rtm

Comment 5

17 years ago
FYI, happens with Mac (OS 9.0), too.

Comment 6

17 years ago
Is this due to the noxif checkin?  Is this caused by _any_ paste operation, or
just some special kinds of pasting?
Whiteboard: [rtm need info]

Comment 7

17 years ago
Any text pasting operation, not special situation.

Comment 8

17 years ago
cc'ing kin.  kin, this stack trace ring any bells?
(Assignee)

Comment 9

17 years ago
I'll take this bug.

What's happening is that the spellchecker is throwing an error, but the code 
that calls it from JS isn't using try/catch to avoid aborting, so spellchecking 
is aborted early, skipping the code that destroys the spellchecker.

By the time the editor is destroyed, some of the data that the spellchecker is 
holding onto is already in a bad state.
Assignee: ducarroz → kin
(Assignee)

Comment 10

17 years ago
Created attachment 17020 [details] [diff] [review]
Patch that fixes the crash.
(Assignee)

Comment 11

17 years ago
I just attatched a fix that contains 2 patches. This crash also happens in 
Composer.

I'd like to get jfrancis or brade to review, and sfraser to super review.

The patches do the following:

  1. nsEditorShell patch - Ensures that if we ever abort in the spellchecker JS 
code, that the spellchecker gets cleaned up at a good time during the 
EditorShell shutdown process.

  2. nsTextServicesDocument.cpp patch - Prevents the spellchecker from 
generating an error by checking to make sure that the range we are about to 
iterate over is not collapsed. This prevents an error from bubbling all the way 
out to the spellchecker JS code.

These 2 patches are trivial and what I consider pretty safe. There are too many 
places in the editor spellcheck JS that we are not using try/catch to try and 
correct at this point in time.

For the curious, the error that #2 fixes, happens only during pastes because the 
edit rules during paste don't force a special moz BR at the end of the document. 
Most other edit operations do.
Status: NEW → ASSIGNED
Priority: P3 → P1
Target Milestone: --- → M19

Comment 12

17 years ago
r=brade

Comment 13

17 years ago
sr=sfraser.

It would be nice later to have a stack-based class that takes care of the
UNLOCK_DOC(this);, but that's later polish.
Whiteboard: [rtm need info] → [rtm+] reviewed patch

Comment 14

17 years ago
rtm++
Whiteboard: [rtm+] reviewed patch → [rtm++] reviewed patch
(Assignee)

Comment 15

17 years ago
Fix checked into the Netscape_20000922_BRANCH:

    mozilla/editor/base/nsEditorShell.cpp                 revision 1.202.2.6
    mozilla/editor/txtsvc/src/nsTextServicesDocument.cpp  revision 1.29.4.1

Fix checked into the TRUNK:

    mozilla/editor/base/nsEditorShell.cpp                 revision 1.209
    mozilla/editor/txtsvc/src/nsTextServicesDocument.cpp  revision 1.30

The fix should appear in the 10/15/00 daily builds.
Status: ASSIGNED → RESOLVED
Last Resolved: 17 years ago
Resolution: --- → FIXED

Comment 16

17 years ago
Well, it appears this is fixed, although there's another spell check bug which
prevents the steps from acting exactly as they did originally... see bug 57165.

I'm guessing this is OK using oct 18th mn6 branch commercial builds with mac OS
9.0, linux rh6.0 and Win98, but I'd rather wait until 57165 resolution before
committing to the OK verification.

Comment 17

17 years ago
57165 is okay in oct20 commercial branch build, linux rh6.0, win98 and mac OS 9.0
Checked this bug (56159) again and it's okay.
Adding vtrunk
Keywords: vtrunk

Comment 18

17 years ago
adding nsonly keyword. unable to verify with mozilla trunk builds which do not
ahve spellchecker.
Keywords: nsonly
Verified FIXED.
Status: RESOLVED → VERIFIED
Product: MailNews → Core
Product: Core → MailNews Core
You need to log in before you can comment on or make changes to this bug.