136696 - Active Accessibility: Support get_accKeyboardShortcut

Status: VERIFIED FIXED

(Core :: Disability Access APIs, defect)

Description

[Aaron Leventhal]

When we first analyzed accessibility support, we didn't think we would need to
support get_accKeyboardShortcut. However, screen readers need this in ordr to
report to the user what the shortcut key is.

The implementation is simple: we need to check the accesskey attribute. For menu
items, we report that single letter value in a string, such as "r". For other
controls, we need to report something like "Alt+r".

Unfortunately, we'll need to add a method nsIAccessible, there currently is no
support at all for this. Accessible::get_accKeyboardShortcut simply returns null
at the moment.

Comment 1

[Aaron Leventhal]

Attachment: This page has HTML accesskeys - we'll need to report the correct modifier with the accesskey

Comment 2

[Aaron Leventhal]

Attachment: This page has HTML accesskeys - we'll need to report the correct modifier with the accesskey

Comment 3

[Aaron Leventhal]

Attachment: This fixes the hardcoding in nsEventStateManager and uses a pref for the accelkey.

Hmm, how can I get rid of the platform-specific coding in nsAccessible.cpp and
nsMenuAccessible.cpp? An ideas?

Once this gets checked in, we'll put the new pref in the customizing document.

Need to let it bake on the trunk. Then I'll come up with a patch for the branch
that doesn't add any localized strings.

Comment 4

[Akkana Peck]

Comment on attachment 78843 [details] [diff] [review]
This fixes the hardcoding in nsEventStateManager and uses a pref for the accelkey.

r=akkana
Comments (none of these block the review, and the review holds whether or not
you decide to change any of these):
- I wonder if gGeneralAccessModifier should default to nonzero (18?) if there's
no prefs service?  Your call, just wondered if you'd thought about it.
- I wish there was some way to have platform-specific .properties files and
avoid the platform ifdefs.  But Aaron and I talked about that and there seems
to be no easy way (no current mechanism for that).
- > // No need to cache pref service, this happens rarely
I'll take your word on that, thanks for putting in the comment for people like
me who wondered. :-)
- I'm not clear on why the windows-only code in windows/Accessible.cpp, though
the code looks fine.
- I might have put the comment in macprefs.js right before generalAccessKey, or
does it also affect menuAccessKey?

Comment 5

[sairuh (rarely reading bugmail)]

updating qa engr...

Comment 6

[Aaron Leventhal]

Attachment: New, better patch does not use ifdefs in accessibility code, makes use of existing platformKeys.properties strings

Seeking new r=

Akkana, I know you already gave me r= for the last one, but this one avoids
using ifdef's in the accessibility module. It also adds no new strings, by
making use of the existing platformKeys.properties file.

Comment 7

[Akkana Peck]

Comment on attachment 81125 [details] [diff] [review]
New, better patch does not use ifdefs in accessibility code, makes use of existing platformKeys.properties strings

Cool -- nice solution!	r=akkana.

One comment: it looks like the inclusion of nsIStringBundle.h in nsAccessible.h
is only for the declared pointers to nsIStringBundle; if that's the case, you
could get by with "class nsIStringBundle" in the .h file and include
nsIStringBundle.h in nsAccessible.cpp.
(If you do that, consider it r=akkana in advance.)

Comment 8

[Johnny Stenback (:jst)]

Comment on attachment 81125 [details] [diff] [review]
New, better patch does not use ifdefs in accessibility code, makes use of existing platformKeys.properties strings

sr=jst

Comment 9

[Aaron Leventhal]

checked in

Comment 10

[Blake Ross]

Was this checked in on the branch? If so, please add the fixed1.0.1 keyword.

Comment 11

[Paul Wyskoczka]

V per Aaron comments (rs)

Comment 12

[Firefox Bug Husbandry Bot]

https://bugzilla.mozilla.org/show_bug.cgi?id=1527381