Last Comment Bug 685846 - DOMMenuItemActive is not fired for richlistitem of richlistbox autocomplete popup if it's selected repeatedly
: DOMMenuItemActive is not fired for richlistitem of richlistbox autocomplete p...
Status: RESOLVED FIXED
: access
Product: Core
Classification: Components
Component: XP Toolkit/Widgets: XUL (show other bugs)
: unspecified
: All Windows 7
: -- normal (vote)
: mozilla9
Assigned To: alexander :surkov
:
Mentors:
Depends on:
Blocks: focuseventa11y 673958
  Show dependency treegraph
 
Reported: 2011-09-09 06:09 PDT by alexander :surkov
Modified: 2011-09-15 07:39 PDT (History)
4 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
patch (811 bytes, patch)
2011-09-09 10:43 PDT, alexander :surkov
enndeakin: review+
Details | Diff | Splinter Review

Description alexander :surkov 2011-09-09 06:09:17 PDT
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 2011-09-09 10:10:02 PDT
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 2011-09-09 10:20:21 PDT
(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 2011-09-09 10:39:46 PDT
(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 2011-09-09 10:43:17 PDT
Created attachment 559511 [details] [diff] [review]
patch
Comment 5 alexander :surkov 2011-09-12 03:28:05 PDT
mark as blocking of bug 673958 (a11y focus rework)
Comment 6 Neil Deakin 2011-09-14 16:47:13 PDT
Comment on attachment 559511 [details] [diff] [review]
patch

OK, let's try it.
Comment 7 alexander :surkov 2011-09-14 19:35:05 PDT
landed on inbound http://hg.mozilla.org/integration/mozilla-inbound/rev/4a4b97057078

Note You need to log in before you can comment on or make changes to this bug.