Closed Bug 109215 Opened 20 years ago Closed 14 years ago

Implement accessibility API support for <xul:slider>

Categories

(Core :: Disability Access APIs, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: aaronlev, Assigned: surkov)

References

(Blocks 1 open bug)

Details

(Keywords: access)

Attachments

(1 file, 1 obsolete file)

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!
Status: NEW → ASSIGNED
Priority: -- → P4
Target Milestone: --- → Future
Blocks: 82207
Target Milestone: Future → mozilla1.9beta
Product: Browser → Seamonkey
Aaron, did you allready found where <slider> is used?
(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.
Target Milestone: mozilla1.9beta → ---
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.
Assignee: aaronleventhal → pilgrim
Blocks: fox2access
Status: ASSIGNED → NEW
Keywords: access
Priority: P4 → --
*** Bug 336781 has been marked as a duplicate of this bug. ***
Blocks: keya11y
No longer blocks: fox2access
Assignee: pilgrim → aaronleventhal
Component: General → Disability Access APIs
Product: Mozilla Application Suite → Core
QA Contact: doronr → accessibility-apis
Summary: Active Accessibility: Implement <slider> support → Implement accessibility API support for <xul:slider>
Blocks: xula11y
No longer blocks: keya11y
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
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.
Attached patch patch (obsolete) — Splinter Review
we'll reuse nsXULSliderAccessible for scrollbars. It looks we should redefine role only.
Assignee: aaronleventhal → surkov.alexander
Status: NEW → ASSIGNED
Attachment #273585 - Flags: superreview?(neil)
Attachment #273585 - Flags: review?(aaronleventhal)
Comment on attachment 273585 [details] [diff] [review]
patch

I'm away for a few days. Ginn, can you review?
Attachment #273585 - Flags: review?(aaronleventhal) → review?(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
Attachment #273585 - Flags: review?(ginn.chen) → review+
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.
Attachment #273585 - Flags: superreview?(neil) → superreview+
Attached patch patch2Splinter Review
with Ginn's and Neil's comments addressed.

for NS_OK_NO_ARIA_VALUE I used 0x21. I hope it's ok.
Attachment #273585 - Attachment is obsolete: true
checked in
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
(In reply to comment #13)
> checked in
> 

patch was backed up
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Attachment #273983 - Flags: approval1.9?
OS: Windows 2000 → All
Hardware: PC → All
Regarding approval for 1.9:  Does patch2 above have sufficient review+?  First one does, but what about the second?
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.
Yes, the second patch fixes problems of the first patch pointed by reviewers.
Attachment #273983 - Flags: approval1.9? → approval1.9+
checked in
Status: REOPENED → RESOLVED
Closed: 14 years ago14 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.