Last Comment Bug 720148 - ARIA button should expose press action
: ARIA button should expose press action
Status: RESOLVED FIXED
[good first bug][mentor=surkov.alexan...
: access
Product: Core
Classification: Components
Component: Disability Access APIs (show other bugs)
: unspecified
: All All
: -- normal (vote)
: mozilla13
Assigned To: Murali
:
Mentors:
Depends on:
Blocks: aria
  Show dependency treegraph
 
Reported: 2012-01-21 11:15 PST by alexander :surkov
Modified: 2012-02-02 06:55 PST (History)
2 users (show)
surkov.alexander: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Buttons made to expose Press Action (2.47 KB, patch)
2012-01-31 04:59 PST, Murali
surkov.alexander: review+
Details | Diff | Splinter Review

Description alexander :surkov 2012-01-21 11:15:28 PST
ARIA button exposes click action while it should expose 'press' action what makes compatible with native button widgets.
Comment 1 alexander :surkov 2012-01-21 11:18:42 PST
1) Introduce ePressAction in EActionRule
2) Change eClickAction to ePressAction in nsARAIMap.cpp for "button" role (http://mxr.mozilla.org/mozilla-central/source/accessible/src/base/nsARIAMap.cpp#108)
3) Fix nsAccessible::GetActionName
4) add tests at actions/test_aria.html (http://mxr.mozilla.org/mozilla-central/source/accessible/tests/mochitest/actions/test_aria.html?force=1)
Comment 2 Murali 2012-01-23 22:40:55 PST
Hi, I'm new here. I am interested in fixing this bug. Can you please guide me? Thanks!
Comment 3 alexander :surkov 2012-01-25 19:36:28 PST
(In reply to Murali from comment #2)
> Hi, I'm new here. I am interested in fixing this bug. Can you please guide
> me? Thanks!

You need to set up your build environment and etc (follow https://developer.mozilla.org/en/Introduction). When you're ready to work on patch then follow instructions given in comment #1. You can use http://mxr.mozilla.org/mozilla-central tool to locate methods, class names etc. If you have question then don't hesitate to ask.
Comment 4 Murali 2012-01-26 02:40:03 PST
I have made it to step 2 successfully.

Can you please tell me whether my understanding of step 3 is correct?

Change the following in nsAccessible.cpp
1843    case eClickAction:
1844      aName.AssignLiteral("click");
1845      return NS_OK;

to

1843    case ePressAction:
1844      aName.AssignLiteral("press");
1845      return NS_OK;

Also, I'm not quite sure what to do in step 4. Could you please guide me? Thanks! :)
Comment 5 alexander :surkov 2012-01-26 18:08:37 PST
(In reply to Murali from comment #4)
> I have made it to step 2 successfully.
> 
> Can you please tell me whether my understanding of step 3 is correct?
> 
> Change the following in nsAccessible.cpp
> 1843    case eClickAction:
> 1844      aName.AssignLiteral("click");
> 1845      return NS_OK;
> 
> to
> 
> 1843    case ePressAction:
> 1844      aName.AssignLiteral("press");
> 1845      return NS_OK;

add (not replace)

> Also, I'm not quite sure what to do in step 4. Could you please guide me?
> Thanks! :)

you need to find role="button" elements and make sure they expose "press" action (not "click" action). All actions are described by actionsArray variable. To get an idea how these things work exactly you need to look at ../actions.js file.
Comment 6 Murali 2012-01-31 04:59:17 PST
Created attachment 593048 [details] [diff] [review]
Buttons made to expose Press Action

Could you kindly let me know of the changes I might have to do to this patch? Thank you so much for the help!
Comment 7 alexander :surkov 2012-01-31 05:03:59 PST
Comment on attachment 593048 [details] [diff] [review]
Buttons made to expose Press Action

r=me, thanks

do you want me to land the patch?
Comment 8 Murali 2012-01-31 05:13:59 PST
Sure, thanks a lot for all the guidance! I am really looking forward to work on more challenging bugs soon! :)
Comment 9 alexander :surkov 2012-01-31 05:17:01 PST
(In reply to Murali from comment #8)
> Sure, thanks a lot for all the guidance! I am really looking forward to work
> on more challenging bugs soon! :)

since you know our tests structure then you might want to look at bug 702560
Comment 11 alexander :surkov 2012-02-01 18:17:44 PST
inbound land https://hg.mozilla.org/integration/mozilla-inbound/rev/42705e4d1175
Comment 12 Ed Morley [:emorley] 2012-02-02 06:55:50 PST
https://hg.mozilla.org/mozilla-central/rev/42705e4d1175

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