280871 - Fix MSAA support for XUL and HTML combo boxes

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect)

Description

[Aaron Leventhal]

We need to fix our MSAA support of combo boxes in the following ways:
   1. the value of the combo box should be the current option's name.
   2. a value change event should be fired for the combo box when the option
changes, whether the listbox is open or closed.
   3. Firefox's Tools -> Options -> Downloads ahould have a proper name, not "1
2 5 7" 
   4. Firefox's Tools -> Options -> Downloads intermittent listitem focus issue
needs to be fixed. This is where you tab to the combobox the first time, and you
don't get a listitem focus. If you then tab away and tab back, you do get a
listitem focus.

Comment 1

[Aaron Leventhal]

Scratch item 4. We should never fire a listitem focus when the combobox is closed.

Comment 2

[Aaron Leventhal]

Attachment: Five part fix

1) Never create combo box name by appending option labels,

Value change events
2) Use MSAA VALUE_CHANGE events for combo boxes that change,
3) fire DOM ValueChange events to trigger the MSAA events,

Focus events:
4) Fire MSAA focus events on list items only if not in collapsed container
5) Fire DOMMenuItemActive events on options, not on combo box itself

Comment 3

[Philip K. Warren]

Comment on attachment 173647 [details] [diff] [review]
Five part fix

>Index: layout/forms/nsListControlFrame.cpp
>+  nsIContent *optionContent = GetOptionContent(focusedIndex);
...
>-    presContext->EventStateManager()->DispatchNewEvent(mContent, event,
>+
>+    presContext->EventStateManager()->DispatchNewEvent(optionContent, event,
>                                                        &noDefault);

You need to NS_RELEASE optionContent after you are done with it. See the other
callers in this file.

r=pkw with that change.

Comment 4

[Aaron Leventhal]

(In reply to comment #3)
>You need to NS_RELEASE optionContent after you are done with it. See the other
Yeah, I keep thinking nsIContent was deCOMIfied like nsIFrame was. My mistake.

Comment 5

[Brian Ryner (not reading)]

Comment on attachment 173647 [details] [diff] [review]
Five part fix

>--- accessible/src/html/nsHTMLSelectAccessible.h	25 Jan 2005 19:35:53 -0000	1.24
>+++ accessible/src/html/nsHTMLSelectAccessible.h	7 Feb 2005 18:11:25 -0000
>@@ -80,10 +80,12 @@ public:
>   NS_DECL_NSIACCESSIBLESELECTABLE
> 
>   nsHTMLSelectableAccessible(nsIDOMNode* aDOMNode, nsIWeakReference* aShell);
>   virtual ~nsHTMLSelectableAccessible() {}
> 
>+  NS_IMETHODIMP GetName(nsAString &aName) { return GetHTMLName(aName, PR_FALSE); }
>+

I'm not sure if it matters or not, but for inline definitions we normally use
NS_IMETHOD instead of NS_IMETHODIMP.

>--- layout/forms/nsComboboxControlFrame.cpp	27 Jan 2005 22:52:52 -0000	1.303
>+++ layout/forms/nsComboboxControlFrame.cpp	7 Feb 2005 18:11:27 -0000
>@@ -2374,18 +2375,34 @@ nsComboboxControlFrame::OnOptionSelected
>       selectFrame->OnOptionSelected(aPresContext, aIndex, aSelected);
>     }
>   } else {
>     if (aSelected) {
>       RedisplayText(aIndex);
>+      FireValueChangeEvent();
>     } else {
>       RedisplaySelectedText();
>     }
>   }
> 
>   return NS_OK;
> }
> 
>+void nsComboboxControlFrame::FireValueChangeEvent()
>+{

Looks ok otherwise.

Comment 6

[Aaron Leventhal]

Checking in layout/forms/nsComboboxControlFrame.cpp;
/cvsroot/mozilla/layout/forms/nsComboboxControlFrame.cpp,v  <-- 
nsComboboxControlFrame.cpp
new revision: 1.305; previous revision: 1.304
done
Checking in layout/forms/nsComboboxControlFrame.h;
/cvsroot/mozilla/layout/forms/nsComboboxControlFrame.h,v  <-- 
nsComboboxControlFrame.h
new revision: 1.127; previous revision: 1.126
done
Checking in layout/forms/nsListControlFrame.cpp;
/cvsroot/mozilla/layout/forms/nsListControlFrame.cpp,v  <--  nsListControlFrame.cpp
new revision: 1.354; previous revision: 1.353
done
Checking in xpfe/global/resources/content/bindings/menulist.xml;
/cvsroot/mozilla/xpfe/global/resources/content/bindings/menulist.xml,v  <-- 
menulist.xml
new revision: 1.34; previous revision: 1.33
done
Checking in toolkit/content/widgets/menulist.xml;
/cvsroot/mozilla/toolkit/content/widgets/menulist.xml,v  <--  menulist.xml
new revision: 1.17; previous revision: 1.16
done
Checking in accessible/src/base/nsAccessible.cpp;
/cvsroot/mozilla/accessible/src/base/nsAccessible.cpp,v  <--  nsAccessible.cpp
new revision: 1.128; previous revision: 1.127
done
Checking in accessible/src/base/nsAccessible.h;
/cvsroot/mozilla/accessible/src/base/nsAccessible.h,v  <--  nsAccessible.h
new revision: 1.56; previous revision: 1.55
done
Checking in accessible/src/base/nsRootAccessible.cpp;
/cvsroot/mozilla/accessible/src/base/nsRootAccessible.cpp,v  <-- 
nsRootAccessible.cpp
new revision: 1.107; previous revision: 1.106
done
Checking in accessible/src/html/nsHTMLSelectAccessible.h;
/cvsroot/mozilla/accessible/src/html/nsHTMLSelectAccessible.h,v  <-- 
nsHTMLSelectAccessible.h
new revision: 1.25; previous revision: 1.24
done
Checking in accessible/src/html/nsHTMLTableAccessible.cpp;
/cvsroot/mozilla/accessible/src/html/nsHTMLTableAccessible.cpp,v  <-- 
nsHTMLTableAccessible.cpp
new revision: 1.25; previous revision: 1.24
done
Checking in accessible/src/xul/nsXULSelectAccessible.cpp;
/cvsroot/mozilla/accessible/src/xul/nsXULSelectAccessible.cpp,v  <-- 
nsXULSelectAccessible.cpp
new revision: 1.24; previous revision: 1.23
done

Comment 7

[Firefox Bug Husbandry Bot]

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