Bug 21107 - New access key combination conflicts with VoiceOver
Summary: New access key combination conflicts with VoiceOver
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Accessibility (show other bugs)
Version: 528+ (Nightly build)
Hardware: Mac OS X 10.5
: P1 Normal
Assignee: Alexey Proskuryakov
URL: http://www.mail-archive.com/discuss@m...
Keywords: InRadar, Regression
Depends on:
Blocks:
 
Reported: 2008-09-25 09:59 PDT by moriond
Modified: 2008-11-07 01:24 PST (History)
3 users (show)

See Also:


Attachments
proposed fix (6.91 KB, patch)
2008-11-06 01:10 PST, Alexey Proskuryakov
darin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description moriond 2008-09-25 09:59:07 PDT
Access keys for Safari 3 no longer work in the recent night webkit builds.  These shortcut keys for the Mail Archive site for viewing archived messages as an accessibility feature are:
Control-n       (Next)            Later message by thread
Control-p       (Previous)      Earlier message by thread
Control-f        (Forward)       Later message by date
Control-b       (Back)            Earlier message by date
Control-i        (Index)           Chronological index
Control-c       (Contents)      Thread index

They work in Safari 3, and webkit builds through early September. No webkit nightly builds for the last week or two supports these.  Blind users must physically navigate to thread links and click them in order to read other archived messages.
Comment 1 Mark Rowe (bdash) 2008-09-25 10:04:23 PDT
The access key combination was changed from Ctrl to Ctrl-Opt in May to address bug 7466.
Comment 2 moriond 2008-09-26 02:19:44 PDT
This replaced combination (where use of Control as the access key has been replaced with the Control+Option combination)) does not work for any visually impaired (or mobility impaired or dyslexic) Mac user who is using the built-in VoiceOver Screen reader as an accessibility solution.  The basic VoiceOVer Keys (or VO-keys) are the Control-Option combination, and none of the access keys I listed in my report work if you turn VoiceOver on (Command-F5 to turn VoiceOver on or off on desktop Macs and recent Mac laptops; older laptops may need to use Fn-Command-F5 to toggle VoiceOver on or off).  Please try this.  VO-keys+I (Control-Option-i) is the item chooser menu; VO-keys+F is the Find command  for web page search navigation, etc. None of those access key combinations are accessible to anyone using VoiceOver as an accessibility solution. Would it be possible to use some other key combination like Option-Shift?  Could you please get in touch with someone on the Mac accessibility team?
Comment 3 Alexey Proskuryakov 2008-09-26 03:40:04 PDT
Control+Option was the only available combination I could find. For example, Option+Shift is used to enter extended characters (e.g. the Apple logo is Option+Shift+K), and thus cannot be used for access keys.

There is no doubt that we need to resolve the conflict with VoiceOver, now that we know about it.
Comment 4 Alexey Proskuryakov 2008-10-02 09:30:29 PDT
<rdar://problem/6264219>
Comment 5 Alexey Proskuryakov 2008-11-06 01:10:41 PST
Created attachment 24938 [details]
proposed fix

Use Ctrl for access keys if VoiceOver is enabled; keep Ctrl+Option otherwise.
Comment 6 Darin Adler 2008-11-06 09:30:55 PST
Comment on attachment 24938 [details]
proposed fix

> +        Use Ctrl when VoiceOver is enabled, because a conflict with Emacs-style key binsings is

Bindings rather than "binsings".

> +        * page/EventHandler.cpp: (WebCore::EventHandler::handleAccessKey):
> +        Also fix an access key matching bug introduced in r32424 - Any superset of specified
> +        modifier set invoked access keys. We can use simple equality comparison instead because
> +        CapsLock is not part of modifiers(), so it doesn't need to be ignored explicitly.

What about the shift key?

> +    // Control+Option key combinations are usually unused on Mac OS X, but not when VoiceOver is enabled.
> +    // So, we use Control in this case, even though it conflicts with Emacs-style key bindings.
> +    // See <https://bugs.webkit.org/show_bug.cgi?id=21107> for more detail.
> +    if (AXObjectCache::accessibilityEnhancedUserInterfaceEnabled())
> +        return PlatformKeyboardEvent::CtrlKey;
> +    else
> +        return PlatformKeyboardEvent::CtrlKey | PlatformKeyboardEvent::AltKey;

Even in parallel cases like this we normally don't do else after return.

r=me
Comment 7 Alexey Proskuryakov 2008-11-06 09:49:26 PST
(In reply to comment #6)
> > +        modifier set invoked access keys. We can use simple equality comparison instead because
> > +        CapsLock is not part of modifiers(), so it doesn't need to be ignored explicitly.
> 
> What about the shift key?

What I meant was: CapsLock state shouldn't matter when deciding whether current modifiers match access key ones. Since modifiers() doesn't include its state, there is no need to explicitly mask it out (other APIs are not that nice, so it's a common mistake).
Comment 8 Alexey Proskuryakov 2008-11-06 21:53:25 PST
Committed revision 38211.

Comment 9 Alexey Proskuryakov 2008-11-07 01:24:25 PST
Follow-up committed revision 38218. Shift behavior is actually more complicated than I thought - restored the old behavior for now, and added a FIXME describing what other browsers do.