328412 - Find in empty document triggers "ASSERTION: No first node!"

Status: RESOLVED FIXED

(Core :: Find Backend, defect)

Description

[Jesse Ruderman]

Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.9a1) Gecko/20060223 Firefox/1.6a1

Steps to reproduce:
1. Load data:text/html,
2. Ctrl+F
3. r

Result:

###!!! ASSERTION: No first node!: 'mFirst', file /Users/admin/trunk/mozilla/content/base/src/nsContentIterator.cpp, line 965

Bug 323065 and bug 252706 are also about ways of hitting this assertion.  I don't know whether this is a dup of one or both of them.

Comment 1

[Jesse Ruderman]

Still happens on trunk with the steps in comment 0.

Comment 2

[Jonathan Kew [:jfkthame]]

Attachment: avoid trying to access nodes if the range is empty [Checkin: Comment 10]

In nsFindContentIterator::Reset(), if the search range is completely empty (e.g., doing Find in about:blank), then the outer iterator will be "done" immediately after initialization. In this case, we should simply return, and not attempt to access its First() or Last() nodes.

Comment 3

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 364285 [details] [diff] [review]
avoid trying to access nodes if the range is empty
[Checkin: Comment 10]

Makes sense.  r=bzbarsky

Comment 4

[Jonathan Kew [:jfkthame]]

Comment on attachment 364285 [details] [diff] [review]
avoid trying to access nodes if the range is empty
[Checkin: Comment 10]

trivial fix - may be appropriate for rs=... ?

Comment 5

[Johnny Stenback (:jst)]

Comment on attachment 364285 [details] [diff] [review]
avoid trying to access nodes if the range is empty
[Checkin: Comment 10]

Yeah, trivial... We should take this crash fix for 1.9.1 too IMO.

Comment 6

[Serge Gautherie (:sgautherie)]

Is this SeaMonkey only or Core ?
Don't we want a crashtest for it ?

Comment 7

[Jonathan Kew [:jfkthame]]

It's not just SeaMonkey - not sure which component of Core, though. Would this be part of "Embedding:GRE Core", or "Find Backend"?

AFAIK, this is not actually a crasher, as the contentIterator handles the null value safely. So in a release build, the error will go unnoticed; in a debug build, the assertion will fire but the program still runs successfully. Do we need another kind of test here as well? The assertion was already drawing attention to the incorrect use of the contentIterator, just like it was meant to do.

Comment 8

[Serge Gautherie (:sgautherie)]

(In reply to comment #7)
> AFAIK, this is not actually a crasher,

I was misleaded by comment 5.

Comment 9

[Jesse Ruderman]

Crashtests are good for assertion bugs, too :)

Comment 10

[Serge Gautherie (:sgautherie)]

Comment on attachment 364285 [details] [diff] [review]
avoid trying to access nodes if the range is empty
[Checkin: Comment 10]


http://hg.mozilla.org/mozilla-central/rev/d431b9fb79fc

Comment 11

[Dão Gottwald [:dao]]

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/44b27c625b7f