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)
Tracking
()
VERIFIED
FIXED
mozilla1.1beta
People
(Reporter: dsirnapalli, Assigned: aaronlev)
Details
(Keywords: access)
Attachments
(2 files, 1 obsolete file)
2.78 KB,
application/vnd.mozilla.xul+xml
|
Details | |
2.45 KB,
patch
|
yuanyi21
:
review+
jst
:
superreview+
|
Details | Diff | Splinter Review |
-- Testcase attached.
Summary describes the bug.
Reporter | ||
Comment 1•23 years ago
|
||
Assignee | ||
Comment 2•23 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.
Assignee | ||
Comment 3•23 years ago
|
||
Seeking r= from Kyle or John Gaunt
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.
Assignee | ||
Comment 6•23 years ago
|
||
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 8•23 years ago
|
||
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•23 years ago
|
||
fixed
Status: ASSIGNED → RESOLVED
Closed: 23 years ago
Resolution: --- → FIXED
Reporter | ||
Comment 10•23 years ago
|
||
-- Verified in todays trunk build. Works fine. Marking bug as verified.
Status: RESOLVED → VERIFIED
Comment 11•6 years ago
|
||
Keywords: sec508
You need to log in
before you can comment on or make changes to this bug.
Description
•