414338 - nsFind crashes if passed a range rooted at a document

Status: VERIFIED FIXED

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

Description

[[e-mail]]

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.

Comment 1

[Boris Zbarsky [:bzbarsky]]

Yeah, the find code needs an overhaul... and an owner.

If you think this is accessible from content, please nominate for blocking?

Comment 2

[Mike Beltzner [:beltzner, not reading bugmail]]

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.

Comment 3

[Mike Schroepfer]

bz thoughts on who could take this bug?

Comment 4

[Boris Zbarsky [:bzbarsky]]

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.

Comment 5

[Olli Pettay [:smaug][[e-mail]]]

So what is the testcase or step-to-reproduce for this?

Comment 6

[[e-mail]]

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);

Comment 7

[Olli Pettay [:smaug][[e-mail]]]

Attachment: testcase, run locally

Comment 8

[Olli Pettay [:smaug][[e-mail]]]

Attachment: some more testing

Comment 9

[Olli Pettay [:smaug][[e-mail]]]

Attachment: null check

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.

Comment 10

[Carsten Book [:Tomcat]]

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