Closed Bug 725432 Opened 8 years ago Closed 8 years ago

HTML buttons with aria-pressed not exposing IA2 TOGGLE_BUTTON role

Categories

(Core :: Disability Access APIs, defect)

x86
macOS
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla14

People

(Reporter: davidb, Assigned: maxli)

References

(Blocks 1 open bug)

Details

(Whiteboard: [good first bug][mentor=surkov.alexander@gmail.com][lang=c++])

Attachments

(1 file, 1 obsolete file)

Haven't confirmed yet but this came from Aaron.
Aaron comment:
The issue is this code which assumes the button role comes from ARIA:

1851 role
1852 nsAccessible::ARIARoleInternal()
turn nsAccessible::ARIARoleInternal() into ARIATranformRole(role aRole) which transforms the given role corresponding to applied ARIA attributes (http://mxr.mozilla.org/mozilla-central/source/accessible/src/base/nsAccessible.h#172).

Trevor, are you ok with ARIATransformRole? Anything nicer?
Whiteboard: [good first bug][mentor=surkov.alexander@gmail.com][lang=c++]
(In reply to alexander :surkov from comment #2)
> turn nsAccessible::ARIARoleInternal() into ARIATranformRole(role aRole)
> which transforms the given role corresponding to applied ARIA attributes
> (http://mxr.mozilla.org/mozilla-central/source/accessible/src/base/
> nsAccessible.h#172).

then we'd pass the result of NativeRole() into it when doing Role() what about ARIARole()?

> Trevor, are you ok with ARIATransformRole? Anything nicer?

seems fine
(In reply to Trevor Saunders (:tbsaunde) from comment #3)
> (In reply to alexander :surkov from comment #2)
> > turn nsAccessible::ARIARoleInternal() into ARIATranformRole(role aRole)
> > which transforms the given role corresponding to applied ARIA attributes
> > (http://mxr.mozilla.org/mozilla-central/source/accessible/src/base/
> > nsAccessible.h#172).
> 
> then we'd pass the result of NativeRole() into it when doing Role() what
> about ARIARole()?

ARIATransformRole(mRoleMapEntry->role)
assigning to Max per email
Assignee: nobody → maxli
Attached patch Patch v1 (obsolete) — Splinter Review
Attachment #610510 - Flags: review?(surkov.alexander)
Comment on attachment 610510 [details] [diff] [review]
Patch v1

Review of attachment 610510 [details] [diff] [review]:
-----------------------------------------------------------------

please add a test to http://mxr.mozilla.org/mozilla-central/source/accessible/tests/mochitest/test_aria_roles.html?force=1#70

for <button aria-pressed="true">

under new "roles transformed by ARIA state attributes" section
Attachment #610510 - Flags: review?(trev.saunders)
Attachment #610510 - Flags: review?(surkov.alexander)
Attachment #610510 - Flags: feedback+
Attached patch Patch v2 w/ testSplinter Review
Attachment #610510 - Attachment is obsolete: true
Attachment #610510 - Flags: review?(trev.saunders)
Attachment #610764 - Flags: review?(trev.saunders)
Comment on attachment 610764 [details] [diff] [review]
Patch v2 w/ test

># HG changeset patch
># User Max Li <maxli@maxli.ca>
># Date 1333065846 14400
># Node ID 32fe30fba0f1a760bdd7a0c6e8df561c057e4f90
># Parent  92fe907ddac8855e733102813db5e860fd7a557a
>Bug 725432 - HTML buttons with aria-pressed not exposing IA2 TOGGLE_BUTTON role
>
>diff --git a/accessible/src/base/nsAccessible.cpp b/accessible/src/base/nsAccessible.cpp
>--- a/accessible/src/base/nsAccessible.cpp
>+++ b/accessible/src/base/nsAccessible.cpp
>@@ -1804,14 +1804,11 @@

in future please use diff arguments -U8 -p

>       //////////////////////////////////////////////////////////////////////////
>+      // roles transformed by ARIA state attributes
>+      testRole("togglebutton", ROLE_TOGGLE_BUTTON);

please add a link to the bug in <a> like the other bug links after the js.  (I bet surkov is willing to take care of that for you :)

looks good otherwise, thanks!
Attachment #610764 - Flags: review?(trev.saunders) → review+
Thanks for the patch - hopefully see you on IRC soon! :-)

https://hg.mozilla.org/mozilla-central/rev/38d50ee4b9ba
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla14
You need to log in before you can comment on or make changes to this bug.