Bug 72479 – Accessibility: Multiselect list boxes need to report focus in addition to selection

• Component: Accessibility
• Status: RESOLVED FIXED

[Dominic Mazzoni]

When a user navigates through a multiselect list box using the arrow keys, the accessible representation of the list box and its options must include not only which options are currently selected, but also which one has focus.

[Dominic Mazzoni]

Created attachment 115345 [details]
Patch

[chris fleizach]

Comment on attachment 115345 [details]
Patch

what platform was this tested on? on the mac, the list box should retain focus() and the selection is returned through selectedChildren(). the concept of selection vs. focus() is different

[Dominic Mazzoni]

(In reply to comment #2)
> (From update of attachment 115345 [details])
> what platform was this tested on? on the mac, the list box should retain focus() and the selection is returned through selectedChildren(). the concept of selection vs. focus() is different

This doesn't send any focus events, so I think Safari will be okay - but I'll test some more to make sure.

If you prefer, I could expose this as something other than focus. It's kind of like an active descendant.

Note that on Windows, the focus really does move to the option within the list. So ideally let's create an abstraction that's cross-platform.

- Dominic

[chris fleizach]

(In reply to comment #3)
> (In reply to comment #2)
> > (From update of attachment 115345 [details] [details])
> > what platform was this tested on? on the mac, the list box should retain focus() and the selection is returned through selectedChildren(). the concept of selection vs. focus() is different
> 
> This doesn't send any focus events, so I think Safari will be okay - but I'll test some more to make sure.
> 
> If you prefer, I could expose this as something other than focus. It's kind of like an active descendant.
> 
> Note that on Windows, the focus really does move to the option within the list. So ideally let's create an abstraction that's cross-platform.
> 
> - Dominic

It seems like this might be a platform decision for windows, i.e.) when asked if an object is isFocused() and the object is a listBoxOptionRole and isSelected() == true, then it returns true.

[chris fleizach]

(In reply to comment #4)
> (In reply to comment #3)
> > (In reply to comment #2)
> > > (From update of attachment 115345 [details] [details] [details])
> > > what platform was this tested on? on the mac, the list box should retain focus() and the selection is returned through selectedChildren(). the concept of selection vs. focus() is different
> > 
> > This doesn't send any focus events, so I think Safari will be okay - but I'll test some more to make sure.
> > 
> > If you prefer, I could expose this as something other than focus. It's kind of like an active descendant.
> > 
> > Note that on Windows, the focus really does move to the option within the list. So ideally let's create an abstraction that's cross-platform.
> > 
> > - Dominic
> 
> It seems like this might be a platform decision for windows, i.e.) when asked if an object is isFocused() and the object is a listBoxOptionRole and isSelected() == true, then it returns true.

Does Chrome  move KB focus to each item in the <select> list? The last time I looked at list boxes the options were not actual nodes that could receive focus.. they were literally just strings that were drawn at the right place

[Dominic Mazzoni]

> > It seems like this might be a platform decision for windows, i.e.) when asked if an object is isFocused() and the object is a listBoxOptionRole and isSelected() == true, then it returns true.
> 
> Does Chrome  move KB focus to each item in the <select> list? The last time I looked at list boxes the options were not actual nodes that could receive focus.. they were literally just strings that were drawn at the right place

No, keyboard focus stays with the <select> list itself, however there is a concept of the focused element within the list that's not the same as the selection when multiple things are selected. On Windows, the accessible object for the list option actually gets focus, even though it doesn't correspond to a browser element. 

In a list of 5 items, if you start on item 2 and press shift+down, now 2 and 3 are selected, but 3 is focused.  But if you start at 3 and press shift+up, 2 and 3 are selected, but 2 is focused. That's what this changelist is attempting to do - identify which of all of the selected items is the active one.

Alternatives:

1. I could add a new interface isActive that would indicate whether an element is the active descendant of its parent widget.

2. Or, I could have AccessibilityListBox return the active element in activeDescendant(), which is currently only used for ARIA but could be used here as well.

[chris fleizach]

(In reply to comment #6)
> > > It seems like this might be a platform decision for windows, i.e.) when asked if an object is isFocused() and the object is a listBoxOptionRole and isSelected() == true, then it returns true.
> > 
> > Does Chrome  move KB focus to each item in the <select> list? The last time I looked at list boxes the options were not actual nodes that could receive focus.. they were literally just strings that were drawn at the right place
> 
> No, keyboard focus stays with the <select> list itself, however there is a concept of the focused element within the list that's not the same as the selection when multiple things are selected. On Windows, the accessible object for the list option actually gets focus, even though it doesn't correspond to a browser element. 
> 
> In a list of 5 items, if you start on item 2 and press shift+down, now 2 and 3 are selected, but 3 is focused.  But if you start at 3 and press shift+up, 2 and 3 are selected, but 2 is focused. That's what this changelist is attempting to do - identify which of all of the selected items is the active one.
> 
> Alternatives:
> 
> 1. I could add a new interface isActive that would indicate whether an element is the active descendant of its parent widget.
> 
> 2. Or, I could have AccessibilityListBox return the active element in activeDescendant(), which is currently only used for ARIA but could be used here as well.

It sounds like this is what isSelected() already does. (as well as selectedChildren())

[Dominic Mazzoni]

(In reply to comment #7)
> It sounds like this is what isSelected() already does. (as well as selectedChildren())

Not quite - if multiple items are selected, isSelected / selectedChildren is enough to tell you that several items are selected, but it doesn't tell you what would happen if you were to press shift+up arrow or shift+down arrow to try to extend or contract the selection. You need to know which of those items is the active one.

It's like text selection - it's not enough to know that the selection is from character 5 to character 10, you have to know whether the selection focus is at the beginning or end.

[chris fleizach]

(In reply to comment #8)
> (In reply to comment #7)
> > It sounds like this is what isSelected() already does. (as well as selectedChildren())
> 
> Not quite - if multiple items are selected, isSelected / selectedChildren is enough to tell you that several items are selected, but it doesn't tell you what would happen if you were to press shift+up arrow or shift+down arrow to try to extend or contract the selection. You need to know which of those items is the active one.
> 
> It's like text selection - it's not enough to know that the selection is from character 5 to character 10, you have to know whether the selection focus is at the beginning or end.

k, i see. 

i think we should have a new attribute (perhaps isActiveSelection()) for this. it's definitely a different concept than focus and selection

thanks

[Dominic Mazzoni]

No problem - I'll switch it to a new attribute, how about isActiveSelectedOption to be a little more clear?

[chris fleizach]

(In reply to comment #10)
> No problem - I'll switch it to a new attribute, how about isActiveSelectedOption to be a little more clear?

Does this concept apply to any other widget? if so, we could put it on AccessibilityObject. if not, we could just put it on the list box option

[Dominic Mazzoni]

Created attachment 115529 [details]
Patch

[WebKit Review Bot]

Please wait for approval from [e-mail] before submitting because this patch contains changes to the Chromium public API.

[chris fleizach]

Comment on attachment 115529 [details]
Patch

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

> Source/WebCore/accessibility/AccessibilityListBoxOption.cpp:93
> +}

extra parens not needed

> Source/WebKit/chromium/src/WebAccessibilityObject.cpp:233
> +

for methods that return a bool, you should use true/false

> LayoutTests/accessibility/multiselect-list-reports-active-option.html:61
> +                layoutTestController.notifyDone();

you need to remove the notificationListener from the object, otherwise notifications can bleed into other tests

[Dominic Mazzoni]

Comment on attachment 115529 [details]
Patch

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

>> Source/WebCore/accessibility/AccessibilityListBoxOption.cpp:93
>> +}
> 
> extra parens not needed

Done.

>> Source/WebKit/chromium/src/WebAccessibilityObject.cpp:233
>> +
> 
> for methods that return a bool, you should use true/false

Done.

>> LayoutTests/accessibility/multiselect-list-reports-active-option.html:61
>> +                layoutTestController.notifyDone();
> 
> you need to remove the notificationListener from the object, otherwise notifications can bleed into other tests

Thanks, didn't realize that.

[Dominic Mazzoni]

Created attachment 115531 [details]
Patch

[chris fleizach]

Comment on attachment 115531 [details]
Patch

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

> LayoutTests/accessibility/multiselect-list-reports-active-option.html:23
> +            }

this will mean you'll get one notification and then it will not get any more. i think you want to remove this right before you notifyDone() at the end

[Dominic Mazzoni]

Created attachment 115533 [details]
Patch

[Dominic Mazzoni]

Comment on attachment 115531 [details]
Patch

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

>> LayoutTests/accessibility/multiselect-list-reports-active-option.html:23
>> +            }
> 
> this will mean you'll get one notification and then it will not get any more. i think you want to remove this right before you notifyDone() at the end

Hmmm, you're right. It looks like the test was still passing for me either way because removeNotificationListener doesn't do anything in the Chromium DumpRenderTree implementation. I can't reproduce the problem where it bleeds into another test, maybe it only affects some platforms?

[Dominic Mazzoni]

I updated the patch earlier to put removeNotificationListener in the right place - take another look?

[chris fleizach]

Comment on attachment 115533 [details]
Patch

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

looks ok. one minor comment to fix before checking in

> Source/WebCore/accessibility/AccessibilityObject.h:414
>  

I think this should be renamed to

isSelectedOptionActive()

and you should put a comment above to explain what it means

[Dominic Mazzoni]

Sounds good, will do, thanks.

[Dominic Mazzoni]

Created attachment 116052 [details]
Patch for landing

[WebKit Review Bot]

Comment on attachment 116052 [details]
Patch for landing

Clearing flags on attachment: 116052

Committed r100892: <http://trac.webkit.org/changeset/100892>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Ryosuke Niwa]

The test added by this patch is failing on Mac:
http://build.webkit.org/results/SnowLeopard%20Intel%20Release%20(Tests)/r101180%20(34953)/accessibility/multiselect-list-reports-active-option-actual.txt

To begin with, why did this patch add a test expectation to platform/chromium? That makes very little sense given this is a script test. Are you expecting this test to fail on non-Chromium platforms? If that were the case, then the correct thing to do is to add it to the skipped list and file appropriate bugs for non-Chromium ports. If not, then you should have added the test expectation to LayoutTests/accessibility/