184858 - Find not finding all matches

Status: VERIFIED FIXED

(Core :: DOM: Editor, defect, P3)

Description

[[e-mail]]

I wanted to search the patch for "ClearMenulist" but find would only find 4 of
11 matches. Even when viewing the patch in its own window only 9 matches are found.

Comment 1

[Jesiah S]

Visited http://bugzilla.mozilla.org/attachment.cgi?id=108902&action=edit was
able to find "ClearMenulist" 11 times.
Used Win98 with Mozilla 1.2.1

Comment 2

[Max Alekseyev]

Found only 4 matches with OS/2 build 2002121112

Comment 3

[Max Alekseyev]

OS -> All

Comment 4

[Johnny Stenback (:jst)]

Attachment: Simplified testcase, look for PopNoReturn, there are two, mozilla finds only one.

Comment 5

[Johnny Stenback (:jst)]

Aaron says this may be related to bug 175046.

Comment 6

[Aaron Leventhal]

This seems like a major bug.

I'll bet the problem is in nsFind.cpp
Here are the recent checkins to nsFind.cpp:
1.16 kin%netscape.com Nov 21 07:07   Fix for bug #176251 (Problems with
nsContentIterator PRE traversal)
1.15 aaronl%netscape.com Nov 5 18:59   Bug 175046, bug 172991, bug 166471.

Can someone look to see if it broke right after Nov 5 or Nov 21?

Comment 7

[Aaron Leventhal]

-> Kin

The test case works after I back out these changes in my local tree:
1.16 kin%netscape.com Nov 21 07:07   Fix for bug #176251 (Problems with
nsContentIterator PRE traversal)

Comment 8

[kinmoz]

I have a feeling that post-order is being assumed in other parts of the find
code even though the code I modified hinted that pre-order may be wanted in the
forward search case.

The temporary fix may be to just use a post-iterator in both the forward and
backward search cases, which  should restore it back to what it was doing prior to
my changes.

Comment 9

[sairuh (rarely reading bugmail)]

yay, mid-air collisions. anyhow, my testing confirms what aaron found.

using linux comm trunk builds (rh8) this regressed sometime btwn 2002.11.20.22
(works fine) and 2002.11.21.08 (broken).

Comment 10

[kinmoz]

Ok looks like the problem is in |nsContentIterator::Init(nsRange*)| and is
specific  to the case where one of the range end points is in a text node, and
the other is in the text node's parent right next to the text node ... this 
results in a case where |mFirst| and |mLast| should be the same text node, but
we're making an empty iterator because the |ContentIsInTraversalRange()| is failing.

Patch coming up.

Comment 11

[kinmoz]

Attachment: Patch Rev 1

This makes |ContentIsInTraversalRange()| handles the special case where
aContent is one of the traversal range end points.

Comment 12

[Asa Dotzler [:asa]]

assuming that was a nomination and setting the ? (only drivers can set the
blocking.1.3b + flag)

Comment 13

[Joe Francis]

Comment on attachment 109564 [details] [diff] [review]
Patch Rev 1

Is there any reason not to QI startContent and endContent to
nsIDOMCharacterData, instead of nsIContent?

Comment 14

[Joe Francis]

Ignore my comment - misread patch and though we were comparing pointers on
different interfaces.

Comment 15

[kinmoz]

Fix checked into TRUNK:

  mozilla/content/base/src/nsContentIterator.cpp  revision 1.55

Comment 16

[[e-mail]]

WFM, debug linux built 6 hours ago.