[Webkit-unassigned] [Bug 142059] AX: Provide API for assistive tech to ignore DOM key event handlers

Wed Mar 4 22:04:47 PST 2015

https://bugs.webkit.org/show_bug.cgi?id=142059

Ryosuke Niwa <rniwa at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #247638|review?                     |review-
              Flags|                            |

--- Comment #15 from Ryosuke Niwa <rniwa at webkit.org> ---
Comment on attachment 247638
  --> https://bugs.webkit.org/attachment.cgi?id=247638
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=247638&action=review

Please add change logs. r- due to the lack of change logs.
See http://www.webkit.org/coding/contributing.html

> Source/WebCore/dom/EventDispatcher.cpp:353
> -    if (!event->propagationStopped() && !eventPath.isEmpty())
> +    if (!EventHandler::accessibilityPreventsDOMEventDispatch(*event, *node) && !event->propagationStopped() && !eventPath.isEmpty())

This is an extremely hot code path. We shouldn't be calling this function on every event dispatch.
Instead, we should be doing this work at where the event is created & dispatched for specific event names.

> Source/WebCore/page/EventHandler.cpp:3225
> +    const AtomicString& eventType = event.type();
> +    if (eventType != eventNames().keydownEvent && eventType != eventNames().keyupEvent && eventType != eventNames().keypressEvent)
> +        return false;

Please do this at where keydownEvent, keyupEvent, and keypressEvent are created.

> LayoutTests/platform/mac/accessibility/prevent-keyboard-event-dispatch-expected.txt:6
> +1
> +
> +2
> +
> +2
> +

Please hide these outputs.

> LayoutTests/platform/mac/accessibility/prevent-keyboard-event-dispatch.html:1
> +<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">

Wrong DOCTYPE.  Use HTML5 style DOCTYPE: <!DOCTYPE html>

> LayoutTests/platform/mac/accessibility/prevent-keyboard-event-dispatch.html:7
> +<!-- add a keydown handler that ignores all key events-->

This comment doesn't explain why. Please remove it.

> LayoutTests/platform/mac/accessibility/prevent-keyboard-event-dispatch.html:41
> +            var webArea = clearSelectionAndFocusOnWebArea();
> +            accessibilityController.enableEnhancedAccessibility(false);
> +            shouldBe("accessibilityController.enhancedAccessibilityEnabled", "false");

Please include "accessibilityController.enableEnhancedAccessibility(false);" in shouldBe as in
shouldBe("accessibilityController.enableEnhancedAccessibility(false); accessibilityController.enhancedAccessibilityEnabled", "false");
so that the expected result reads itself.

Otherwise, the person trying to interpret the test result has to read the test file separately.
Ditto for all other should* in this test.

> LayoutTests/platform/mac/accessibility/prevent-keyboard-event-dispatch.html:44
> +            

Please don't add whitespaces in blank lines.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20150305/2a707325/attachment-0002.html>