51550 - Find in This Page should only find visible content

Status: VERIFIED FIXED

(SeaMonkey :: UI Design, defect, P3)

Description

[ekrock's old account (dead)]

Using Commercial 2000090508 on WinNT 4.0 SP4.

To repro:
1) Go to cnn.com
2) Search-->Find in This Page
3) enter "address" (there are multiple instances of this word on the page)
4) hit return and keep finding the next instance

Expected: each time an instance of "address" is found, it will be highlighted
and the page will scroll so that instance is visible

Actual: instances off the visible screen area are being found and (presumably)
highlighted outside the visible area; you have to scroll manually to find them

Marking polish and Future since we don't have the time to fix this kind of minor
polish issue for RTM.

Comment 1

[Blake Ross]

cc beppe

Comment 2

[Simon Fraser [no longer active]]

This should work -- perhaps the autoscroll code broke it? cc kin.

Comment 3

[kinmoz]

This is my bug. The TextServices module is passing the find component the text 
that is between the <SCRIPT> tags in the html document. This JavaScript text 
also happens to use the variable name 'address' in several of it's methods, so 
we are finding them in the JavaScript, but since JavaScript text does not 
render, we don't scroll.

You'll find if you click enough times on the cnn page, that you'll eventually 
get past the JS 'address' variable instances and start seeing the page scroll.

Comment 4

[kinmoz]

In my last comment, I meant, if you click find again, with the word 
'address' enough times, you'll find that the page scrolls when it hits the word 
'address' that actually renders on the screen.

Comment 5

[kinmoz]

Accepting bug.

Comment 6

[Matthew T (active 1999-2002)]

[Resummarizing based on Kin's comment]

Comment 7

[Stephen Ostermiller]

How does this bug differ from bug 10470

In both cases the find is finding things that are not visible on the page.  My
guess is that fixing one, would likely fix the other?

Comment 8

[kinmoz]

Yeah, it's the same problem.

Comment 9

[sairuh (rarely reading bugmail)]

kin, should i dup this in favor of bug 10470, then?

Comment 10

[sairuh (rarely reading bugmail)]

just spoke with kin --will keep this one open. same backend issue, different
manifestation: good to keep open for qa purposes, at the very least!

Comment 11

[Simon Fraser [no longer active]]

Kin: the fix should be fairly simple, no? Just do a GetPrimaryFrame for the 
content, and if there is no frame, ignore this match.

Comment 12

[sairuh (rarely reading bugmail)]

clearing milestone for reconsideration and nominating for moz0.9.2.

sorry this fell off my radar, but it bit me recently on all platforms --eg,
searching for "mozilla" while viewing the tinderbox page. [insert disgruntled
feline noises. ;) ]

Comment 13

[kinmoz]

To answer Simon's question:

The find code knows nothing about the document structure since it uses the 
TextServicesDocument iterator.

The approach I was going to take, was to add some sort of filtering mechanism 
that would allow components that created the TextServicesDocument to specify 
which subtrees/nodes in the document leave out during the iteration process.

With this mechanism we could do things like skip over mail quoted text (which is 
visible) during spellchecking, or skip over invisible text (like JS, and form 
element text)during a find.

Comment 14

[Tuukka Tolvanen (sp3000)]

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

Comment 15

[Akkana Peck]

I have a new design for the Find code which does have access to the document
structure.  I'm hoping to avoid calls like GetPrimaryFrameFor (at least as much
as possible) for performance reasons.  Excluding script tags is easy (I already
have that in my code) but there will probably be other gotchas that will show up
as we test this.

Comment 16

[Asa Dotzler [:asa]]

Bugs targeted at mozilla1.0 without the mozilla1.0 keyword moved to mozilla1.0.1 
(you can query for this string to delete spam or retrieve the list of bugs I've 
moved)

Comment 17

[sairuh (rarely reading bugmail)]

mass moving open bugs pertaining to find in page/frame to [e-mail] as
qa contact.

to find all bugspam pertaining to this, set your search string to
"AppleSpongeCakeWithCaramelFrosting".

Comment 18

[rubydoo123]

removing myself from the cc list

Comment 19

[Akkana Peck]

Whoops, I kept this bug open because I was confusing it with a different one. 
As far as I know, this is fixed with the current find code.  The test case
(cnn.com) no longer contains the search string "address", so if there are any
remaining cases where this bug happens, please attach a current test case. 
Otherwise I'll consider it fixed.

Comment 20

[Patty Mac]

Verified on all platforms (netscape trunk build: 2002-11-26-08-TRUNK)