Bug 15816 – Cannot select multiple non-adjacent items in a multiple select control with the keyboard only

• Status: RESOLVED
• Resolution: FIXED
• Component: Forms

[Benjamin Hawkes-Lewis]

You can select multiple non-adjacent items in a multiple select using Command-Click. It should be possible to do the same with the keyboard. In Firefox Mac you can press space to select an item, then press Control-Up or Control-Down to move through the options without losing your selection and press space to select additional items. Apple Documentation does mention a way to select multiple items with the VoiceOver cursor, but this doesn't seem to work in HTML select boxes.

http://docs.info.apple.com/article.html?path=VoiceOver/1.0/en/mh2083.html

You can use the simple multiple select test case above to confirm this.

[Benjamin Hawkes-Lewis]

When I say "this doesn't seem to work" above, I mean you can select items but it seems to be impossible to reliably preserve that selection when you start moving around in the list.

[Robert Blaut]

Confirmed.

[chris fleizach]

don't think this is just an accessibility problem (if it still happens)

[chris fleizach]

<rdar://problem/6100088>

[chris fleizach]

VoiceOver does handle this correctly. the "keyboard focus follows VO focus" just needs to be turned off, then the user can move up and down the list with VO and select what they want with VO+spacebar. You may need to wait until the next release of VoiceOver to verify, but this should work in Leopard as well

[Benjamin Hawkes-Lewis]

Hang on a second, that sounds good for VO users, but what if you're not using VoiceOver and are just trying to use keyboard navigation (for example, if you have a physical disability that makes mouse use problematic: http://www.apple.com/accessibility/physical/ )?

[chris fleizach]

good point. the initial comment which referenced VoiceOver threw me off

[Joanmarie Diggs]

I can reproduce this issue using WebKitGtk from git master. I'd update the platform, but I don't seem to have that priv.

[Gustavo Noronha (kov)]

Reassigning to Forms instead of Accessibility, since there is no functionality to make accessible yet. It needs to work before that.

[Mario Sanchez Prada]

I might be wrong, but while working on bug 25679 I found out that items inside a selection listbox are not focusable, but just selectable so that makes impossible to navigate through the list of items with the keyboard without losing a previous selection unless you're using the Shift modifier, but that's only valid for adjacent items...

I don't have a strong opinion here (still have to look more and more code to be 100% sure about this), but I feel like allowing items to be focusable might be a requirement to fix this bug (and help with bug 25679 as well), because that way you could do something like this:

  1. Arrow to the first item to be selected, which will be 'selected' and 'focused' at that time.
  2. Press Ctrl+arrow to keep navigating through the list *without* selecting anything, and keeping the previous selection untouched. This way, the previously selected item would be in 'selected' state only, while the current item being navigated would be in 'focused' (in firefox this is indicated by a dotted rectangle around it).
  3. Press space to select another item, which will be 'focused' by that time and that would be added to the list of 'selected' items.
  4. Go to 2 or finish with the selection process

So, could somebody comment on this and confirm whether this makes any sense and/or that the items in a selection listbox are 'selectable' but not 'focusable' ?

Thanks!

[Mario Sanchez Prada]

Adding Chris Fleizach to CC

[chris fleizach]

(In reply to comment #10)
> I might be wrong, but while working on bug 25679 I found out that items inside a selection listbox are not focusable, but just selectable so that makes impossible to navigate through the list of items with the keyboard without losing a previous selection unless you're using the Shift modifier, but that's only valid for adjacent items...
> 

The listbox itself is focusable, the items inside the list are selectable. you don't want to press tab to move through every item in a list, which is why the list items are not focusable. This is standard across OSs.

> I don't have a strong opinion here (still have to look more and more code to be 100% sure about this), but I feel like allowing items to be focusable might be a requirement to fix this bug (and help with bug 25679 as well), because that way you could do something like this:

MacOS does not have a standard way to do non-contiguous selection of items in a list using a keyboard only (I'm sure it's a long-standing request however). For this to be made to work on the Mac, that would have to be resolved. If other platforms have a standard way to do this kind of selection, then it should be straightforward to implement that behavior

[Carlos Garcia Campos]

Created attachment 73223 [details]
Patch that implements multi-selection of non-adjacents items with keyboard

This patch does the same than firefox, when Control key is pressed you can move the focus over the list items and select/deselect with Space key. List items aren't focusable, focus is handled by the ListBox so that pressing TAB doesn't make the focus iterate over list items.

[Chang Shu]

Great patch! This is exactly what I am looking for as I am trying to make spatial navigation work on multiple select box. I just have one suggestion. Is it ok we remove the current behavior of up/down key and make your ctrl+up/down as the default up/down behavior? The reasons are as follows:
1. Use up/down key for single-select type of behavior of multiple-select does not make a lot sense;
2. Using less keys makes the code more portable. For example, on mobile phones, usually no ctrl key is available.
3. This is easy for a user to use the select box without reading some instructions.
Thanks!

[Joanmarie Diggs]

(In reply to comment #14)
> Great patch! 

Yay! Thanks for doing this implementation Carlos. :-)

> I just have one suggestion. Is it ok we remove the current behavior of up/down key and make your ctrl+up/down as the default up/down behavior?

I would really (really) encourage you not to do this.

> 1. Use up/down key for single-select type of behavior of multiple-select does not make a lot sense;

Perhaps not. HOWEVER, this is what users who use the keyboard instead of the mouse are accustomed to having: In Windows, in GNOME, and (I believe) in OS X. And you don't always know something is multiselect until you try to select multiple items. So maybe it doesn't make sense from a design perspective, but if it suddenly goes away, I fear we shall confuse a non-trivial number of users.

> 2. Using less keys makes the code more portable. For example, on mobile phones, usually no ctrl key is available.

Point very well taken. And I don't have any brilliant answers to this issue, other than could it be handled at the platform/port level?

> 3. This is easy for a user to use the select box without reading some instructions.

Exactly. See my response to number 1. ;-) ;-)

All joking aside, the removal of (unmodified) Up/Down in this case may mean less reading of instructions for one group, but it's going to mean more reading of instructions for another.

So... Ideas?

[Chang Shu]

From what I understand, your major concern is "regression". But I feel it's really frustrating to do both up/down and ctrl+up/down at the same time: you can easily erase all your selections by one accidental up or down. Anyway, for this kind of UI design issues, I don't know how WebKit finalizes this.
In terms of the patch, it is missing the layout tests so it can't land as it is.

> > 1. Use up/down key for single-select type of behavior of multiple-select does not make a lot sense;
> 
> Perhaps not. HOWEVER, this is what users who use the keyboard instead of the mouse are accustomed to having: In Windows, in GNOME, and (I believe) in OS X. And you don't always know something is multiselect until you try to select multiple items. So maybe it doesn't make sense from a design perspective, but if it suddenly goes away, I fear we shall confuse a non-trivial number of users.
> 
> > 2. Using less keys makes the code more portable. For example, on mobile phones, usually no ctrl key is available.
> 
> Point very well taken. And I don't have any brilliant answers to this issue, other than could it be handled at the platform/port level?
> 
> > 3. This is easy for a user to use the select box without reading some instructions.
> 
> Exactly. See my response to number 1. ;-) ;-)
> 
> All joking aside, the removal of (unmodified) Up/Down in this case may mean less reading of instructions for one group, but it's going to mean more reading of instructions for another.
> 
> So... Ideas?

[Carlos Garcia Campos]

(In reply to comment #16)
> From what I understand, your major concern is "regression". But I feel it's really frustrating to do both up/down and ctrl+up/down at the same time: you can easily erase all your selections by one accidental up or down. Anyway, for this kind of UI design issues, I don't know how WebKit finalizes this.
> In terms of the patch, it is missing the layout tests so it can't land as it is.
> 

Yes, I know, just wanted to make sure it made sense before spending more time. I'll write the tests and upload a new patch

[Carlos Garcia Campos]

Created attachment 73584 [details]
New patch including tests

[Chang Shu]

ping reviewer. :)

[Martin Robinson]

Comment on attachment 73584 [details]
New patch including tests

View in context: https://bugs.webkit.org/attachment.cgi?id=73584&action=review

> WebCore/dom/SelectElement.cpp:785
> +            bool isCtrlKey = data.multiple() && static_cast<KeyboardEvent*>(event)->metaKey();

This variable has a pretty confusing name, since it doesn't just track whether the control key is down or not.

[Mario Sanchez Prada]

(In reply to comment #20)
> (From update of attachment 73584 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=73584&action=review
> 
> > WebCore/dom/SelectElement.cpp:785
> > +            bool isCtrlKey = data.multiple() && static_cast<KeyboardEvent*>(event)->metaKey();
> 
> This variable has a pretty confusing name, since it doesn't just track whether the control key is down or not.

My humble "casual review" :-):

What about naming it something like 'keepCurrentSelection' ? After reading the patch I think its purpose si to know precisely when we don't want to change the current selection despite of moving down or up through the list, so I guess that would be a good name.

Other than that, I think this patch lacks some comments in the ChangeLogs in order to qualify for the r+

[Carlos Garcia Campos]

Created attachment 76522 [details]
Updated patch rebased to current git master

Renamed isCtrlKey as keepSelection and more verbose changelogs.

[Mario Sanchez Prada]

Ping reviewers? ;-)

[Adam Barth]

Comment on attachment 76522 [details]
Updated patch rebased to current git master

cfleizach says we shouldn't accept this patch until there is a standardized way of doing this on the Mac.  Also, the platform ifdefs in SelectElement probably aren't the right approach for introducing platform-specific behavior.

[Mario Sanchez Prada]

(In reply to comment #24)
> (From update of attachment 76522 [details])
> cfleizach says we shouldn't accept this patch until there is a standardized way > of doing this on the Mac.  Also, the platform ifdefs in SelectElement probably 
> aren't the right approach for introducing platform-specific behavior.

Problem is that, as far as I know, Mac doesn't need this patch since they can get the same functionality through the VO cursor, which is something that we don't have in GTK (so that's why we need the ability to select multiple, non adjacent, options by using keyboard navigation only).

Opinions, thoughts?

[chris fleizach]

yea i didn't mean that we shouldn't accept the patch ever, since other platforms support this mode. it should be integrated in a way that is cognizant of that fact

[Dominic Mazzoni]

It'd be nice to have this in Chromium on Windows. What would be the correct way to modify this patch to support these keyboard shortcut changes on only some platforms?

[Pascal Jacquemart]

Created attachment 216818 [details]
Updated Carlos patch to the trunk

[Pascal Jacquemart]

Short update: Multiple selection can already be achieved with keyboard only by enabling the spatial navigation feature.
The latest patch follows this path while pressing CTRL modifier + Arrow (CMD modifier on Mac)

[Pascal Jacquemart]

Anyone for review? Be kind this is my very first patch for WebCore

[Darin Adler]

I think it’s clear that we do not want this behavior on Mac, so this patch can’t go in as is. We need to figure out what behavior we want on the various platforms before we can figure out exactly how to code this. I believe this is wanted on Windows or any platform where the UI is a copy of how Windows does things (most Unix platforms, I think).

[Pascal Jacquemart]

Thanks for feedback... 
  
This is just a general accessibility feature we would like on Gtk (active without spatial navigation)
I know that support for full keyboard navigation is a general requirement for OS X applications so I just had a look on OS X Accessibilty Keyboard Shortcuts -> https://developer.apple.com/library/mac/documentation/Accessibility/Conceptual/AccessibilityMacOSX/OSXAXKeyboardShortcuts/OSXAXKeyboardShortcuts.html

It seems that these shortcuts are defined for "full keyboard access" mode
- Control-arrow key: Moves focus to another value or cell within a control such as a table
- Space bar: Selects the highlighted control (equivalent to clicking the mouse button)

In the meantime Command-arrow up / down is a very useful shortcut within the finder so it could be confusing for user if the same is used in Safari
Let me know if you can commit on a shortcut for Safari or enable this for other platforms or give up?

[chris fleizach]

Comment on attachment 216818 [details]
Updated Carlos patch to the trunk

I think the name meta_selection could be better. maybe something like m_allowsNonContiguousSelection or something.

Does GTK already have a system wide paradigm for doing this. the Mac does not, so we're not going to add something for the Mac, but if GTK already has a platform idiom then it seems to be sense to support it for just that platform

[Mario Sanchez Prada]

(In reply to comment #33)
> (From update of attachment 216818 [details])
> I think the name meta_selection could be better. maybe something like 
> m_allowsNonContiguousSelection or something.

I agree with this too.

> Does GTK already have a system wide paradigm for doing this. the Mac does not,
> so we're not going to add something for the Mac, but if GTK already has a
> platform idiom then it seems to be sense to support it for just that platform

I'm not sure if I'm properly answering your question Chris, but in GTK this is is the expected behaviour when navigating with the keyboard through elements inside widgets supporting multiple selection.

So I would say that yes, it's an expected idiom in the GTK platform.

[Pascal Jacquemart]

Yes, I can tell Mario made the same comment offline ;-)

Sorry but I am afraid m_allowsNonContiguousSelection is worst
What about m_isMultipleSelectionWithKeyboard ?

Second point is to enable for all platforms but MAC. Are you ok with something like...

#if !PLATFORM(MAC)
            m_isMultipleSelectionWithKeyboard = m_multiple && static_cast<KeyboardEvent*>(event)->ctrlKey();
#endif

Being aware that m_isMultipleSelectionWithKeyboard would then defaults to false on MAC

[Pascal Jacquemart]

Created attachment 219416 [details]
Rebase to master 2013-12-16

Updated after latest comments from Mario and Chris
The code is simpler and ctrl+arrow shortcut is disabled for Mac

[chris fleizach]

Comment on attachment 219416 [details]
Rebase to master 2013-12-16

View in context: https://bugs.webkit.org/attachment.cgi?id=219416&action=review

> LayoutTests/fast/forms/listbox-selection-expected.txt:35
> +PASS selectionPattern("sl15") is "01010"

won't this fail on Mac?

> Source/WebCore/html/HTMLSelectElement.cpp:1429
> +            m_allowsNonContiguousSelection = m_multiple && (static_cast<KeyboardEvent*>(event)->ctrlKey() || isSpatialNavigationEnabled(document().frame()));

is this supported on Windows with the same key combo (sorry for my obtuseness)

[Build Bot]

Comment on attachment 219416 [details]
Rebase to master 2013-12-16

Attachment 219416 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/48798060

New failing tests:
fast/forms/listbox-selection.html

[Build Bot]

Created attachment 219561 [details]
Archive of layout-test-results from webkit-ews-09 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-09  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.5

[Build Bot]

Comment on attachment 219416 [details]
Rebase to master 2013-12-16

Attachment 219416 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/46878147

New failing tests:
fast/spatial-navigation/snav-multiple-select-optgroup.html
fast/forms/listbox-selection.html
fast/spatial-navigation/snav-multiple-select.html

[Build Bot]

Created attachment 219598 [details]
Archive of layout-test-results from webkit-ews-01 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-01  Port: mac-mountainlion  Platform: Mac OS X 10.8.5

[Build Bot]

Comment on attachment 219416 [details]
Rebase to master 2013-12-16

Attachment 219416 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/50098438

New failing tests:
fast/spatial-navigation/snav-multiple-select-optgroup.html
fast/forms/listbox-selection.html
fast/spatial-navigation/snav-multiple-select.html

[Build Bot]

Created attachment 219607 [details]
Archive of layout-test-results from webkit-ews-03 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-03  Port: mac-mountainlion  Platform: Mac OS X 10.8.5

[Pascal Jacquemart]

Created attachment 219655 [details]
Listbox multiple selection - Skipped on Mac

Slightly modified not to interfere with spatial navigation tests
Created a separate test in order to keep original listbox tests unchanged
- Actual listbox selection tests will still be running on Mac
- The new test for multiple non contiguous selection with keyboard is skipped on Mac

[Pascal Jacquemart]

Thanks Chris,

Indeed it was a mess on Mac, sorry about that

I am fine in enabling it for Windows because this CTRL + Arrow shortcut is already available in explorer to achieve multiple selection.

But I am afraid I am not able to compile on Windows. What happened to the win build bot?

[chris fleizach]

Comment on attachment 219655 [details]
Listbox multiple selection - Skipped on Mac

View in context: https://bugs.webkit.org/attachment.cgi?id=219655&action=review

> LayoutTests/ChangeLog:9
> +        * fast/forms/listbox-selection-3.html: Added.

is it possible to rename this test
listbox-non-contiguous-keyboard-selection.html

so that it's self describing? otherwise, i think this is ready to go

[Pascal Jacquemart]

Created attachment 220223 [details]
Listbox multiple non contiguous selection

Simply renamed the new layout test

[Pascal Jacquemart]

Comment on attachment 220223 [details]
Listbox multiple non contiguous selection

Then with the flags should be better

[chris fleizach]

Comment on attachment 220223 [details]
Listbox multiple non contiguous selection

View in context: https://bugs.webkit.org/attachment.cgi?id=220223&action=review

> Source/WebCore/html/HTMLSelectElement.cpp:1464
> +#endif

I feel like this block of code should be

#if !PLATFORM(MAC)
    m_allowsNonContiguousSelection = m_multiple && (isSpatialNavigationEnabled || static_cast<KeyboardEvent*>(event)->ctrlKey())
#endif

[Pascal Jacquemart]

> I feel like this block of code should be
> 
> #if !PLATFORM(MAC)
>     m_allowsNonContiguousSelection = m_multiple && (isSpatialNavigationEnabled || static_cast<KeyboardEvent*>(event)->ctrlKey())
> #endif

Fine, indeed this more a C like coding style and I recognize mixing bitwise and logical operator might lead to unexpected issues

Uploading a new patch...

[Pascal Jacquemart]

Created attachment 220732 [details]
 Listbox multiple non contiguous selection 2

[WebKit Commit Bot]

Comment on attachment 220732 [details]
 Listbox multiple non contiguous selection 2

Clearing flags on attachment: 220732

Committed r161558: <http://trac.webkit.org/changeset/161558>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.