Closed Bug 152370 Opened 23 years ago Closed 23 years ago

menulist and listbox not reporting correct accessible value

Categories

(Core :: Disability Access APIs, defect, P1)

x86
Windows 2000
defect

Tracking

()

VERIFIED FIXED
mozilla1.1beta

People

(Reporter: dsirnapalli, Assigned: aaronlev)

Details

(Keywords: access)

Attachments

(2 files, 1 obsolete file)

-- Testcase attached. Summary describes the bug.
Dharma, the value of a combobox or list should be the name of the item that is currently selected. It should not be reporting an error at all. If it does report an error, I think NS_ERROR_FAILURE would be the correct failure code, because this is in fact implemented. More about the problem: When I look at the menulists in font appearance prefs, I see that the Serif, Sans-serif and monospace font menulist's report their value correctly. It turns out that the menu items in those menulists are not supporting nsIDOMXULSelectControlItemElement, as they should be. It appears they're not picking up the correct bindings.
Status: NEW → ASSIGNED
Keywords: access, sec508
Priority: -- → P1
Summary: nsIAccessible accValue returns NS_ERROR_FAILURE instead of NS_ERROR_NOT_IMPLEMENTED for XUL Menulist Node → menulist and listbox not reporting correct accessible value
Target Milestone: --- → mozilla1.1beta
Aaron, seems this fix is only for menulist. I have a different idea about fixing this bug. Since you fixed bug 147756, now we can do nsCOMPtr<nsIDOMXULSelectControlElement> select(do_QueryInterface (mDOMNode)) for menulist/listbox successfully. I think current implemetations of getAccValue are logical and consistent with HTML impl. There is one thing should be considered, we should use selectedItem->GetLabel or selectedItem->GetValue as the return value. (currently, listitem supports GetLabel, but menuitem doesn't)?
this is my fix :) Index: nsXULSelectAccessible.cpp =================================================================== RCS file: /cvsroot/mozilla/accessible/src/xul/nsXULSelectAccessible.cpp,v retrieving revision 1.6 diff -u -r1.6 nsXULSelectAccessible.cpp --- nsXULSelectAccessible.cpp 12 Jun 2002 05:16:30 -0000 1.6 +++ nsXULSelectAccessible.cpp 18 Jun 2002 04:47:25 -0000 @@ -534,7 +534,7 @@ if (select) { nsCOMPtr<nsIDOMXULSelectControlItemElement> selectedItem; select->GetSelectedItem(getter_AddRefs(selectedItem)); - return selectedItem->GetValue(_retval); + return selectedItem->GetLabel(_retval); } return NS_ERROR_FAILURE; } Index: menu.xml =================================================================== RCS file: /cvsroot/mozilla/xpfe/global/resources/content/bindings/menu.xml,v retrieving revision 1.14 diff -u -r1.14 menu.xml --- menu.xml 13 Apr 2002 14:25:31 -0000 1.14 +++ menu.xml 18 Jun 2002 04:49:30 -0000 @@ -51,6 +51,8 @@ <implementation> <property name="value" onset="this.setAttribute('value',val); return val;" onget="return this.getAttribute('value');"/> + <property name="label" onget="return this.getAttribute('label');" + onset="this.setAttribute('label', val); return val;"/> </implementation> </binding> If we decide to use GetValue, then there is nothing to be changed.
Kyle, I didn't use your change to nsComboBoxAccessible::GetAccValue() because it doesn't work for the case where the entire list isn't loaded yet. For example, run the MSAA Inspect tool and open Prefs -> Appearance -> Fonts. If you inspect the combo box, your patch doesn't produce the correct value. This is because the entire list isn't actually loaded yet, but the textfield shows the currently selected value. However, adding label support to menu item was definitely useful. Thank you. Seeking r=
Attachment #88066 - Attachment is obsolete: true
Comment on attachment 88175 [details] [diff] [review] Fixes GetAccValue() for <listbox> as well. Uses label of selected element for value. r=kyle
Attachment #88175 - Flags: review+
Comment on attachment 88175 [details] [diff] [review] Fixes GetAccValue() for <listbox> as well. Uses label of selected element for value. sr=jst
Attachment #88175 - Flags: superreview+
fixed
Status: ASSIGNED → RESOLVED
Closed: 23 years ago
Resolution: --- → FIXED
-- Verified in todays trunk build. Works fine. Marking bug as verified.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: