109215 - Implement accessibility API support for <xul:slider>

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect)

Description

[Aaron Leventhal]

We need to implement <slider> using ROLE_SLIDER.
GetAccValue should return the the "value" attribute
Supports normal form control states and name labels

This is not a priority until I can find somewhere where the <slider> is used!

Comment 1

[Thomas]

Aaron, did you allready found where <slider> is used?

Comment 2

[Aaron Leventhal]

(In reply to comment #1)
> Aaron, did you allready found where <slider> is used?

It's not being used anywhere in the Seamonkey, Firefox or Thunderbird UI.

Comment 3

[Aaron Leventhal]

This needs to be a higher priority, because it's a basic widget and extensions will use it.

See patch from bug 290255 "Accessibility changes" for some code that might be a good starting point.

Comment 4

[Aaron Leventhal]

*** Bug 336781 has been marked as a duplicate of this bug. ***

Comment 5

[Aaron Leventhal]

What's the difference between a XUL slider and a XUL scale?

We don't have a11y for either.

Note: we should expose anything we know about the data type via object to attributes, to help alternative input AT's. See http://wiki.mozilla.org/Accessibility/Datatypes

Comment 6

[Aaron Leventhal]

Neil tells me "the slider is just an internal element used to implement both scale and crollbar. It shouldn't be used directly."

See also bug 285167 about making XUL scrollbars accessible.

Comment 7

[alexander :surkov (:asurkov)]

mac slider http://developer.apple.com/documentation/Accessibility/Conceptual/AccessibilityMacOSX/OSXAXRoleReference/chapter_7_section_32.html
mac thumb
http://developer.apple.com/documentation/Accessibility/Conceptual/AccessibilityMacOSX/OSXAXRoleReference/chapter_7_section_42.html#//apple_ref/doc/uid/TP40001078-CH211-DontLinkElementID_46

msaa slider/thumb/scrollbar
http://msdn2.microsoft.com/en-us/library/ms697649.aspx

Comment 8

[alexander :surkov (:asurkov)]

Attachment: patch

we'll reuse nsXULSliderAccessible for scrollbars. It looks we should redefine role only.

Comment 9

[Aaron Leventhal]

Comment on attachment 273585 [details] [diff] [review]
patch

I'm away for a few days. Ginn, can you review?

Comment 10

[Ginn Chen]

Comment on attachment 273585 [details] [diff] [review]
patch

1) nsXULSliderAccessible.cpp should include "nsIDOMElement.h"
2) nsXULSliderAccessible.h
Line 40
+#ifndef _nsXULSliderAccessible_H_
+#define _nsXULFormControlAccessible_H_
Should be #define _nsXULSliderAccessible_H_

3) In nsAccessible::SetCurrentValue
and nsAccessible::GetCurrentValue
We can save lines, if we use
if (!mRoleMapEntry || mRoleMapEntry->valueRule == eNoValue)
  return NS_OK_NOARIAVALUE;

4) I suggest use
NS_OK_NO_ARIA_VALUE instead of NS_OK_NOARIAVALUE

5) +#define NS_OK_NOARIAVALUE \
+NS_ERROR_GENERATE_SUCCESS(NS_ERROR_MODULE_GENERAL, 1)

We may want to use some value other than 1.
There're already several places defined this value.
See:http://lxr.mozilla.org/seamonkey/search?string=_SUCCESS%28NS_ERROR_MODULE_GENE

Comment 11

[[e-mail]]

Comment on attachment 273585 [details] [diff] [review]
patch

>+  nsresult rv = nsAccessibleWrap::GetMaximumValue(aValue);
>+  NS_ENSURE_SUCCESS(rv, rv);
>+
>+  // ARIA redefined maximum value.
>+  if (rv != NS_OK_NOARIAVALUE)
>+    return NS_OK;
>+
>+  return GetSliderAttr(nsAccessibilityAtoms::maxpos, aValue);
I'm tempted to say you should write
if (rv != NS_OK_NOARIAVALUE)
  return rv;

... which would make the NS_ENSURE_SUCCESS unnecessary.

Comment 12

[alexander :surkov (:asurkov)]

Attachment: patch2

with Ginn's and Neil's comments addressed.

for NS_OK_NO_ARIA_VALUE I used 0x21. I hope it's ok.

Comment 13

[alexander :surkov (:asurkov)]

checked in

Comment 14

[alexander :surkov (:asurkov)]

(In reply to comment #13)
> checked in
> 

patch was backed up

Comment 15

[Damon Sicore (:damons)]

Regarding approval for 1.9:  Does patch2 above have sufficient review+?  First one does, but what about the second?

Comment 16

[Aaron Leventhal]

Damon, usually if the reviewer thinks another review will be necessary, it's customer to minus the patch. If the review thinks the recommended changes are totally clear and don't need to be checked, they just give r+ with the recommendations.

Comment 17

[alexander :surkov (:asurkov)]

Yes, the second patch fixes problems of the first patch pointed by reviewers.

Comment 18

[alexander :surkov (:asurkov)]

checked in