[webkit-reviews] review granted: [Bug 21107] New access key combination conflicts with VoiceOver : [Attachment 24938] proposed fix

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Alexey Proskuryakov <ap at webkit.org>'s
request for review:
Bug 21107: New access key combination conflicts with VoiceOver
https://bugs.webkit.org/show_bug.cgi?id=21107

Attachment 24938: proposed fix
https://bugs.webkit.org/attachment.cgi?id=24938&action=edit

------- Additional Comments from Darin Adler <darin at apple.com>
> +	   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