253793 - Find in page won't stop if you go to the next page

Status: VERIFIED FIXED

(Toolkit :: Find Toolbar, defect, P3)

Description

[Peter van der Woude [:Peter6]]

Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.7) Gecko/20040730
Firefox/0.9.1+

1. Open a page
2. press ctrl+F and type the letter "a" in the findbar
3. press highlight
4. close the findbar
5. goto bookmarks and open another random page
6 .note that all "a"'s are highlighted even though you closed the findbar

to prevent this to happen you must clear the findbar before you "close" it

expected:
to stop searching/highlighting if the findbar is closed.

Comment 1

[Michael Clackler]

Attachment: patch to fix

This patch resets the highlight find setting to OFF when opening a new URL in
the current window by using a bookmark.

Comment 2

[Michael Clackler]

Comment on attachment 154953 [details] [diff] [review]
patch to fix

request review

Comment 3

[Michael Clackler]

requesting blocking-aviary1.0PR since patch exists

Comment 4

[Raanan Barzel]

Reply to Comment #1: Closing the findbar should be considered as the end of
interest in the results of the search. Consequently, resetting the highlight
find setting when changing URL is not logical: a user may well be searching for
something over several URLs. On the other hand, text which was highlighted
should be un-highlighted when the findbar is closed.

Comment 5

[Michael Clackler]

(In reply to comment #4)
> Consequently, resetting the highlight
> find setting when changing URL is not logical: a user may well be searching for
> something over several URLs. On the other hand, text which was highlighted
> should be un-highlighted when the findbar is closed.
> 

The patch only turns off the highlight when opening a new url through a
bookmark.  If you click a url on the page, the highlight stays on.  For example,
if using a search engine the highlight will remain as you click through pages. 
If you use a bookmark to go to a new URL, the highlight will turn off but the
find value will remain.  If you want the highlight on the new URL, all you need
to do is re-click highlight.

I think for most people when they use a bookmark to change pages, instead of
surfing through the page, they are finished with the original reason for the
highlight request.

Comment 6

[Raanan Barzel]

(In reply to Comment #5)

What bothered me was that found (and highlited) items remained highlighted even
after I closed the Findbar.

I agree with the strategy regarding changing URLs.

Comment 7

[chris hofmann]

one liner.

blake, can you review?

Comment 8

[Ben Goodger (use ben at mozilla dot org for email)]

Comment on attachment 154953 [details] [diff] [review]
patch to fix

1. Please use -p when making patches. 
2. This would be best (and more reliably) done in onLocationChange in the
browser's status handler
3. Use .checked = false, not removeAttribute("checked").

Comment 9

[Michael Clackler]

(In reply to comment #8)
> 2. This would be best (and more reliably) done in onLocationChange in the
> browser's status handler

Is there a place in onLocationChange that is active only when using a bookmark
to change the location, or is this active on any change (including "clicking"
through a page URL)?

The reason I chose the location in the patch was I followed the code to this
location from following the bookmark code.

I really only want the highlight to reset if a bookmark is used, not if a url is
clicked in a page.  See comment #5 for more explanation. 

Comment 10

[Ben Goodger (use ben at mozilla dot org for email)]

I might agree with you except for the bugs with highlight that are unlikely to
be fixed prior to 1.0 - best to have it disable fully for now when the page
changes. 

Comment 11

[Michael Clackler]

Attachment: patch with changes ben requested

This patch turns off the highlight in onLocationChange as Ben requested.  

I tried ".checked = false" instead of ".removeAttribute("checked")", but it did
not work.

Comment 12

[Michael Clackler]

Attachment: patch

Same as the last patch, just removes errant blank line

Comment 13

[Ben Goodger (use ben at mozilla dot org for email)]

Comment on attachment 161612 [details] [diff] [review]
patch

r+a=[e-mail]

Comment 14

[Steffen Wilberg]

Branch checkin 2004-10-13 15:24.

Comment 15

[u58943]

(In reply to comment #10)
> I might agree with you except for the bugs with highlight that are unlikely to
> be fixed prior to 1.0 - best to have it disable fully for now when the page
> changes. 

and then go to all the hassle of someone submitting another bug somewhere down
the line for this to behave as it should, meanwhile everyone has the hassle of
trying to re-search every page they want to highlight, when it could be fixed
here and now and not need touching again.

Comment 16

[sairuh (rarely reading bugmail)]

vrfy'd fixed with 200410150x-0.9+ bits on linux fc2 and mac os x 10.3.5.