Last Comment Bug 528368 - crash during spell check [@ nsTextServicesDocument::IsBlockNode(nsIContent*)]
: crash during spell check [@ nsTextServicesDocument::IsBlockNode(nsIContent*)]
[tb31wanted][tbird topcrash][fixed TB...
: crash, topcrash
Product: Core
Classification: Components
Component: Editor (show other bugs)
: 1.9.1 Branch
: x86 All
-- critical (vote)
: mozilla2.0b7
Assigned To: Nobody; OK to take it and work on it
: Makoto Kato [:m_kato]
Depends on:
  Show dependency treegraph
Reported: 2009-11-12 16:29 PST by Wayne Mery (:wsmwk, NI for questions)
Modified: 2011-06-13 10:01 PDT (History)
6 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

Patch (v1) (981 bytes, patch)
2010-08-24 20:27 PDT, :Ehsan Akhgari
roc: review+
Details | Diff | Splinter Review
Patch (v1.1) (1.02 KB, patch)
2010-08-24 21:35 PDT, :Ehsan Akhgari
vladimir: approval2.0+
dveditz: approval1.9.2.11+
dveditz: approval1.9.1.14+
Details | Diff | Splinter Review

Description User image Wayne Mery (:wsmwk, NI for questions) 2009-11-12 16:29:36 PST
crash during spell check?  [@ nsTextServicesDocument::IsBlockNode(nsIContent*)]
Crash Address	0x8 in most cases
rarely seen in FF (avg 2 per month) bp-ceff975e-8aac-4bdc-af7f-524422091020

Ran spell checker and when I chose to replace a word, the carsh occurred.
0	thunderbird.exe	nsTextServicesDocument::IsBlockNode	 editor/txtsvc/src/nsTextServicesDocument.cpp:3095
1	thunderbird.exe	nsTextServicesDocument::FirstTextNodeInNextBlock	editor/txtsvc/src/nsTextServicesDocument.cpp:4263
2	thunderbird.exe	nsTextServicesDocument::GetFirstTextNodeInNextBlock	editor/txtsvc/src/nsTextServicesDocument.cpp:4325
3	thunderbird.exe	nsTextServicesDocument::FirstBlock	editor/txtsvc/src/nsTextServicesDocument.cpp:615
4	thunderbird.exe	mozSpellChecker::Replace	extensions/spellcheck/src/mozSpellChecker.cpp:223
5	thunderbird.exe	nsEditorSpellCheck::ReplaceWord	editor/composer/src/nsEditorSpellCheck.cpp:308
6	xpcom_core.dll	NS_InvokeByIndex_P	xpcom/reflect/xptcall/src/md/win32/xptcinvoke.cpp:101
7	thunderbird.exe	XPCWrappedNative::CallMethod	js/src/xpconnect/src/xpcwrappednative.cpp:2291
8	thunderbird.exe	XPC_WN_CallMethod	js/src/xpconnect/src/xpcwrappednativejsops.cpp:1583 

Comment 1 User image Jason Oster (:Parasyte) 2010-05-06 08:17:26 PDT
Comment 2 User image Wayne Mery (:wsmwk, NI for questions) 2010-06-14 06:59:55 PDT
can you reproduce this crash using trunk build (v3.2 alpha)?
  backup your profile before testing. and note, your global index will reindex

The reason for asking about trunk, is hunspell got updated on trunk a couple days ago. [1]

#25 crash for 3.0.4, and exists in 3.1
~3% of crashes have email addresses (pretty amazing). PMed them to see if anyone can reproduce with trunk build.

[1] as noted in mdat newsgroup...
On Sat, 12 Jun 2010 12:58:15 -0400, Wayne Mery wrote:
> new 1.2.11 2010-05-06
> previous 1.2.8 2009-03-03
Bug 564608 - Update Hunspell to 1.2.11

Comment 3 User image Jason Oster (:Parasyte) 2010-06-14 07:17:50 PDT
I've been unable to reproduce this at all, unfortunately.  The breakpad ID I linked in comment #1 was from one of our employees.
Comment 4 User image timeless 2010-06-14 16:46:12 PDT
bug 302775 is the last to change nearby lines.

the crash is because:
4242 nsTextServicesDocument::FirstTextNodeInNextBlock(nsIContentIterator *aIterator)

4254 nsCOMPtr<nsIContent> content = do_QueryInterface(aIterator->GetCurrentNode());

content is null

4256 if (IsTextNode(content))

this is false.

4257 {
4262 }
4263 else if (!crossedBlockBoundary && IsBlockNode(content)) 

we pass null to IsBlockNode() which crashes.
Comment 5 User image :Ehsan Akhgari 2010-06-15 10:26:53 PDT
The analysis in comment 4 is true, and it's very easy to add the null check, but I'm trying to understand what the underlying reason for the crash is.
Comment 6 User image :Ehsan Akhgari 2010-08-24 20:27:42 PDT
Created attachment 468915 [details] [diff] [review]
Patch (v1)

Well, I guess taking a fix here won't hurt.
Comment 7 User image Robert O'Callahan (:roc) (email my personal email if necessary) 2010-08-24 20:42:56 PDT
Comment on attachment 468915 [details] [diff] [review]
Patch (v1)

Can you put an NS_ERROR in here and leave the bug open? I think we should at some point understand how null can get in here.
Comment 8 User image :Ehsan Akhgari 2010-08-24 21:35:20 PDT
Created attachment 468928 [details] [diff] [review]
Patch (v1.1)

(In reply to comment #7)
> Comment on attachment 468915 [details] [diff] [review]
> Patch (v1)
> Can you put an NS_ERROR in here and leave the bug open? I think we should at
> some point understand how null can get in here.

Sure, makes sense.
Comment 10 User image Daniel Veditz [:dveditz] 2010-09-27 14:18:12 PDT
Comment on attachment 468928 [details] [diff] [review]
Patch (v1.1)

Approved for and, a=dveditz
Comment 11 User image Wayne Mery (:wsmwk, NI for questions) 2010-09-27 15:22:41 PDT
thanks to all, for the quick attention to this topcrash.
reopening per comment 7
Comment 13 User image Mike Beltzner [:beltzner, not reading bugmail] 2011-02-23 10:19:19 PST
Should this bug be marked FIXED?
Comment 14 User image :Ehsan Akhgari 2011-02-23 19:23:38 PST
(In reply to comment #13)
> Should this bug be marked FIXED?

No, see comment 7 please.
Comment 15 User image :Ehsan Akhgari 2011-03-22 11:25:15 PDT
OK, I give up, and I'm closing this.  The possible NS_ERRORs that we get can probably be filed as new bugs.  This bug is showing up in all sorts of queries all the time, and it's really misleading to leave this open.

Note You need to log in before you can comment on or make changes to this bug.