101710 - Find crashes in frames [@ nsContentIterator::NextNode]

Status: VERIFIED FIXED

(Core :: DOM: Editor, defect)

Description

[Conrad Carlen (not reading bugmail)]

Go to the given URL.
Search for the word "Carbon"
Crash.

It crashes here:
http://lxr.mozilla.org/mozilla/source/content/base/src/nsContentIterator.cpp#711

This broke recently.

Comment 1

[Conrad Carlen (not reading bugmail)]

Just noticed this was on the console after crash if that helps.

!!! ASSERTION: ContentIterator stack underflow: 'aIndexes->Count() > 0', file
d:\moz\mozilla\content\base\src\nsContentIterator.cpp, line 705
!!! ASSERTION: ContentIterator stack underflow: 'aIndexes->Count() > 0', file
d:\moz\mozilla\content\base\src\nsContentIterator.cpp, line 705
!!! ASSERTION: You can't dereference a NULL nsCOMPtr with operator->().:
'mRawPtr != 0', file ..\..\..\dist\include\nsCOMPtr.h, line 649

Comment 2

[Jerome Lacoste]

WFM If I use 2001092308, [before the find patch landed]. So its a regression one.

I don't know if it is related but when searching with my version, the search
code never stops. I.e. when doing 'Search', I end up doing a loop over the text
(going on the left frame and coming back to the main one), never reaching a End
of text reached.
I don't know if this is related to the frames.
Is this normal behavior?

Comment 3

[Conrad Carlen (not reading bugmail)]

> I.e. when doing 'Search', I end up doing a loop over the text (going on the
> left frame and coming back to the main one), never reaching a End of text
> reached.

That's an unrelated problem but, since you mention it, it bug 92102.

Comment 4

[Randell Jesup [:jesup] (needinfo me)]

This is definitely caused by 31770.  I'm going to try my older patch and see if
the same thing happens.  In any case, I believe there is an easy solution if
this is a index stack underflow.

Comment 5

[Randell Jesup [:jesup] (needinfo me)]

Verified that this bug does not occur with the last patch I posted to bug 31770,
so it must be part of jfrancis' rewrite (I must have missed it in review).  I'll
look at it more closely after lunch.

Comment 6

[Randell Jesup [:jesup] (needinfo me)]

I have a patch that appears to fix the bug, though it's probably not minimal. 
I'll attach it shortly.

Comment 7

[Randell Jesup [:jesup] (needinfo me)]

Attachment: Fixes crash due to underflow of index stack

Comment 8

[Randell Jesup [:jesup] (needinfo me)]

Tested with Linux and FreeBSD on the site given.  Still may do more than
absolutely required, but I think all the changes are good.

Looking for r/sr, also suggesting for nsbranch because 31770 was suggested for
nsbranch.  Adding dependency

Comment 9

[David Einstein]

*** Bug 101737 has been marked as a duplicate of this bug. ***

Comment 10

[Kevin McCluskey (gone)]

Marking nsbranch-. 31770 was marked nsbranch-.

Comment 11

[kinmoz]

- Appending dummy zeros to mIndexes should not be necessary since we should
never go above mCommonParent.

- PositionAt() is not implemented for the subtree iterator, so First()/Last()
will be broken for the subtree iterator.

Comment 12

[Randell Jesup [:jesup] (needinfo me)]

After discussion with Kin, new patch coming.  Overrides First & Last in the
subtree iterator so we can use PositionAt in the content iterator's First/Last. 
Also removed some unneeded parts of the earlier patch.  Tested on BSD/Linux.

Comment 13

[Randell Jesup [:jesup] (needinfo me)]

Attachment: Updated patch

Comment 14

[kinmoz]

Comment on attachment 50953 [details] [diff] [review]
Updated patch

sr=[e-mail]

For the places where you added:

  +      if (aIndexes->Count() > 1)


do we want to add asserts to indicate when that isn't true?

Comment 15

[Randell Jesup [:jesup] (needinfo me)]

Taking the bug; will commit if I get an r=.

I prefer not to assert on those cases.  I don't consider that a real bug, and
non-bug assertions are very annoying in the source tree.  However, I agree that
I don't _think_ they'll ever be hit.  I'd rather just be safe, since we don't
want to take array[-1].

Comment 16

[Simon Fraser [no longer active]]

Comment on attachment 50953 [details] [diff] [review]
Updated patch

r=sfraser

Comment 17

[Randell Jesup [:jesup] (needinfo me)]

Fix checked in

Comment 18

[Ashley Bischoff (blog at handcoding.com)]

*** Bug 101889 has been marked as a duplicate of this bug. ***

Comment 19

[James Baxter]

Verified Fixed on 10-03 Trunk Build

Comment 20

[Aleksey Nogin]

I might be seing a version of this bug with BuildId 2002030616 on RedHat Linux. 

Reproducible: always.
To reproduce:
0) Get Mozilla with spellchecker (see http://spellchecker.mozdev.org/ or bug 56301)
1) Compose a message with a word "misteke" on a line by itself.
2) Spellcheck it.
3) When spellchecker highlights "misteke" and proposes "mistake", click on "Change".

"misteke misteke" does not cause the crash.

Stack trace:

#0  0x40e1fa99 in nsContentIterator::NextNode () from
/usr/lib/mozilla/components/libgkcontent.so
#1  0x40e20161 in nsContentIterator::Next () from
/usr/lib/mozilla/components/libgkcontent.so
#2  0x4274fea8 in nsTextServicesDocument::FirstTextNodeInNextBlock ()
   from /usr/lib/mozilla/components/libtxtsvc.so
#3  0x42750084 in nsTextServicesDocument::GetFirstTextNodeInNextBlock ()
   from /usr/lib/mozilla/components/libtxtsvc.so
#4  0x42748b9c in nsTextServicesDocument::FirstBlock () from
/usr/lib/mozilla/components/libtxtsvc.so
#5  0x427597b8 in mozSpellChecker::SetupDoc () from
/usr/lib/mozilla/components/libspellchecker.so
#6  0x427586bb in mozSpellChecker::NextMisspelledWord () from
/usr/lib/mozilla/components/libspellchecker.so
#7  0x4272bf6c in nsEditorShell::GetNextMisspelledWord () from
/usr/lib/mozilla/components/libcomposer.so
#8  0x40185fa2 in XPTC_InvokeByIndex () from /usr/lib/libxpcom.so
 ...

Does this look like the same bug, or a different one?

P.S. This bug was initially discovered by [e-mail]