Closed Bug 414338 Opened 14 years ago Closed 14 years ago

nsFind crashes if passed a range rooted at a document

Categories

(Core Graveyard :: Embedding: APIs, defect, P2)

defect

Tracking

(Not tracked)

VERIFIED FIXED

People

(Reporter: neil, Assigned: smaug)

Details

(Keywords: crash)

Attachments

(2 files, 1 obsolete file)

Bug 414018 crashes because the range passed to nsFind was unexpectedly rooted at a document. I say unexpected because we don't normally seem to allow that (the problem was only exposed because I accidentally checked in the patch for bug 399427) however the interface declaration for nsIDOMRange shows that it accepts nodes as endpoints which obviously could fail to QI to nsIContent.

The methods used look as if they might be available from content.
Yeah, the find code needs an overhaul... and an owner.

If you think this is accessible from content, please nominate for blocking?
Flags: blocking1.9?
Flags: blocking1.9? → blocking1.9+
Setting priority to P2 for now, if someone thinks that's wrong, let us know. This also needs an owner if it's going to get fixed, otherwise I don't think it blocks.
Priority: -- → P2
Flags: tracking1.9+ → blocking1.9+
bz thoughts on who could take this bug?
Any of a number of people none of whom really know this code...  me, jonas, jst, peterv, smaug, mats.  No idea which of them have time for this.
So what is the testcase or step-to-reproduce for this?
The easiest test case I could write was this chrome testcase:

var range = document.createRange();
range.selectNodeContents(document);
const NS_FIND_CONTRACTID = '@mozilla.org/embedcomp/rangefind;1';
const nsIFind = Components.interfaces.nsIFind;
var find = Components.classes[NS_FIND_CONTRACTID].createInstance(nsIFind);
find.Find("text", range, range, range);
Attached file testcase, run locally (obsolete) —
Attached file some more testing
Attachment #310006 - Attachment is obsolete: true
Attached patch null checkSplinter Review
At least for that crash a simple null check is enough.
The crash happens because code which tries find something in form controls
doesn't check if content is null - it does check if content has textcontrol frame etc. and if it doesn't have, just returns.
I looked at also other places where nsIContent is used and those have null checks.
The testcase shows that when null check is added, nsIFind initialized with document-rooted-range does find the searchstring.

I have locally some more tests, have to convert those to mochitest.
Assignee: nobody → Olli.Pettay
Status: NEW → ASSIGNED
Attachment #310038 - Flags: superreview?(jst)
Attachment #310038 - Flags: review?(jst)
Attachment #310038 - Flags: superreview?(jst)
Attachment #310038 - Flags: superreview+
Attachment #310038 - Flags: review?(jst)
Attachment #310038 - Flags: review+
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Flags: in-testsuite?
Resolution: --- → FIXED
verified fixed using Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9pre) Gecko/2008050804 Minefield/3.0pre and Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9pre) Gecko/2008050614 Minefield/3.0pre - no crash on testcases

--> Verified fixed
Status: RESOLVED → VERIFIED
Flags: in-testsuite? → in-testsuite-
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.