crash in nsTextServicesDocument::DeleteSelection while using spell checker

VERIFIED FIXED in Firefox 17

Status

()

Core
Editor
--
critical
VERIFIED FIXED
5 years ago
4 years ago

People

(Reporter: Usul, Assigned: ayg)

Tracking

({crash, regression})

16 Branch
mozilla19
x86
Windows NT
crash, regression
Points:
---
Bug Flags:
in-testsuite ?

Firefox Tracking Flags

(firefox17+ fixed, firefox18 fixed)

Details

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

Attachments

(1 attachment)

(Reporter)

Description

5 years ago
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]
tracking-firefox17: --- → ?
tracking-firefox-esr17: --- → ?
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

Comment 2

5 years ago
I haven't heard of any crash related to spell check at this time...

Comment 3

5 years ago
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
tracking-firefox17: ? → +
tracking-firefox-esr17: ? → ---
Sure, I can help.  Aryeh, can you please take a look and see if you find out anything?

Thanks!
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.
Status: NEW → ASSIGNED
Created attachment 675087 [details] [diff] [review]
Add null check

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: https://tbpl.mozilla.org/?tree=Try&rev=6c0b12df1d34
Attachment #675087 - Flags: review?(ehsan)
Attachment #675087 - Flags: review?(ehsan) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/9a9ae8ce3b8d
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?
https://hg.mozilla.org/mozilla-central/rev/9a9ae8ce3b8d

Should this have a test?
Status: ASSIGNED → RESOLVED
Last Resolved: 5 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+
https://hg.mozilla.org/releases/mozilla-aurora/rev/005b6a9a807e
https://hg.mozilla.org/releases/mozilla-beta/rev/a2cc1ba86817
status-firefox17: --- → fixed
status-firefox18: --- → fixed
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
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.