[webkit-reviews] review granted: [Bug 119605] AX: Not able to use arrow keys to read text in a WK2 app : [Attachment 208380] Patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Beth Dakin <bdakin at apple.com>'s
request for review:
Bug 119605: AX: Not able to use arrow keys to read text in a WK2 app
https://bugs.webkit.org/show_bug.cgi?id=119605

Attachment 208380: Patch
https://bugs.webkit.org/attachment.cgi?id=208380&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=208380&action=review


> Source/WebCore/page/EventHandler.cpp:3349
> +	   // provides KB navigation and selection for enhanced accessibility
users

Given the title of this function and the code below, I am not sure we need this
comment any more. Or if we ever did!

> Source/WebKit2/ChangeLog:22
> +	   This function now gives accessibility a chance to handle the event
too. And it 
> +	   also actually tracks whether or not the event was handled by
scrolling instead of 
> +	   assuming that it was and universally returning true.

Why do we need to fix the boolean result here as part of this patch? Nice fix,
but how is it relevant?

> Source/WebKit2/WebProcess/WebPage/mac/WebPageMac.mm:633
> +    // First give accessibility a chance to handle the event.
> +    Frame* frame = frameForEvent(event);
> +   
frame->eventHandler()->handleKeyboardSelectionMovementForAccessibility(event);
> +    if (event->defaultHandled())
> +	   return true;

Can the event ever be zero?