Closed Bug 328412 Opened 16 years ago Closed 13 years ago

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

Categories

(Core :: Find Backend, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla1.9.2a1

People

(Reporter: jruderman, Assigned: jfkthame)

References

()

Details

(Keywords: assertion, fixed1.9.1, testcase)

Attachments

(1 file)

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.
Still happens on trunk with the steps in comment 0.
Product: Core → SeaMonkey
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.
Attachment #364285 - Flags: review?(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
Attachment #364285 - Flags: review?(bzbarsky) → review+
Attachment #364285 - Flags: superreview?(jst)
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=... ?
Attachment #364285 - Flags: superreview?(jst)
Attachment #364285 - Flags: superreview+
Attachment #364285 - Flags: approval1.9.1+
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.
Assignee: nobody → jfkthame
Keywords: checkin-needed
OS: Mac OS X → All
Hardware: PowerPC → All
Whiteboard: [needs landing]
Is this SeaMonkey only or Core ?
Don't we want a crashtest for it ?
Status: NEW → ASSIGNED
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.
(In reply to comment #7)
> AFAIK, this is not actually a crasher,

I was misleaded by comment 5.
Component: Find In Page → Find Backend
Product: SeaMonkey → Core
QA Contact: keyboard.fayt → find-backend
Crashtests are good for assertion bugs, too :)
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
Attachment #364285 - Attachment description: avoid trying to access nodes if the range is empty → avoid trying to access nodes if the range is empty [Checkin: Comment 10]
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Flags: in-testsuite?
Resolution: --- → FIXED
Whiteboard: [needs landing] → [needs 1.9.1 landing]
Target Milestone: --- → mozilla1.9.2a1
Depends on: 499115
You need to log in before you can comment on or make changes to this bug.