250429 - Find Toolbar: 'find again' does not work after changing tabs Find text should persist across tabs and windows (but not sessions.)

Status: VERIFIED FIXED

(Toolkit :: Find Toolbar, defect)

Description

[Oakwine]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7) Gecko/20040707 Firefox/0.9
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7) Gecko/20040707 Firefox/0.9

FAYT used to keep the search criteria when switching between tabs.  This was
useful when searching through a number of Google search results; I could close
the current tab, hit Ctrl-G on the next tab, and search for the same textstring.

With the new Find Toolbar, the textbox clears when you go to the next tab.

Reproducible: Always
Steps to Reproduce:
1. Go to http://jargon.net/jargonfile/b/
2. Middle-click on the first two links to open two new tabs
3. Switch to the first-created tab (B1FF)
4. Type {/the} to search for the string 'the'.
5. Type Ctrl-W to close the tab and switch to the next tab (BAD)
6. Type Ctrl-G to search for 'the' again

Actual Results:  
The textbox on the Find Toolbar is empty.  No search occurs.

Expected Results:  
The textbox on the Find Toolbar still contains the word 'the', and the first
occurence of the word 'the' is highlighted.

Comment 1

[Jon Henry]

I think this might be the desired behavior, to give each tab its own find
toolbar, although I can see an argument for both ways.

Confirming, because this does happen, but it might get marked invalid eventually.

Comment 2

[Oakwine]

I realize that the current behavior might be intentional, but I think that the
opposite would be expected.  As precedent, I would point to the Find sidebar
behavior of Adobe Acrobat 6.02.  I don't know of any precedents with the
behavior shown by Firefox.

Comment 3

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

-> Find Toolbar component.

Comment 4

[Oakwine]

The Find Toolbar changed behavior with the 20040722 build, so I changed the
summary slightly (it was 'Find Toolbar: does not persist between tabs', which
wasn't very clear).  The Steps to Reproduce remain the same, but I need to
change the Results section, as follows:

Actual Results: The Find Toolbar appears with nothing in it, and the Find
Toolbar is highlighted red.

Expected Results: The Find Toolbar appears with the word 'the' in it, and the
first occurrence of the word is highlighted green.

Comment 5

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

not sure if this is still a bug... works for me in an 0725 build. 

Comment 6

[Oakwine]

(In reply to comment #5)
> not sure if this is still a bug... works for me in an 0725 build. 

I still see this bug in an 0802 build, with the following change:

Actual Results: The Find Toolbar appears with nothing in it.  The text is not found.

I experimented further with the current behavior, and discovered that you can
search for 'this' on Tab A, 'that' on Tab B, switch back to Tab A, and Ctrl-G
will find the next occurrence of 'this'.  The current behavior is clearly
planned; however, it feels like a regression to me.  It exhibits most obviously
when searching for content on Google; I will often open five or six tabs from a
search results page, then search each tab for the same textstring.  It was much
easier to accomplish this with the previous behavior.

Comment 7

[Asaf Romano (gone)]

Ye, i also expect it not to be on per-tab basis.

(Some desktops (e.g Mac OS X) use cross app. find, meaning: if you search for
something using the Find Dialog in App. A, the find string will be saved when
you are moving to another app)

Comment 8

[Michael Clackler]

Attachment: patch to fix 'find again' after changing tabs

This patch stores that last find completed in any tab for re-use with find
next/previous in another tab.

The last "stored" find string will only be used if:
1.  ctrl-g, shift-ctrl-g is used with the find bar hidden
2.  ctrl-g, shift-ctrl-g is used with the findbar open and the find field is
blank

Otherwise, if the find bar is visible and there is a current string in the find
field, with ctrl-g or shift-ctrl-g the current find field string will be used. 
The find next/previous buttons will allways use the text in the find field.

This patch also fixes the following bugs:
bug 255193 - "Find in this page" is stuck to an old tab after tab focu...
bug 256420 - Find toolbar doesn't disappear after 5 seconds if it was ...

Comment 9

[Michael Clackler]

Attachment: patch to fix 'find again' after changing tabs, v2

A minor tweak to ensure a "not found" ctrl-g find shows that appropriate not
found state.

Comment 10

[Ng Ming Hong]

*** Bug 259868 has been marked as a duplicate of this bug. ***

Comment 11

[Yves Bellefeuille]

Version 1.0 PR.
Mozilla/5.0 (X11; U; Linux i686; rv:1.7.3) Gecko/20040914 Firefox/0.10

The old Find function, Ctrl-F, (not FAYT) had the behaviour the original poster
reported, i.e. the find text persisted across tabs and windows. Ctrl-F now calls
the Find Toolbar, which has a different behaviour. This is inconvenient for
those of us who are used to the behaviour of Ctrl-F and wish to continue using
it that way.

Therefore, I agree that "Find text should persist across tabs and windows", as
least for those who use plain Find instead of FAYT.

Comment 12

[Leo]

A hidden pref (ie, to use about:config) to change between having the find text
persists between tabs and not doing so would be great.

The find text persisting between tabs is great for visiting news sites that
require you to hit "Complete Story" style links to see the full article.
It means one needs only hit F3(or ctrl+g?) to go to the complete story link and
hit enter intead of scrolling down and clicking it.

Comment 13

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

Comment on attachment 156708 [details] [diff] [review]
patch to fix 'find again' after changing tabs, v2

>+      var gFindService = Components.classes["@mozilla.org/find/find_service;1"]
>+                           .getService(Components.interfaces.nsIFindService);

Never prefix locals with 'g'

>+    } catch(e) {
>+      dump("No find service!\n"); gFindService = 0;

Why the try..catch? Is there a reason other than out-of-memory that getService
of the find service might throw? If not, ditch the wordy error handling
altogether. 

... snip ...

I assume the rest of this and that in onFindPreviousCmd was copied from
elsewhere. Please put all of it into a separate function to cut down on code
duplication.

Comment 14

[Uri Bernstein]

*** Bug 264120 has been marked as a duplicate of this bug. ***

Comment 15

[Blake Ross]

Fixed.

Appreciate the patch, Michael, but I decided to do this another way. Basically,
I made it so all browsers within a given tabbrowser share the same instance of
nsITypeAheadFind.

Comment 16

[Jesse Ruderman]

*** Bug 265168 has been marked as a duplicate of this bug. ***

Comment 17

[:deanis74]

Verified fixed using 20041019.

Comment 18

[Uri Bernstein]

*** Bug 270617 has been marked as a duplicate of this bug. ***