Closed Bug 802995 Opened 9 years ago Closed 9 years ago

crash in nsTextServicesDocument::DeleteSelection while using spell checker


(Core :: DOM: Editor, defect)

16 Branch
Windows NT
Not set



Tracking Status
firefox17 + fixed
firefox18 --- fixed


(Reporter: Usul, Assigned: ayg)



(Keywords: crash, regression, Whiteboard: [tbird topcrash][TB16 regression][gs])

Crash Data


(1 file)

This bug was filed from the Socorro interface and is 
report bp-bba5f0ee-9249-4daa-b23f-58e7d2121016 .
0 	xul.dll 	nsTextServicesDocument::DeleteSelection 	editor/txtsvc/src/nsTextServicesDocument.cpp:1387
1 	xul.dll 	nsTextServicesDocument::InsertText 	editor/txtsvc/src/nsTextServicesDocument.cpp:1737
2 	xul.dll 	mozSpellChecker::Replace 	extensions/spellcheck/src/mozSpellChecker.cpp:222
3 	xul.dll 	nsEditorSpellCheck::ReplaceWord 	editor/composer/src/nsEditorSpellCheck.cpp:394
4 	xul.dll 	NS_InvokeByIndex_P 	xpcom/reflect/xptcall/src/md/win32/xptcinvoke.cpp:70
5 	xul.dll 	XPCWrappedNative::CallMethod 	js/xpconnect/src/XPCWrappedNative.cpp:2418
6 	xul.dll 	XPC_WN_CallMethod 	js/xpconnect/src/XPCWrappedNativeJSOps.cpp:1474
7 	mozjs.dll 	js::InvokeKernel 	js/src/jsinterp.cpp:344
8 	mozjs.dll 	js::Interpret 	js/src/jsinterp.cpp:2442
9 	mozjs.dll 	js::RunScript 	js/src/jsinterp.cpp:293
10 	mozjs.dll 	js::InvokeKernel 	js/src/jsinterp.cpp:355
11 	mozjs.dll 	js::Invoke 	js/src/jsinterp.cpp:387
12 	mozjs.dll 	JS_CallFunctionValue 	js/src/jsapi.cpp:5604
13 	xul.dll 	nsJSContext::CallEventHandler 	dom/base/nsJSEnvironment.cpp:1892
14 	kernel32.dll 	SystemTimeToFileTime 	
15 		@0xe1ea05f

Comments found in Crash-stats :
This has now happened 4 times, every time I start the spell checker. I have not tried rebooting yet.

I was spell checking a reply to an e-mail; then it just disapeared
another regression

vincent, roland, does this correlate to gsfn issue reported by vincent in tb-support-crew?  can you hook up the topic URL/tag to this bug

also exists in TB17 bp-8165cfcf-acf3-4731-b889-6610e2121017
Crash Signature: [@ nsTextServicesDocument::DeleteSelection()] → [@ nsTextServicesDocument::DeleteSelection()] [@ nsTextServicesDocument::DeleteSelection]
Keywords: regression
Summary: crash in nsTextServicesDocument::DeleteSelection → crash in nsTextServicesDocument::DeleteSelection while using spell checker
Whiteboard: [tbird topcrash][TB16 regression][gs]
Version: 17 Branch → 16 Branch
I haven't heard of any crash related to spell check at this time...
Ehsan - can you help the TB guys out and take a look a this? I know you've done Editor work in the past. Thanks :)
Assignee: nobody → ehsan
Sure, I can help.  Aryeh, can you please take a look and see if you find out anything?

Assignee: ehsan → ayg
The line of the crash was last changed by bug 763283 part 2, which uses ->AsContent() without a null check.  Without digging to see how exactly this could happen, that seems like the obvious reason for the crash.  The regression should be in 16 in that case, which seems to be what happened.
Attached patch Add null checkSplinter Review
The original code before bug 763283 was

      if (mIteratorStatus != nsTextServicesDocument::eIsDone)
        // The old iterator is still pointing to something valid,
        // so get its current node so we can restore it after we
        // create the new iterator!

        curContent = do_QueryInterface(mIterator->GetCurrentNode());

so I changed it to better match that.  No test, because I have no way to know how to reproduce.  Try:
Attachment #675087 - Flags: review?(ehsan)
Attachment #675087 - Flags: review?(ehsan) → review+
Comment on attachment 675087 [details] [diff] [review]
Add null check

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 763283
User impact if declined: Thunderbird topcrash
Testing completed (on m-c, etc.): We can't reproduce the crash, but the fix is straightforward
Risk to taking this patch (and alternatives if risky): Minimal
String or UUID changes made by this patch: none.
Attachment #675087 - Flags: approval-mozilla-beta?
Attachment #675087 - Flags: approval-mozilla-aurora?

Should this have a test?
Closed: 9 years ago
Flags: in-testsuite?
Resolution: --- → FIXED
Target Milestone: --- → mozilla19
Yes, if anyone can create one.  We only know about the crash from crash reports, and don't have steps to reproduce, so we don't know how to write a test.  It would probably take an excessive amount of work to test this.
Comment on attachment 675087 [details] [diff] [review]
Add null check

Risk is low enough, let's see if this takes down the volume on branches, approving.
Attachment #675087 - Flags: approval-mozilla-beta?
Attachment #675087 - Flags: approval-mozilla-beta+
Attachment #675087 - Flags: approval-mozilla-aurora?
Attachment #675087 - Flags: approval-mozilla-aurora+
will need to check crash-stats after TB17 release
Blocks: 763283
Thanks for the fix.

v.fixed - no crash sigs in TB17.
But there some other sigs - bug 834487, and bug  835314
You need to log in before you can comment on or make changes to this bug.