Closed Bug 92102 Opened 23 years ago Closed 22 years ago

Finding in frames always wraps through the frames


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






(Reporter: ccarlen, Assigned: akkzilla)



(Keywords: embed, topembed-, Whiteboard: reviewed, needs sr)


(1 file)

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.
this has been requested by other folks as well -- see related bug in bugscape 7869
Priority: -- → P4
Target Milestone: --- → mozilla0.9.5
Keywords: topembed
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.
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' 
Removing topembed., do we need features beyond what's
Keywords: topembed
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.
Target Milestone: mozilla0.9.5 → mozilla0.9.6
Blocks: 106961
Target Milestone: mozilla0.9.6 → mozilla0.9.8
To 0.9.8
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). 
Keywords: edt0.9.4, topembed
Maybe akk would like to tackle this along with the other Find stuff?
EDT - Marek has exchanged emails with Mike Golay, per this email trail we are
not going to implement it in 0.9.4.
Keywords: edt0.9.4edt0.9.4-
Re-nominating pending discussion with embedding stakeholders
Keywords: edt0.9.4-edt0.9.4
Again minusing for 094 after dev discussion
Keywords: edt0.9.4edt0.9.4-
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.
Assignee: sfraser → ccarlen
*** Bug 82708 has been marked as a duplicate of this bug. ***
Mass move to 0.9.9
Target Milestone: mozilla0.9.8 → mozilla0.9.9
Target Milestone: mozilla0.9.9 → mozilla1.0
*** Bug 78305 has been marked as a duplicate of this bug. ***
Keywords: topembedtopembed+
Mass move to 1.0.1
Target Milestone: mozilla1.0 → mozilla1.0.1
Keywords: topembed+embed, topembed-
Taking this -- I have a fix.
Assignee: ccarlen → akkana
Attached patch Proposed fixSplinter Review
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?)
> What this patch does:

Thanks for the description - that's exactly how I think it should behave.
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
Attachment #79845 - Flags: review+
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.
Is there any embedding-type person who can sr this?
Whiteboard: reviewed, needs sr
> Is there any embedding-type person who can sr this?

How about Alec?
Comment on attachment 79845 [details] [diff] [review]
Proposed fix

ok, seems reasonable. sr=alecf
Attachment #79845 - Flags: superreview+
Closed: 22 years ago
Resolution: --- → FIXED
QA Contact: sujay → sairuh
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.
You need to log in before you can comment on or make changes to this bug.