Type ahead find will go up before it goes down

VERIFIED FIXED

Status

()

Core
Keyboard: Navigation
VERIFIED FIXED
16 years ago
16 years ago

People

(Reporter: Aaron Leventhal, Assigned: Aaron Leventhal)

Tracking

Trunk
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

16 years ago
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)
(Assignee)

Updated

16 years ago
Blocks: 30088
OS: Windows 2000 → All
Hardware: PC → All
(Assignee)

Updated

16 years ago
No longer blocks: 30088
Depends on: 30088
(Assignee)

Comment 1

16 years ago
Created attachment 93681 [details] [diff] [review]
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

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

r=akkana
Attachment #93681 - Flags: review+
roc+moz@cs.cmu.edu should have a look at these nsIViewManager changes, if he's
ok with them I'll sr.
(Assignee)

Comment 4

16 years ago
Created attachment 94522 [details] [diff] [review]
Much better view manager solution

IsRectVisible becomes GetRectVisibility which returns enum - with no extra work
Attachment #93681 - Attachment is obsolete: true
(Assignee)

Comment 5

16 years ago
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
Attachment #94522 - Flags: review+
Comment on attachment 94522 [details] [diff] [review]
Much better view manager solution

sr=jst with roc+moz@cs.cmu.edu's approval of this change.
Attachment #94522 - Flags: superreview+
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.
(Assignee)

Comment 8

16 years ago
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.
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?
(Assignee)

Comment 10

16 years ago
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?
I'll trust you. Go ahead and check in :-)
(Assignee)

Comment 12

16 years ago
checked in
Status: NEW → RESOLVED
Last Resolved: 16 years ago
Resolution: --- → FIXED
(Assignee)

Updated

16 years ago
No longer depends on: 30088
typeahead find follows the expected order described in comment 0. vrfy'd fixed
using 2002.09.24.08 mozilla trunk builds.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.