100186 - Dropdown/listbox keyboard navigation & issues

Status: VERIFIED FIXED

(Core :: Layout: Form Controls, defect)

Description

[jhp (no longer active)]

This bug is for several fixes that I implemented a few months ago bug forgot to
open a bug for.  It fixes several issues, but since they all apply to combobox
navigation, I decided to put them in one bug.  If you want me to split this up,
let me know.  Also, this goes beyond just specifying keys to enable drop-down,
as in 57192.

Here are the issues:
1) Alt+Down/F4 to drop down menu as in 62425/57192. 
2) Any keyboard navigation (using arrows, page down, etc), should pop down the
dropdown. This is would fix two issues.  First, when the drop down is tied to
javascript (such as in
http://www.pc.ibm.com/support?lang=en_US&page=brand&brand=IBM+NetVista&family=IBM+NetVista%7CNetVista+personal+computer&machineType=&model=&doctype=&subtype=All),
any new selection causes the page to reload;  having it pop down first stops
this behavior.  Secondly, it's good to see what you are selecting.
3) One should be able to select multiple elements in listboxes using
PageDown/PageUp/Home/End.
4) After implementing F4 to toggle dropdown, pressing 's' to select word will
open/close dropdown (since 's' = 'F4' = 0x74)

Comment 1

[jhp (no longer active)]

Attachment: patch for nsListControlFrame.cpp

Comment 2

[jhp (no longer active)]

Proposed fix above.  Most of the fixes are self-explanatory;  however, in
regards to (4) above, I simply split the character code from the key code
variable, since the lower case character codes overlap the values of some of the
DOM_VK_* values.

Comment 3

[Mike Kaply [:mkaply]]

This is an accessibility issue as well.

Comment 4

[Aaron Leventhal]

I recommend we follow IE's behavior. F4 is only used to drop down the list on
the URL bar.

Alt+Down is used for all other popups.

Comment 5

[Aaron Leventhal]

Item #1 is covered by bug 57192

I believe Item #2 in the bug description is invalid. That's what Alt+Down is for
- to pop open the list. Typing a letter or pressing down arrow should not affect
whether the list is open. We neeed to keep our HTML keyboard support as close to
the IE standard as possible.

Item #3 - multiple selections is another issue, it is possible to use a multi
select listbox with the keyboard now, using Shift+Up and Shift+Down

Item #4 - we don't need to implement F4 for this. See previous bugzilla comment.

Therefore, I believe this bug is INVALID, since all the cases needed are covered
by bug 57192.

Comment 6

[Mike Kaply [:mkaply]]

IE is not a standard.

We should do the right thing.

In all other apps except IE including Nav 4.x, F4 pops down all combos.

As far as the typing letter popping down the combobox, the current behavior on 
IE and Mozilla and Netscpae is BROKE for accessibility.

A lot of sites key the Javascript off of the change of the combo, and when you 
hit a letter, it causes this change.

See the IBM site mentioned.

Comment 7

[jhp (no longer active)]

I have to agree with Kaply here.  The Mozilla project is not about making an IE
clone.  F4 seems to be the accepted way to drop down comboboxes, at least on
windows and os/2.

Also, this patch fixes the issue mentioned by Neil in bug 57192: "Function keys
currently have interesting effects in HTML drop-down lists - they map to letters
F1=P ... F9=X"

Comment 8

[Boris Zbarsky [:bzbarsky]]

ccing jkeiser who is currently gutting nsListControlFrame.cpp

You guys may want to coordinate to avoid conflicts....

Comment 9

[Aaron Leventhal]

I didn't mean to create a big deal about it -- but in most cases I feel we
should follow IE on keyboard stuff unless there is a good reason not to.

I would go along with F4 to pop open list boxes.

However, I'm not convinced that typing other things should open the drop down
the list box though --  do you know any other windows apps that do that? Or is
that strictly OS/2 behavior?

Comment 10

[jhp (no longer active)]

It is not the behavior for any application I know of.  I just thought it would
help with keyboard navigation give (1) the above javascript problem (given the
current implementation, I just use the arrow keys or type the letter to get to
the desired item;  i rarely think of doing alt+down in order to pop down the
listbox and then select) and (2) that when selecting, it would help to see the
contents of the listbox, rather than cycling one by one.  It does not introduce
 any harmful side-effects; in fact, it speeds things up, since you don't have to
press alt+down every time you tab to a listbox.
Granted, I think this 'feature' is up for discussion.  However, I believe that
all the other points in the first post are valid.

Comment 11

[John Keiser (jkeiser)]

Yes, the code you referenced is going to be completely different (and easier to
work with) when bug 34297 gets its fix in.  Could we revisit then?

Comment 12

[rods (gone)]

->>

Comment 13

[jhp (no longer active)]

Attachment: patch based on new code

This patch implements F4-dropdown support.  Most of the other issues were taken
care of by the rewrite or there are bugs already open for them.

Comment 14

[jhp (no longer active)]

Attachment: patch: code -> keycode

Comment 15

[John Keiser (jkeiser)]

Comment on attachment 64591 [details] [diff] [review]
patch: code -> keycode

Could you move the definition of charcode into the default: case?  It's not
used anywhere else and it's not good to have stray vars lying around.  Other
than that, it looks good.

Comment 16

[John Keiser (jkeiser)]

Comment on attachment 64591 [details] [diff] [review]
patch: code -> keycode

Could you move the definition of charcode into the default: case?  It's not
used anywhere else and it's not good to have stray vars lying around.  Other
than that, it looks good.

Comment 17

[John Keiser (jkeiser)]

Javier, give it a couple days before making any new patches if you could--bug
112241 is landing shortly that affects this code.

Comment 18

[jhp (no longer active)]

Attachment: update to trunk

How about this patch?

Comment 19

[John Keiser (jkeiser)]

Comment on attachment 66806 [details] [diff] [review]
update to trunk

Looks good to me, r=jkeiser.  I like the keycode/charcode change, it makes it
less deceptive :)

Please note that bug 112713 is getting close to fulfillment, so you may want to
hold off on major functionality fixes to the select frame for the time being. 
I'll still be happy to accept them, but our work on the frame will (thankfully)
ultimately be in vain.

Comment 20

[Kevin McCluskey (gone)]

Marking nsbeta1+

Comment 21

[michael wendell]

Attachment: testcase

This is just an HTML file with a dropdown.

Simple testcase.

Comment 22

[John Keiser (jkeiser)]

Javier?  I was talking to some of our crotchety UI guys and they tell me this
needs to be Windows-only (the F4 part at least).  I still like the patch, can
you resubmit with #ifdef XP_WIN around the F4 case and we'll go forth with sr=
and stuff?

Comment 23

[jhp (no longer active)]

Attachment: ifdefed F4 code

How's this?

Comment 24

[jhp (no longer active)]

Attachment: ifdefed F4 (2nd attempt)

Comment 25

[John Keiser (jkeiser)]

Comment on attachment 72479 [details] [diff] [review]
ifdefed F4 (2nd attempt)

r=jkeiser

Comment 26

[Marc Attinasi]

Comment on attachment 72479 [details] [diff] [review]
ifdefed F4 (2nd attempt)

sr=attinasi

Comment 27

[Asa Dotzler [:asa]]

Comment on attachment 72479 [details] [diff] [review]
ifdefed F4 (2nd attempt)

a=asa (on behalf of drivers) for checkin to the 1.0 trunk

Comment 28

[Mike Kaply [:mkaply]]

Fix checked in.

Javier, open an RFE bug for the "pop down as you type" idea.

Comment 29

[Madhur Bhatia]

verified