Aaron Leventhal Reporter
	Description • 23 years ago

assume user press 'A', and:
1. the list has neither shortcut key 'A', nor first letter 'A' 
   -- do nothing
2. the list has only one item which has either shortcut key 'A' or first 
letter 'A' 
   -- select it and do its action
3. the list has more than one item that has either shortcut key 'A' or first 
letter 'A' 
   -- go circularly within them but do not act them

For condition 3, we should also support incremental typing (IE's Favorities 
does not support).

	Aaron Leventhal Reporter
	Comment 1 • 23 years ago

Tree and outliner are undergoing major revision. There is no sense working on
this until that is finished.

	Kyle Yuan Assignee
	Comment 2 • 23 years ago

Oh, I've done a no sence patch :(

Should I put any patches here?

	Aaron Leventhal Reporter
	Comment 3 • 23 years ago

Kyle, it makes sense to wait until bug 110156 is fixed. Until then, you should
not work on any patches for this bug.

	Jan Varga [:janv]
	Comment 4 • 23 years ago

What about bug 104503 ?

	Aaron Leventhal Reporter
	Comment 5 • 23 years ago

Hmmm, didn't see that one before.

We also have bug 133365 for listbox.

I suggest we make bug 104503 the outliner bug, and keep bug 133365 a separate
bug for listbox.

Kyle Yuan is going to be working on these.

	Jesse Ruderman
	Comment 6 • 23 years ago

Do outliner items ever have "shortcut keys" or was that inadvertently copied
from the spec for menus?

When an outliner has multiple columns, what should Mozilla do?  Here are several
possibilities:
1. Use the leftmost text column
2. Use the widest text column
3. Use a specific column, specified by the outliner
4. Use the column by which the user has sorted the rows of the outliner
5. Search in all columns

3 and 5 make the most sense to me, but I haven't thought about it much or tried
other programs.

	Jesse Ruderman
	Comment 7 • 23 years ago

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

	Kyle Yuan Assignee
	Comment 8 • 23 years ago

I think 1 is good enough for the most user requirement. Of course 3 and 4 are 
better if they can be implemented. If the current active(focus) column can be 
determined, we should navigate by this column first. Otherwise, we should use 
the first column.

I dont like 5, it will confuse me where the focus will go next.

	Jan Varga [:janv]
	Comment 9 • 23 years ago

I would prefer having 1,3,4.
Also look at mpt's proposal in bug 104503.

	Kyle Yuan Assignee
	Comment 10 • 23 years ago

Since bug 110156 fixed, we should change the widget's name.
"outliner" -> "tree"

	Kyle Yuan Assignee
	Comment 11 • 23 years ago

A new method was added into nsITreeBoxObject.idl -- getColumnID.
This ColumnID is used to get cell text.

Currently, I just try to match typing with text in the first column.

This patch is only for evaluation. Any comments are welcome.

	Jesse Ruderman
	Comment 12 • 23 years ago

mpt's proposal in bug 104503 was basically "3".  I agree with him -- I'd get
confused if selecting bookmarks or mail messages worked differently depending on
how I had sorted them.  mpt also suggested going to the first item starting with
'f' if I hit 'e' and no item starts with 'e', which seems like a good idea to me.

This feature would have to be disabled in the mail thread pane, because mail
uses single-letter shortcuts for things like "next unread message".

	Jesse Ruderman
	Comment 13 • 23 years ago

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

	Dean Tessman
	Comment 14 • 23 years ago

> mpt also suggested going to the first item starting with
> 'f' if I hit 'e' and no item starts with 'e', which seems like a good idea to me.

That's contrary to how windows behaves, and doesn't make any sense to me.
  * assume a list with "apple", "orange", and "tomato" in it
  * I type "p" because I'm looking for "pear"
  * "tomato" is selected.  WTF?  I wasn't looking for tomato.

Windows doesn't change the selection if the item doesn't exist.

	Jesse Ruderman
	Comment 15 • 23 years ago

If pressing 'p' does nothing, you'll think you mistyped the letter.  If it
selects 'tomato', you'll be able to see quickly that there is no item beginning
with 'p'.  I encountered that situation once.

	Jesse Ruderman
	Comment 16 • 23 years ago

On the other hand, if we're hoping to have a visual indication of which letters
you've typed so far, that won't work well if the item selected doesn't always
start with the letters you've typed.

	Dean Tessman
	Comment 17 • 23 years ago

I can't think of any widget in any app that behaves the way you describe.

	Kyle Yuan Assignee
	Comment 18 • 23 years ago

think there are 3 types of columns: first column, primary column, sorted 
column. (seems like there is not focused column). what's the priority of them 
when key-navigating? I prefer: sorted > primary > first.

	Kyle Yuan Assignee
	Comment 19 • 23 years ago

suit my previous comment

	Jan Varga [:janv]
	Comment 20 • 23 years ago

Kyle, your patch looks pretty good.
Some nits:
- the name GetPrimaryColumnIndex() is quite misleading,
  what about GetKeyColumnIndex() ?
- XUL developers should be able to enable/disable this feature,
  use enableKeyNavigation or disableKeyNavigation or something better (look at
  enableColumnDrag for example)
  not sure what should be default

	Dean Tessman
	Comment 21 • 23 years ago

I would think that key navigation should be true by default.  It's a pretty
standard feature that should be supplied unless the XUL author explicitly turns
it off.

	Kyle Yuan Assignee
	Comment 22 • 23 years ago

Thanks for your comments. I should add a new nsXULAtom -- disableKeyNavigation. 
If XUL author does not turn it on, we always do key navigation.

	Jan Varga [:janv]
	Comment 23 • 23 years ago

Just to be clear, you don't need a new nsXULAtom

add a new property
<property name="disableKeyNavigation"> (see enableColumnDrag for example)

and in keypress handler something like this:
else if (!this.disableKeyNavigation &&
         event.charCode >= 32 && event.charCode <= 122) {  // ' ' - 'z'

	Kyle Yuan Assignee
	Comment 24 • 23 years ago

1. Change the function's name to GetKeyColumnIndex
2. Add a new property disableKeyNavigation

Need r=

	Kyle Yuan Assignee
	Comment 25 • 23 years ago

1. Change the function's name to GetKeyColumnIndex
2. Add a new property disableKeyNavigation

Need r=

	Kyle Yuan Assignee
	Comment 26 • 23 years ago

Comment on attachment 77416 [details] [diff] [review]
patch (revised according to Jan's comments)


double clicked the "Submit" button :(

	Jan Varga [:janv]
	Comment 27 • 23 years ago

- you forgot to change getPrimaryColumnIndex() in tree.xml
- add disableKeyNavigation="true" to trees in folderPane.xul and threadPane.xul

other than that, r=varga

	Kyle Yuan Assignee
	Comment 28 • 23 years ago

done!

	Jan Varga [:janv]
	Comment 29 • 23 years ago

Comment on attachment 77568 [details] [diff] [review]
patch 2 (revised according to Jan's comments)

r=varga

	Aaron Leventhal Reporter
	Comment 30 • 23 years ago

Rather than .8 seconds, for the keyboard repeat delay on Windows I think we're
supposed to use:

GetSystemParametersInfo()
SPI_GETKEYBOARDDELAY Get settings for delay time of key inputs.

	Aaron Leventhal Reporter
	Comment 31 • 23 years ago

Spun off bug 135749, to complete XUL tree keyboard implementation.

	Dean Tessman
	Comment 32 • 23 years ago

Hrm, I don't know about that system parameter.  That's the amount of time before
keys start repeating, but I don't think it's tied to incremental searching.  I
have  a short delay but my incremental search delay seems unaffected.

It's 2500 ms in the patch not 800, isn't it?

Maybe if people complain we could look into moving this into nsLookAndFeel in
the future, to make it per-platform.

	Aaron Leventhal Reporter
	Comment 33 • 23 years ago

That's my reading of
http://msdn.microsoft.com/library/default.asp?url=/library/en-us/dnacc/html/ATG_KeyboardShortcuts.asp

	Dean Tessman
	Comment 34 • 23 years ago

More descriptive information from the API Help:

SPI_GETKEYBOARDDELAY
Retrieves the keyboard repeat-delay setting. The pvParam parameter must point to
an integer variable that receives the setting.
SPI_GETKEYBOARDSPEED
Retrieves the keyboard repeat-speed setting. The pvParam parameter must point to
a DWORD variable that receives the setting.

This say to me that these settings are only for key repeat.  But try it for
yourself.  Change your key repeat settings and see if they affect the
incremental search window at all.

	Matthew T (active 1999-2002)
	Comment 35 • 23 years ago

Whoo, this is looking nice.

+                 event.charCode >= 32 && event.charCode <= 122) {  // ' ' - 'z'

This won't work for characters like ä and Ú, when it should. I don't see any
need to limit the possible characters -- surely control characters will be
ignored already?

+           if (event.timeStamp - this._lastKeyTime > 2500)

On Mac OS this appears to be hard-coded to 1000, not varying with any key-repeat
settings.

-  <tree id="folderTree" class="plain" flex="1" seltype="single"
+  <tree id="folderTree" class="plain" flex="1" seltype="single"
disableKeyNavigation="true"

Please don't do that. The folder pane is perhaps where this feature would be
most useful, since the items in it are usually all visible and well-known, and
since scrolling through the list one item at a time can be very slow.

	Kyle Yuan Assignee
	Comment 36 • 23 years ago

Hi, Matthew, thanks for your comment 
1. I think your are right. The charCode of control key is always 0, so we can 
modify this condition to (event.charCode > 0);
2. Do you think we should change the interval to 1000ms for all platform? This 
value is same as bug 92491 (XUL menu) and bug 133365 (XUL listbox);
3. Jan suggest to do this on comment 27.

	Kyle Yuan Assignee
	Comment 37 • 23 years ago

tested in mail module, when the focus is in folder pane, you press "n", the 
focus will jump to the first unread message in this folder. It still works even 
the keyboard navigation feature is on.

	Kyle Yuan Assignee
	Comment 38 • 23 years ago

changes:
1. event.charCode >= 32 && event.charCode <= 122 =>
   event.charCode > 0
2. if (event.timeStamp - this._lastKeyTime > 2500) =>
   if (event.timeStamp - this._lastKeyTime > 1000)
3. remove disableKeyNavigation="true" from folderpane.xul

Jan, can you r= again?

	Jan Varga [:janv]
	Comment 39 • 23 years ago

Comment on attachment 78862 [details] [diff] [review]
patch (revised according to mpt's comments)

looks good, the lower value seems to be much better
r=varga

	Jan Varga [:janv]
	Comment 41 • 23 years ago

Comment on attachment 80365 [details] [diff] [review]
change nothing, just keep the patch up to date.

r=varga

	Jesse Ruderman
	Comment 42 • 23 years ago

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

	Kyle Yuan Assignee
	Comment 43 • 22 years ago

New interface method was checked in with bug109217. Only keep the changes of
tree.xml.

	Jan Varga [:janv]
	Comment 44 • 22 years ago

Comment on attachment 83470 [details] [diff] [review]
something was changed in trunk, keep the patch up to date.

r=varga

	Johnny Stenback (:jst)
	Comment 45 • 22 years ago

Comment on attachment 83470 [details] [diff] [review]
something was changed in trunk, keep the patch up to date.

sr=jst

	Kyle Yuan Assignee
	Comment 46 • 22 years ago

checked into trunk!

	Daniel Bratell
	Comment 47 • 22 years ago

This has (probably) caused problems in MailNews. See bug 145532.

	David :Bienvenu
	Comment 48 • 22 years ago

hey, this broke keyboard navigation in the thread pane (not acceptable!) and was
not reviewed by a mail/news person!

	David :Bienvenu
	Comment 49 • 22 years ago

if the focus is in the folder pane, and you press "n", it selects the first
folder who's name starts with "n".  It could be that this needs to be disabled
for the folder pane.

	Aaron Leventhal Reporter
	Comment 50 • 22 years ago

Kyle needs to take care of this. We can back him out or wait until he gets to
work He's in Beijing, so that should be around 4 pm California time.

	Jan Varga [:janv]
	Comment 51 • 22 years ago

There is already a patch in 145532.

	Joe Maimon
	Comment 52 • 22 years ago

The thread seems to have died down here, but I would like to report that as of
moz 1.1 navigating by keys seems to only work for first letter and also (more
importantly) seems to uncheck my mail filter rules when I use it there - and yes
I am cycling through the many matches.

	shrirang khanzode
	Comment 53 • 22 years ago

.