Last Comment Bug 725432 - HTML buttons with aria-pressed not exposing IA2 TOGGLE_BUTTON role
: HTML buttons with aria-pressed not exposing IA2 TOGGLE_BUTTON role
Status: RESOLVED FIXED
[good first bug][mentor=surkov.alexan...
:
Product: Core
Classification: Components
Component: Disability Access APIs (show other bugs)
: unspecified
: x86 Mac OS X
: -- normal (vote)
: mozilla14
Assigned To: Max Li [:maxli]
:
: alexander :surkov
Mentors:
Depends on:
Blocks: aria
  Show dependency treegraph
 
Reported: 2012-02-08 11:55 PST by David Bolter [:davidb]
Modified: 2012-03-30 12:55 PDT (History)
2 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Patch v1 (2.74 KB, patch)
2012-03-29 04:47 PDT, Max Li [:maxli]
surkov.alexander: feedback+
Details | Diff | Splinter Review
Patch v2 w/ test (3.74 KB, patch)
2012-03-29 17:13 PDT, Max Li [:maxli]
tbsaunde+mozbugs: review+
Details | Diff | Splinter Review

Description David Bolter [:davidb] 2012-02-08 11:55:15 PST
Haven't confirmed yet but this came from Aaron.
Comment 1 alexander :surkov 2012-02-08 20:41:56 PST
Aaron comment:
The issue is this code which assumes the button role comes from ARIA:

1851 role
1852 nsAccessible::ARIARoleInternal()
Comment 2 alexander :surkov 2012-02-08 21:08:19 PST
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?
Comment 3 Trevor Saunders (:tbsaunde) 2012-02-10 01:29:03 PST
(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 alexander :surkov 2012-02-10 09:56:25 PST
(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 alexander :surkov 2012-03-25 20:15:32 PDT
assigning to Max per email
Comment 6 Max Li [:maxli] 2012-03-29 04:47:12 PDT
Created attachment 610510 [details] [diff] [review]
Patch v1
Comment 7 alexander :surkov 2012-03-29 05:56:17 PDT
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
Comment 8 Max Li [:maxli] 2012-03-29 17:13:04 PDT
Created attachment 610764 [details] [diff] [review]
Patch v2 w/ test
Comment 9 Trevor Saunders (:tbsaunde) 2012-03-29 17:38:53 PDT
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!
Comment 11 Ed Morley [:emorley] 2012-03-30 12:55:26 PDT
Thanks for the patch - hopefully see you on IRC soon! :-)

https://hg.mozilla.org/mozilla-central/rev/38d50ee4b9ba

Note You need to log in before you can comment on or make changes to this bug.