158756 - Type ahead find will go up before it goes down

Status: VERIFIED FIXED

(Core :: DOM: UI Events & Focus Handling, defect)

Description

[Aaron Leventhal]

Type ahead find (implemented in bug 30088), prefers to find something that's
already visible. If it can't, right now it will start over at the beginning of
the document.

It should prefer matches in this order:
1. Something visible
2. Something below what's currently visible
3. Something above what's currently visible (start at the top again)

Comment 1

[Aaron Leventhal]

Attachment: Changes nsIViewManager::IsRectVisible()

Changes nsIViewManager::IsRectVisible() to also set *aIsBelowViewPort =
PR_TRUE, if the aRect passed in is below the current viewport. With this patch,
type ahead find now treats something below the current viewport as just as good
as if it's in the current viewport.

Comment 2

[Akkana Peck]

Comment on attachment 93681 [details] [diff] [review]
Changes nsIViewManager::IsRectVisible() 

r=akkana

Comment 3

[Johnny Stenback (:jst)]

[e-mail] should have a look at these nsIViewManager changes, if he's
ok with them I'll sr.

Comment 4

[Aaron Leventhal]

Attachment: Much better view manager solution

IsRectVisible becomes GetRectVisibility which returns enum - with no extra work

Comment 5

[Aaron Leventhal]

Comment on attachment 94522 [details] [diff] [review]
Much better view manager solution

Carrying forward Akkana's r= on the non-viewmanager parts. Seeking r=roc+moz
for the viewmanager parts

Comment 6

[Johnny Stenback (:jst)]

Comment on attachment 94522 [details] [diff] [review]
Much better view manager solution

sr=jst with [e-mail]'s approval of this change.

Comment 7

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 93681 [details] [diff] [review]
Changes nsIViewManager::IsRectVisible() 

r=roc+moz for the view manager changes

The changes are OK but this whole thing is a little weird. What's supposed to
happen when content is outside the viewport to the left or right? It looks like
you start at the top again.

Comment 8

[Aaron Leventhal]

No, the line:
if (rectVisibility != eAboveViewport && rectVisibility != eZeroAreaRect)
  return PR_TRUE;

means that if we're visible, left, right, or below then we can be a successful
match. It's only when the current match is above the viewport that I want to try
for a better match. It will wrap around to get a match above the viewport if
need be.

Comment 9

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Sorry, I was commenting on the old patch by mistake.

The new patch is better. However, I hate to be a pest but "ERectVisibility"
isn't consistent with the other names used in views and Gecko. How about
"nsRectVisibility", with constants "nsRectVisibility_Visible", etc? Sound OK?

Comment 10

[Aaron Leventhal]

Okay, np I'll change the names to be more consistent. Jst do I need a new patch
or do you trust me on the name changes?

Comment 11

[Robert O'Callahan (:roc) (email my personal email if necessary)]

I'll trust you. Go ahead and check in :-)

Comment 12

[Aaron Leventhal]

checked in

Comment 13

[sairuh (rarely reading bugmail)]

typeahead find follows the expected order described in comment 0. vrfy'd fixed
using 2002.09.24.08 mozilla trunk builds.