Bug 370229 - Command image buttons cannot be used via keyboard
Summary: Command image buttons cannot be used via keyboard
Status: RESOLVED FIXED
Alias: None
Product: Orion (Archived)
Classification: ECD
Component: Client (show other bugs)
Version: 0.4   Edit
Hardware: PC Windows 7
: P3 normal (vote)
Target Milestone: 0.4 RC1   Edit
Assignee: Susan McCourt CLA
QA Contact:
URL:
Whiteboard:
Keywords: accessibility
Depends on:
Blocks: 365361
  Show dependency tree
 
Reported: 2012-01-31 10:29 EST by Max Li CLA
Modified: 2012-02-10 13:05 EST (History)
1 user (show)

See Also:
maxli: review?


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Max Li CLA 2012-01-31 10:29:18 EST
For the command buttons that have images (e.g. the add favorites button in the navigator or the add branch button in the repository page), while you can tab to them via keyboard, you cannot activate them by pressing Enter (or any other button).
Comment 1 Max Li CLA 2012-01-31 11:37:12 EST
I have fixed this bug.

The commit is in the bug370229 branch.

https://github.com/max-li/orion.client/commit/4a67cbd1179e9b9290bf53a4072083e170dbcdf8
Comment 2 Susan McCourt CLA 2012-01-31 12:29:22 EST
I was under the impression that a button role alone would fix the problem.  So can you explain what the keypress gives me vs. the button role?
Comment 3 Max Li CLA 2012-01-31 12:54:19 EST
(In reply to comment #2)
> I was under the impression that a button role alone would fix the problem.  So
> can you explain what the keypress gives me vs. the button role?

Setting an ARIA role does not currently have an effect if the user is not using an assistive technology (e.g. a screen reader). So in order to make the button usable for keyboard-only users who don't use a screen reader, you have to set the onkeypress event.
Comment 4 Susan McCourt CLA 2012-01-31 21:22:41 EST
(In reply to comment #3)
> (In reply to comment #2)
> > I was under the impression that a button role alone would fix the problem.  So
> > can you explain what the keypress gives me vs. the button role?
> 
> Setting an ARIA role does not currently have an effect if the user is not using
> an assistive technology (e.g. a screen reader). So in order to make the button
> usable for keyboard-only users who don't use a screen reader, you have to set
> the onkeypress event.

thanks for the explanation. 
I haven't consulted a keyboard/accessibility guide, but I do notice in web interfaces (like bugzilla) both enter and space key will activate a button.  Should we do this?
Comment 5 Max Li CLA 2012-02-01 11:49:05 EST
(In reply to comment #4)
> thanks for the explanation. 
> I haven't consulted a keyboard/accessibility guide, but I do notice in web
> interfaces (like bugzilla) both enter and space key will activate a button. 
> Should we do this?

Yes, you are right. The ARIA authoring practices (found here: http://www.w3.org/TR/wai-aria-practices/#button) do state that you should be able to use the spacebar too. I have updated the fix to reflect this.

Also, I noticed a case I missed in the original fix. In the case of the git status page, the compare button seems to actually function as a link, as opposed to the rest of the image buttons. I have updated the fix to take into account of this case.

Here is the new commit (in the bug370229 branch).

https://github.com/max-li/orion.client/commit/de4766aee25cfa6fb409e0908bbce899e9211e79
Comment 6 Susan McCourt CLA 2012-02-01 13:57:44 EST
thanks, Max.  May take me a couple days to get to this but I'll handle any merge issues.
Comment 7 Susan McCourt CLA 2012-02-03 04:01:42 EST
having finally finished bug 360986, I can look at this now.
Note that the implementation has changed a bit, we are now using spans in a button role with an image inside, vs. just images.  So I think the answer is to put the keypress handling on the span.

We shouldn't need to add any handling to the links.

I had hoped to switch to using real buttons in bug 360986 and avoid all this,  but the browser user agent styling for buttons interferes.
Comment 8 Max Li CLA 2012-02-07 11:26:50 EST
I have updated this fix to take into account the new implementation. In particular, since there is no longer text associated with the buttons (which used to be done with alt text), screen readers would just read button upon tabbing to a button. I have fixed this.

The commit is in the bug370229new branch.

https://github.com/max-li/orion.client/commit/0a3207a793728612ccff61da9c3f76dbff3d303f
Comment 9 Susan McCourt CLA 2012-02-07 12:41:29 EST
(In reply to comment #8)
> I have updated this fix to take into account the new implementation. In
> particular, since there is no longer text associated with the buttons (which
> used to be done with alt text), screen readers would just read button upon
> tabbing to a button. I have fixed this.
> 
> The commit is in the bug370229new branch.
> 
> https://github.com/max-li/orion.client/commit/0a3207a793728612ccff61da9c3f76dbff3d303f

great, thanks!
So...the part with the keyboard handling was fixed with our commit for bug 370760.  It made sense to use common code there, now that we can.

What I took from this commit was the setting of the ARIA label.  However I changed it slightly.  I use the name if there is one (the text that would have been on the text button).  I only use the tooltip if there is no name (which can happen)

So....I think I'm caught up on your fixes, if I have missed something please ping me on whatever bug needs review.

Can you verify that all buttons/links/etc. in toolbars, section headings, and in-line are now accessible?
Comment 10 Max Li CLA 2012-02-08 09:06:29 EST
(In reply to comment #9)
> Can you verify that all buttons/links/etc. in toolbars, section headings, and
> in-line are now accessible?

Yes, they all appear to be accessible.