Closed Bug 685846 Opened 8 years ago Closed 8 years ago

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

Categories

(Core :: XUL, defect)

All
Windows 7
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla9

People

(Reporter: surkov, Assigned: surkov)

References

(Blocks 1 open bug)

Details

(Keywords: access)

Attachments

(1 file)

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.
Component: Disability Access APIs → XP Toolkit/Widgets: XUL
QA Contact: accessibility-apis → xptoolkit.xul
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?
(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.
(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.
Attached patch patchSplinter Review
Assignee: nobody → surkov.alexander
Status: NEW → ASSIGNED
Attachment #559511 - Flags: review?(enndeakin)
mark as blocking of bug 673958 (a11y focus rework)
Blocks: 673958
Comment on attachment 559511 [details] [diff] [review]
patch

OK, let's try it.
Attachment #559511 - Flags: review?(enndeakin) → review+
https://hg.mozilla.org/mozilla-central/rev/4a4b97057078
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla9
Moving to Core:XUL per https://bugzilla.mozilla.org/show_bug.cgi?id=1455336
Component: XP Toolkit/Widgets: XUL → XUL
You need to log in before you can comment on or make changes to this bug.