menulist and listbox not reporting correct accessible value

VERIFIED FIXED in mozilla1.1beta

Status

()

Core
Disability Access APIs
P1
normal
VERIFIED FIXED
16 years ago
16 years ago

People

(Reporter: dsirnapalli, Assigned: Aaron Leventhal)

Tracking

({access, sec508})

Trunk
mozilla1.1beta
x86
Windows 2000
access, sec508
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 1 obsolete attachment)

(Reporter)

Description

16 years ago
-- Testcase attached.
Summary describes the bug.
(Reporter)

Comment 1

16 years ago
Created attachment 87976 [details]
Test Case to reproduce the bug.
(Assignee)

Comment 2

16 years ago
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
(Assignee)

Comment 3

16 years ago
Created attachment 88066 [details] [diff] [review]
Sometimes the list of items for the dropdown wasn't fully loaded, but the text field in the combobox contained the appropriate value. This patch uses the value in the text  field now.

Seeking r= from Kyle or John Gaunt

Comment 4

16 years ago
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)?

Comment 5

16 years ago
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.
(Assignee)

Comment 6

16 years ago
Created attachment 88175 [details] [diff] [review]
Fixes GetAccValue() for <listbox> as well. Uses label of selected element for value.

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 7

16 years ago
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+
(Assignee)

Comment 9

16 years ago
fixed
Status: ASSIGNED → RESOLVED
Last Resolved: 16 years ago
Resolution: --- → FIXED
(Reporter)

Comment 10

16 years ago
-- 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.