685846 - DOMMenuItemActive is not fired for richlistitem of richlistbox autocomplete popup if it's selected repeatedly

Status: RESOLVED FIXED

(Core :: XUL, defect)

Description

[alexander :surkov (:asurkov)]

DOMMenuItemActive is not fired for richlistitem of richlistbox autocomplete popup if it's selected repeatedly.

steps to reproduce:
1) focus autocomplete (having richlistbox autocomplete popup)
2) show popup (type a letter to start a search in case of address bar autocomplete widget or down arrow in case of toolkit autocomplete widget)
3) down arrow to select first item (DOMMenuItemActive is fired)
4) escape to close popup
5) down arrow to show popup
5) down arrow to select first item (DOMMenuItemActive is not fired)

This bug is important for accessibility focus support.

Note, this works for tree autocomplete popup.

Comment 1

[alexander :surkov (:asurkov)]

MDN listbox docs aren't clear (https://developer.mozilla.org/en/XUL/listbox) because it states that currentItem is just last selected item while currentIndex is "focused" item.

selectItem is unique method that changes current item, other selection methods don't. selectedIndex uses selectItem for positive values and clearSelection for negative values. So the problem with autocompletes that listbox current item isn't changed in the case of -1 value and no events are fired.

What is the correct behavior? Should selectedIndex = -1 change current item as well?

Comment 2

[Neil Deakin]

(In reply to alexander surkov from comment #1)
> MDN listbox docs aren't clear (https://developer.mozilla.org/en/XUL/listbox)
> because it states that currentItem is just last selected item while
> currentIndex is "focused" item.
> 

Both currentItem and currentIndex should refer to the same item in the list, if they are set. That item may or may not be part of the selection.

> What is the correct behavior? Should selectedIndex = -1 change current item
> as well?

Don't have a strong opinion here but if it is causing a problem, it seems changing it wouldn't be an issue.

Comment 3

[alexander :surkov (:asurkov)]

(In reply to Neil Deakin from comment #2)
> (In reply to alexander surkov from comment #1)
> > MDN listbox docs aren't clear (https://developer.mozilla.org/en/XUL/listbox)
> > because it states that currentItem is just last selected item while
> > currentIndex is "focused" item.
> > 
> 
> Both currentItem and currentIndex should refer to the same item in the list,
> if they are set. That item may or may not be part of the selection.

agree, obviously the doc should be fixed

> > What is the correct behavior? Should selectedIndex = -1 change current item
> > as well?
> 
> Don't have a strong opinion here but if it is causing a problem, it seems
> changing it wouldn't be an issue.

We always have an option to fix autocomplete binding where the problem appears. I just though it might be reasonable to have selectedIndex changing current item for negative indexes if it changes it for positive indexes.

Comment 4

[alexander :surkov (:asurkov)]

Attachment: patch

Comment 5

[alexander :surkov (:asurkov)]

mark as blocking of bug 673958 (a11y focus rework)

Comment 6

[Neil Deakin]

Comment on attachment 559511 [details] [diff] [review]
patch

OK, let's try it.

Comment 7

[alexander :surkov (:asurkov)]

landed on inbound http://hg.mozilla.org/integration/mozilla-inbound/rev/4a4b97057078

Comment 8

[(no longer active)]

https://hg.mozilla.org/mozilla-central/rev/4a4b97057078

Comment 9

[Firefox Bug Husbandry Bot]

Moving to Core:XUL per https://bugzilla.mozilla.org/show_bug.cgi?id=1455336