HTML buttons with aria-pressed not exposing IA2 TOGGLE_BUTTON role

RESOLVED FIXED in mozilla14

Status

()

Core
Disability Access APIs
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: davidb, Assigned: maxli)

Tracking

(Blocks: 1 bug)

unspecified
mozilla14
x86
Mac OS X
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

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

Attachments

(1 attachment, 1 obsolete attachment)

Haven't confirmed yet but this came from Aaron.

Comment 1

6 years ago
Aaron comment:
The issue is this code which assumes the button role comes from ARIA:

1851 role
1852 nsAccessible::ARIARoleInternal()

Comment 2

6 years ago
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

Comment 4

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

Comment 5

6 years ago
assigning to Max per email
Assignee: nobody → maxli
(Assignee)

Comment 6

6 years ago
Created attachment 610510 [details] [diff] [review]
Patch v1
Attachment #610510 - Flags: review?(surkov.alexander)

Comment 7

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

Comment 8

6 years ago
Created attachment 610764 [details] [diff] [review]
Patch v2 w/ test
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+

Comment 10

6 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/38d50ee4b9ba

Max, thank you for the fix! If you like then you can pick up another one:
https://bugzilla.mozilla.org/buglist.cgi?emailtype1=exact;resolution=---;email1=nobody%40mozilla.org;emailassigned_to1=1;component=Disability%20Access%20APIs;product=Core;status_whiteboard=mentor;list_id=2709998;status_whiteboard_type=allwordssubstr;query_format=advanced
Thanks for the patch - hopefully see you on IRC soon! :-)

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