92102 - Finding in frames always wraps through the frames

Status: VERIFIED FIXED

(Core :: DOM: Editor, defect, P4)

Description

[Conrad Carlen (not reading bugmail)]

This is probably an RFE. When searching in multiple frames, it doesn't seem
possible not to wrap through the frames. The frame enumeration, when it comes to
the last frame, starts again at the first. We don't have UI to control this but,
from the API, I don't see how it would be possible even if we did. I would think
that the "Wrap Around" check box (although it has diferent meaning) would also
control this as well.

Comment 1

[rubydoo123]

this has been requested by other folks as well -- see related bug in bugscape 7869

Comment 2

[Simon Fraser [no longer active]]

To fix this, we need:
1. A checkbox in the Find dialog "Search through all frames" which is enabled
   when you are on a framed site.
2. The Text Services code needs to expose an API that Find can use to remember
   the starting position in the document.
3. The Find code needs to be smarter, and use the above features.

Comment 3

[Simon Fraser [no longer active]]

Embedding apps can control whether searching goes through all frames by setting 
nsIWebBrowserFind.searchFrame (Set/GetSearchFrames()). I just verifed that this 
works correctly by adding it to the XUL UI.

Note that there is a known problem with 'wrap' and finding in frames (bug 78305). 
If 'wrap' is on, it will only search the current frame, and never move onto the 
next frame. In addition, if searching through all frames, it won't correctly 
remember where you started searching from in the starting frame (so doesn't wrap 
correctly in a multi-frame world).

Is the current level of support sufficient to allow removal of the 'topembed' 
keyword?

Comment 4

[Marek Z. Jeziorek]

Removing topembed. [e-mail], do we need features beyond what's
implemented?

Comment 5

[Alex Weyers]

I think the present issues are fine for now.

As long bug 78305 is fixed at some point in the future, we can make do with the 
present find-in-frames implementation.

Comment 6

[Simon Fraser [no longer active]]

Boing.

Comment 7

[Simon Fraser [no longer active]]

To 0.9.8

Comment 8

[Jennifer Kobayashi]

I show a vantive bug open on this with stopper status, so apparently they are 
waiting for a fix. According to my guidelines, since there's an associated 
vantive bug, this bug should have kw: topembed and edt0.9.4. Adding these 
keywords, as instructed. If this is a mistake and there are objections, let 
Marek know (or myself). 

Comment 9

[Simon Fraser [no longer active]]

Maybe akk would like to tackle this along with the other Find stuff?

Comment 10

[Marcia Knous [:marcia]]

EDT - Marek has exchanged emails with Mike Golay, per this email trail we are
not going to implement it in 0.9.4.

Comment 11

[Michael Dunn]

Re-nominating pending discussion with embedding stakeholders

Comment 12

[Michael Dunn]

Again minusing for 094 after dev discussion

Comment 13

[Akkana Peck]

In response to Simon's earlier question: this is unrelated to the find stuff I'm
working on right now, but after that lands and any XBL changes from Mike's
embedding branch have stabilized, extending new find to do this should become
somewhat easier.

Comment 14

[Conrad Carlen (not reading bugmail)]

Taking.

Comment 15

[Conrad Carlen (not reading bugmail)]

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

Comment 16

[Conrad Carlen (not reading bugmail)]

Mass move to 0.9.9

Comment 17

[Simon Fraser [no longer active]]

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

Comment 18

[Conrad Carlen (not reading bugmail)]

Mass move to 1.0.1

Comment 19

[Akkana Peck]

Taking this -- I have a fix.

Comment 20

[Akkana Peck]

Attachment: Proposed fix

I may not understand the bug correctly, so I guessed at the intended behavior.

What this patch does:
(1) Search to the end of the current frame.
(2) Continue searching through frames until we get to the end of the document.
(3) If wrapping is off, return, else:
  (4) Start again at the beginning of the first frame of the document, and
continue searching until we get back to the start frame.
  (5) Search from the beginning of the starting frame to the starting point.

Please enlighten me if I mis-guessed what it's supposed to do.

Requesting sr from Simon, since he wrote the frames-looping code, and review
from someone else (Valeski?  Conrad?)

Comment 21

[Conrad Carlen (not reading bugmail)]

> What this patch does:

Thanks for the description - that's exactly how I think it should behave.

Comment 22

[Conrad Carlen (not reading bugmail)]

Comment on attachment 79845 [details] [diff] [review]
Proposed fix


Is part of some other patch - to skip over certain kinds of nodes?

>Index: nsFind.cpp
>===================================================================
>RCS file: /cvsroot/mozilla/embedding/components/find/src/nsFind.cpp,v
>retrieving revision 1.10
>diff -u -r1.10 nsFind.cpp
>--- nsFind.cpp	27 Mar 2002 03:47:33 -0000	1.10
>+++ nsFind.cpp	18 Apr 2002 19:19:16 -0000
>@@ -71,6 +71,7 @@
> nsIAtom* nsFind::sHRAtom = nsnull;
> nsIAtom* nsFind::sCommentAtom = nsnull;
> nsIAtom* nsFind::sScriptAtom = nsnull;
>+nsIAtom* nsFind::sNoframesAtom = nsnull;

In terms of wrapping behavior, looks good - and the comment describing the
wrapFind attribute of nsIWebBrowserFind is still good. r=ccarlen

Comment 23

[Akkana Peck]

Sorry, I was going to explain about that when I attached the patch, but forgot
it. In using framed pages to test this, I discovered that Find was finding text
inside the noframes tag (so the visible effect was that the user clicks Find and
the selection disappears).  I don't actually encounter framed pages that often
these days, so it was the first time I'd seen it, and I don't know of an
existing bug on it, so I fixed that while I was there.

Comment 24

[Akkana Peck]

Is there any embedding-type person who can sr this?

Comment 25

[Conrad Carlen (not reading bugmail)]

> Is there any embedding-type person who can sr this?

How about Alec?

Comment 26

[Alec Flett]

Comment on attachment 79845 [details] [diff] [review]
Proposed fix

ok, seems reasonable. sr=alecf

Comment 27

[Akkana Peck]

Fixed.

Comment 28

[sairuh (rarely reading bugmail)]

vrfy'd fixed: unless "wrap around" is selected, searching is limited to the
active frame. tested with 2003.01.08 trunk builds on all platforms.