281538 - No focus or selection events fired as user arrows through download manager

Status: RESOLVED FIXED

(Toolkit :: Downloads API, defect, P2)

Description

[Aaron Leventhal]

No focus or selection events fired as user arrows through download manager.

At the very least we need to fire DOMMenuItemActive events for each item's
element if we don't want to use real focus. See other examples in the Mozilla
code that do this.

The current lack of events breaks screen readers. Currently the say nothing as
the user arrows through the list.

Comment 1

[Doron Rosenberg (IBM)]

The latest patch in bug 285584 does send DOMMenuItemActive events.

Comment 2

[Aaron Leventhal]

The upcoming patch will fix richlistbox so that 
1) accessible objects are created for the list and the list items.
2) focus events are fired for the list items
3) accessible name is supported for each list item
4) home/end keys are supported

Comment 3

[Aaron Leventhal]

Attachment: Fire DOMMenuItemActive on the list item itself not the list, create accessibles for list and list items, support part of nsIDOMXULSelectControlItemElement

I suggest a spinnoff bug to implement the rest of
nsIDOMXULSelectControlItemElement nsIDOMXULSelectControlElement.

Are we ever going to implement seltype="multiple" for this? If we do, at that
point we'll need nsIDOMXULMultiSelectControlElement as well.

In general I think we should implement complete interfaces when we add a new
widget. It's important for accessibility at least. That's how we get labels and
states.

Comment 4

[Aaron Leventhal]

Attachment: Minor change -- use this.control instead of this.parentNode to get to listbox

Comment 5

[Mike Connor [:mconnor]]

Comment on attachment 186930 [details] [diff] [review]
Minor change -- use this.control instead of this.parentNode to get to listbox

>       <handler event="contextmenu" phase="capturing">
>       <![CDATA[
>          // This needed to be called before the contextmenu gets shown to handle
>          // someone rightclicking on an unselected item
>          if (event.target == this) {
>-           this.parentNode.selectedItem = this;
>+           var listbox = this.control;
>+           if (listbox) {
>+             listbox.selectedItem = this;
>+           }

The brackets aren't necessary here, please drop them, and the other places
where this is done.  And man, how'd I miss the indenting here?	We've got some
weird indentation all around the CDATA chunks throughout the patch (mostly
inconsistent).	Can you fix this up a bit while you're in here? (doesn't have
to be global, just bits you've touched)

Yes, multi-select needs to be added for this at some point.  Not a blocker, but
a useful thing to have for the binding.  Doron was supposed to file a bug on
this, can you add comments about what needs to be included for the
accessibility side when that gets implemented?

r=me with the cleanup noted

Comment 6

[Aaron Leventhal]

Comment on attachment 186930 [details] [diff] [review]
Minor change -- use this.control instead of this.parentNode to get to listbox

Will fix whitespace and extra { }

Comment 7

[Doron Rosenberg (IBM)]

298371 is the bug for multi-select

Comment 8

[Aaron Leventhal]

Forgot to mark this fixed.

Comment 9

[Firefox Bug Husbandry Bot]

https://bugzilla.mozilla.org/show_bug.cgi?id=1527381