Closed Bug 132612 Opened 23 years ago Closed 23 years ago

'Find in This Page' does not find all matches


(SeaMonkey :: UI Design, defect)

Windows 98
Not set


(Not tracked)



(Reporter: mh.mozilla, Assigned: akkzilla)




(Whiteboard: FIX IN HAND, reviewed)


(2 files, 3 obsolete files)

From Bugzilla Helper:
User-Agent: Mozilla/5.0 (Windows; U; Win98; en-US; rv:0.9.9) Gecko/20020311
BuildID:    2002031104

In the mentioned page (mozilla bug list with about 500 bugs)
'Find in This Page' does not work.
Searching for owners starting with 'c' (e.g. 'curt') finds only some
matches and misses others.

Reproducible: Always
Steps to Reproduce:
1. Go to the mentioned URL or a different Mozilla-Bugzilla page with
   many bugs (> 300)
2. Sort by owner
3. 'Find on This Page' (Ctrl+F) and enter an owner name starting
   with 'c' (e.g. 'curt'), search forward.
4. Repeat search: press 'Find' again in the search dialog or 
   leave search dialog and press Ctrl+G

Actual Results:  When searching forward, only one match is found.
When searching backward, all matches (nine) are found.

Expected Results:  All matches should be found.

- Search fails only on large pages
- Search fails only if the text starts with 'c'
- Search fails only if search direction is forward
to akkana
Assignee: sgehani → akkana
Whiteboard: DUPEME
I bet this was caused by the same problems that caused 129971.  In any case, I
don't see it in today's build.  Try it and see if you agree.

*** This bug has been marked as a duplicate of 129971 ***
Closed: 23 years ago
Resolution: --- → DUPLICATE
I still see this bug. I used the sources from 2002-03-27.
Did you search for 'curt'?

I will try the latest nightly build.
Now I tried build 2002032803 and still see the bug.
Resolution: DUPLICATE → ---
This is working for me, in today's build.  I can go through the "curt"
instances with either return-return in the find dialog, or with accel-G (find

I wasn't able to test this for the last few days because bugzilla slowness has
made the query time out.  I finally got a successful query, so I saved it,
edited the initial "Please wait" stuff out and am attaching it here for

If you are still seeing this, please save your query to a file, test on the
file and see if you're still seeing it.  Maybe we're getting different queries
somehow, based on different bugzilla cookies.  Do you see the behavior when
searching for curt in this attachment?
You are right, I don't see the bug with your query.
My query (which shows the bug) looks different. I get other columns.
I will save as file an create an attachment.
Attached file copy of a query that shows the bug (obsolete) —
qa -->
QA Contact: paw → pmac
Yes, now I see it in your query.
Ever confirmed: true
Attachment #77805 - Attachment is obsolete: true
Attachment #77685 - Attachment is obsolete: true
Target Milestone: --- → mozilla1.0.1
my first thought is it looks like its a problem with
backwards(checked/unchecked) state with the first search and forward searching
doesn't get set correctly initially upon loading find in this page dialog.. 
maybe its a problem with the state widget file like the scripts & windows bug
that was just fixed.. 

No, actually the problem is that it's getting confused by the two 'c' characters
in a row, spanning from one text node to the next one.  It doesn't happen in all
cases, just some of them.  In the reverse direction the comparison is completely
different (it goes back to front) so you don't see the same behavior.
Whiteboard: DUPEME
I was idly wondering... are the forward and reverse cases so different that the
bug simply cannot occur in reverse?
It turns out that the simple patch in bug 134275 also fixes this bug.  But
that's accidental and doesn't fix the real cause, so I'm still working on a fix
to the algorithm.  I still don't know what causes this -- it's very sensitive to
position, has to have exactly the right sort of table entries plus an entity
immediately after the end of the found string.  When I figure out why it's
happening, I may be able to answer the question of whether it could be made to
happen in the reverse direction too.
Attached patch A fix (obsolete) — Splinter Review
Got it.  It was a problem with the way we went back to the match anchor node
when ending a partial match.  The important part of this patch is that it sets
mIterOffset back to the match anchor offset, and sets frag=0, so that when we
loop we'll know we're changing nodes and need to start findex at mIterOffset. 
A less important change is that restart wasn't being used, but missed being
deleted from the code in the last go-round, so I'm removing it now.
Comment on attachment 81104 [details] [diff] [review]
A fix

diff -u is usual :-)
Attached patch diff -uSplinter Review
Argh!  I should think about doing alias diff diff -u -- I keep thinking I typed
it and find out I didn't.  Sorry.
Attachment #81104 - Attachment is obsolete: true
Kathy, can you review this?  Kin, can you sr?
Whiteboard: FIX IN HAND, needs r, sr
Comment on attachment 81113 [details] [diff] [review]
diff -u

Attachment #81113 - Flags: review+
Comment on attachment 81113 [details] [diff] [review]
diff -u

Do we want to outdent this comment?

+      mIterOffset = matchAnchorOffset;
	   // +incr will be added to findex when we continue
-      nsCOMPtr<nsIContent> content (do_QueryInterface(matchAnchorNode));

Should |content| ever be null in the code below, what are the chances of us
going into an infinite loop since the outer loop is a |while(1)|? Should we be
bailing in such a case?

+	 nsCOMPtr<nsIContent> content (do_QueryInterface(matchAnchorNode));
+	 nsresult rv = NS_ERROR_UNEXPECTED;
+	 if (content)
+	   rv = mIterator->PositionAt(content);
+	 frag = 0;
I don't think we can get an infinite loop from that, since we reset
matchAnchorNode to nsnull right after that clause, before we loop around.  So
we'll go through the loop looking to start a new match, and we can't go back to
the problematic node (the one that didn't QI to nsIContent) because we've
already forgotten about that node and moved on.

I can outdent the comment if you prefer it that way.  (I actually indented it on
purpose, though that made it look more like it applied to the previous line than
to the next line, but I don't feel strongly about it.)
Comment on attachment 81113 [details] [diff] [review]
diff -u

I just mentioned the comment thing, cause I didn't see any of the other
comments indented that way. It's up to you. :-)
Attachment #81113 - Flags: superreview+
Whiteboard: FIX IN HAND, needs r, sr → FIX IN HAND, reviewed
Fixed (on the trunk).
Closed: 23 years ago23 years ago
Resolution: --- → FIXED
QA Contact: pmac → sairuh
vrfy'd fixed on all platforms using 2003.01.08 trunk bits.
Product: Core → Mozilla Application Suite
Depends on: 830549
You need to log in before you can comment on or make changes to this bug.