Closed Bug 132612 Opened 22 years ago Closed 22 years ago
'Find in This Page' does not find all matches
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
Assignee: sgehani → akkana
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 ***
Status: UNCONFIRMED → RESOLVED
Closed: 22 years ago
Resolution: --- → DUPLICATE
Status: RESOLVED → VERIFIED
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.
Status: VERIFIED → UNCONFIRMED
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 again). 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 testing. 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.
qa --> email@example.com
QA Contact: paw → pmac
Yes, now I see it in your query.
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
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.
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.
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.
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 r=brade
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 firstname.lastname@example.org 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+
Fixed (on the trunk).
Status: ASSIGNED → RESOLVED
Closed: 22 years ago → 22 years ago
Resolution: --- → FIXED
vrfy'd fixed on all platforms using 2003.01.08 trunk bits.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.