63241 - 'Find in this page' should search thru all frames, not just active one

Status: VERIFIED FIXED

(SeaMonkey :: UI Design, defect, P2)

Description

[sairuh (rarely reading bugmail)]

spun off from bug 52727... i couldn't find an existing one for this issue. pls
reassign if needed! anyhow, the summary sez it all: since mozilla/6.0 currently
doesn't show which frame is active, we should thus make 'Find in this page'
search through all of the frames on the page. a user shouldn't have to worry
about whether or not a page has frames --Find should just search through the
frames in order, without caring which one's in focus.

Comment 1

[Paul Chen]

nav triage team:
Would be nice, but.... Of course, a patch is ALWAYS welcome. ;-) Marking 
nsbeta1-

Comment 2

[rubydoo123]

this should go over to Bill

Comment 3

[Paul Chen]

Marking nsbeta1- bugs future

Comment 4

[Judson Valeski]

re-assess

Comment 5

[Simon Fraser [no longer active]]

No way joki will do this. It's XPApps work.

Comment 6

[Marek Z. Jeziorek]

does this belong to pchen. if yes, how can we make it for 0.9? if no, who does 
this belong to?

Comment 7

[Bill Law]

By rights, it should be assigned to me and/or Kin.  To make "find" work across 
a frame set requires either the code driving the find/replace (in 
mozilla/xpfe/components/src/nsFindComponent.cpp) to cycle through the frameset, 
or, the underlying text services document or nsIFindReplace to do so.

As to getting it in mozilla0.9...I'd look for a miracle.

Comment 8

[kinmoz]

I really think this should be handled by the app or front end nsFindComponent, 
since the backend TextServices Find code has no clue about what frame/document 
currently has focus, etc.

Comment 9

[Bill Law]

Agreed, except I'm holding out hope that if one could attach to the parent
frameset, then the editor components could take care of it.

How do nested <iframe>s work, currently?

Comment 10

[Simon Fraser [no longer active]]

You simply need to walk nsIDocShellTreeItem trees, which is fairly simple. I 
think the harder problem here is indicating to the user that you are doing this, 
and focussing the frame in which the result is found.

Comment 11

[Bill Law]

I would like to believe that nsIFindAndReplace::Find would do the right thing 
with respect to focus, etc.  It already manages scrolling the found text into 
view and showing selection.

My concern is in navigating the <frame> tree in the midst of the search.  It 
seems kind of hard to manage this outside of nsIFindAndReplace.  I think that 
interface, at a minimum, has to manage its nested (child) <iframe>s.  It is the 
only object with the necessary info to possibly know when to descend into the 
child <iframe>s.

As it stands, I can search a given document/window, but the only way to deal 
with child frames would be to detect the case where nsIFindAndReplace returned 
a failure on a search.  Currently, that comes only at the end of the document.  
If we started searching child <frame>s at that point, then the occurrences 
wouldn't be found in a linear fashion.  In a document with structure:
    <p>Some text.
    <iframe src="subframeContainingText.html"/>
    <p>Some more text.
then searching from the top for "text" would match the first line, then the 
last line, then the contents of the child <frame>.

Plus, we'd have to manage the wrap/search-backwards flags during this process
(we couldn't ever pass "wrap" into nsIFindAndReplace because then we'd never 
know to search child <frame>s).

But if nsIFindAndReplace were smart enough to recurse into its child <frame>s 
at the right time, then we could change nsBrowserInstance.cpp so that it 
searched starting at the root (GetContentWindow()) instead of 
GetFocusedContentWindow().  Although it would be nice to *start* the search at 
the focused content window...

We *could* manage the cross-frame searching from outside nsIFindAndReplace if 
we're willing to accept the resulting non-linear "search order" and the perhaps 
more complicated code that results from putting control outside of that 
interface.

In reply to Kin's comment about the lower level code not knowing about focus: I 
interpret this bug as saying that we basically want to ignore the focused 
<frame> when searching.  So not knowing about focus isn't necessarily a problem.

Is there any reason why we wouldn't want to deal with this in nsIFindAndReplace 
(and its nsITextServicesDocument)?

Comment 12

[Simon Fraser [no longer active]]

Do we really need to worry about the <iframe> case, or can we punt and just 
search each docShell in hierarchy order? Other browser actions (e.g. selection) 
don't treat <iframe> contents in-flow, and doing this would make the logic *much* 
harder.

Comment 13

[Simon Fraser [no longer active]]

I'm going to work on this.

Comment 14

[Simon Fraser [no longer active]]

cc valeski. We have to figure out whether this is doable without changing 
nsIWebBrowserFind, which is frozen.

Comment 15

[Simon Fraser [no longer active]]

To avoid writing all the code twice, I'm going to change the Find in browser to 
use the same code as embedding does. This bug is therefore dependent on bug 
68307.

Comment 16

[Judson Valeski]

IE 5.5 supports implicit multi frame searching. NS 4.x does not. NS 6.x does not.

It's not clear to me why this would have interface ramifications. we maintain a
text level cursor behind the scenes, why can't we just maintain a frame
(nsIDOMWindow) cursor back in the impl too? It would start w/ the current
focused frame, then wrap around the window list; no?

Comment 17

[Conrad Carlen (not reading bugmail)]

OK, if implicit multi-frame searching is supported on IE 5.5, that's a better
precedent than the split-pane text editor model I brought up. Forget my prior
reservations - I'm all for it now. 

Comment 18

[Marek Z. Jeziorek]

I set the target milestone to 0.9.1 (it's not needed in 0.9, but we need to have 
it in the following milestone).

Comment 19

[Marek Z. Jeziorek]

Simon, if this could be done for soon, please reset the milestone to 0.9.

Comment 20

[Simon Fraser [no longer active]]

I have this pretty much working in my tree, but the changes are quite large (I 
had to reorganise the embedding Find code that that I can use it in the browser).

I'd rather wait for 0.9.1, unless there is a real urgency to get it into 0.9.

Comment 21

[Simon Fraser [no longer active]]

Diffs coming. These diffs use the docShellEnumerator described bug 76758

Comment 22

[Simon Fraser [no longer active]]

Attachment: mozilla/embedding/components/find/nsWebBrowserFind.h

Comment 23

[Simon Fraser [no longer active]]

Attachment: mozilla/embedding/components/find/nsWebBrowserFind.cpp

Comment 24

[Simon Fraser [no longer active]]

This version of find deals with searching multiple frames, either forwards or 
backwards.

It does not yet correctly handle wrapping while finding in multiple frames, since 
the underlying nsIFindAndReplace needs some API changes for this (related to bug 
6915).

These changes supersede the nsWebBrowserFind versions in bug 68307.

Comment 25

[Simon Fraser [no longer active]]

Attachment: Final, tree-wide patch

Comment 26

[Simon Fraser [no longer active]]

Fixed.

Comment 27

[Marek Z. Jeziorek]

how difficult would this be to check this into 0.9 branch?

Comment 28

[Simon Fraser [no longer active]]

It's do-able. I have most of the changes in a tree.

Comment 29

[sairuh (rarely reading bugmail)]

vrfy fixed: 2001.05.18.13 [comm bits] on linux, mac and winnt.