AX: Provide API for assistive tech to ignore DOM key event handlers

• Component: Accessibility
• Status: RESOLVED FIXED

[Doug Russell]

Assistive technology applications on the desktop are heavily dependent on keyboard navigation being reliable. This is greatly hindered by sites that handle key events without updating keyboard selection and then consume the event. It is important for assistive technology apps to allow users to decide to ignore these handlers that are incorrect for their purposes.

This can be fixed by exposing, via a new accessibility attribute, a way to decide, for a given WebCore::Frame, to pre-empt DOM dispatch and instead let accessibility caret browsing take place.

[Radar WebKit Bug Importer]

<rdar://problem/19976512>

[Doug Russell]

Created attachment 247449 [details]
Ignore Dom

[Doug Russell]

Created attachment 247450 [details]
Patch

[chris fleizach]

Comment on attachment 247450 [details]
Patch

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

> Source/WebCore/accessibility/AccessibilityObject.h:986
> +    void setCaretBrowsingEnabled(bool);

these are part of your previous patch right?
we should probably get that other one ingested first or strip out that code for this patch first so that it doesn't look like we're reviewing two things

[Doug Russell]

It is. I wanted to confirm everything would build, etc. I'll strip all that out once the other commit lands.

[Doug Russell]

Created attachment 247479 [details]
Patch

[chris fleizach]

Comment on attachment 247479 [details]
Patch

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

> Source/WebCore/dom/EventDispatcher.cpp:353
> +#if PLATFORM(COCOA) && !PLATFORM(IOS)

is there a reason we don't want this for iOS? it seems like it might be useful someday

> Source/WebCore/dom/EventDispatcher.cpp:358
> +    if (!preventDOMDispatch && !event->propagationStopped() && !eventPath.isEmpty())

instead of putting the #define here, i think moving it into the method itself would be a bit cleaner. then you don't need to make a separate variable to hold the result

> Source/WebCore/page/EventHandler.cpp:3218
> +bool EventHandler::accessibilityWantsToHandleEvent(Event& event, Node& node)

My suggestion for a name is "accessibilityPreventsEventDispatch())

> Source/WebCore/page/EventHandler.cpp:3220
> +    if (!AXObjectCache::accessibilityEnhancedUserInterfaceEnabled())

you might want to check accessibilityEnabled and enhancedUserInterface enabled, in case they mistakenly got out of date

> Source/WebCore/page/EventHandler.cpp:3224
> +    if (eventType != eventNames().keydownEvent && eventType != eventNames().keypressEvent)

what about keyUp? seems like webapps could get into a bad state if they only got keyUps

> Source/WebCore/page/EventHandler.cpp:3235
> +    Frame* frame = nodeAccessibility->frame();

i don't think you need to get an axObject from the node in order to just get back to the frame

I think you can just do node.document()->frame()

> Source/WebCore/page/EventHandler.cpp:3241
> +        // Tab

instead of adding comments that say what they are which are, can you add a comment explaining why we are snarfing tab and arrow keys

> Source/WebCore/page/Settings.in:48
> +ignoreDOMKeyEventHandlers initial=false

i'm not crazy about the name.
One idea: preventKeyboardEventDispatch

[Doug Russell]

(In reply to comment #7)
> Comment on attachment 247479 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=247479&action=review
> 
> > Source/WebCore/dom/EventDispatcher.cpp:353
> > +#if PLATFORM(COCOA) && !PLATFORM(IOS)
> 
> is there a reason we don't want this for iOS? it seems like it might be
> useful someday

No reason. I just wanted to constrain it to the platforms I had actually tested it on for now. I'll open it up to iOS.

> 
> > Source/WebCore/dom/EventDispatcher.cpp:358
> > +    if (!preventDOMDispatch && !event->propagationStopped() && !eventPath.isEmpty())
> 
> instead of putting the #define here, i think moving it into the method
> itself would be a bit cleaner. then you don't need to make a separate
> variable to hold the result

That should be doable.

> 
> > Source/WebCore/page/EventHandler.cpp:3218
> > +bool EventHandler::accessibilityWantsToHandleEvent(Event& event, Node& node)
> 
> My suggestion for a name is "accessibilityPreventsEventDispatch())

That works. I'll make the attribute rename you suggested later to make it all match.

> 
> > Source/WebCore/page/EventHandler.cpp:3220
> > +    if (!AXObjectCache::accessibilityEnhancedUserInterfaceEnabled())
> 
> you might want to check accessibilityEnabled and enhancedUserInterface
> enabled, in case they mistakenly got out of date

Will do.

> 
> > Source/WebCore/page/EventHandler.cpp:3224
> > +    if (eventType != eventNames().keydownEvent && eventType != eventNames().keypressEvent)
> 
> what about keyUp? seems like webapps could get into a bad state if they only
> got keyUps

Good point. I imagine that could go weird.

> 
> > Source/WebCore/page/EventHandler.cpp:3235
> > +    Frame* frame = nodeAccessibility->frame();
> 
> i don't think you need to get an axObject from the node in order to just get
> back to the frame
> 
> I think you can just do node.document()->frame()

I'll try that.

> 
> > Source/WebCore/page/EventHandler.cpp:3241
> > +        // Tab
> 
> instead of adding comments that say what they are which are, can you add a
> comment explaining why we are snarfing tab and arrow keys

Will do.

> 
> > Source/WebCore/page/Settings.in:48
> > +ignoreDOMKeyEventHandlers initial=false
> 
> i'm not crazy about the name.
> One idea: preventKeyboardEventDispatch

[Doug Russell]

Created attachment 247638 [details]
Patch

[Doug Russell]

I investigated how accessibilityEnabled() is used vs accessibilityEnhancedUserInterfaceEnabled() and I think it's use is appropriate as is. Enabled dictates if the cache is currently producing accessibility objects, where as enhanced dictates if an assistive tech application is actively interacting. In this case event routing and the existence of accessibility objects aren't meant to be coupled in that way.

[James Craig]

(In reply to comment #0)
> This can be fixed by exposing, via a new accessibility attribute, a way to
> decide, for a given WebCore::Frame, to pre-empt DOM dispatch and instead let
> accessibility caret browsing take place.

This makes me nervous. Are you planning to propose new web API that indicates to an web application when no key handlers are available?

[Doug Russell]

(In reply to comment #11)
> (In reply to comment #0)
> > This can be fixed by exposing, via a new accessibility attribute, a way to
> > decide, for a given WebCore::Frame, to pre-empt DOM dispatch and instead let
> > accessibility caret browsing take place.
> 
> This makes me nervous. Are you planning to propose new web API that
> indicates to an web application when no key handlers are available?

If there should be an API to surface this behavior and what that API looks like will need to be informed by actual usage that hasn't happened yet, so I think that would be premature.

This is primarily (right now) about to deferring to users to decide how keyboard events are processed.

[chris fleizach]

Comment on attachment 247638 [details]
Patch

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

> Source/WebCore/accessibility/AccessibilityObject.cpp:2608
> +#if PLATFORM(COCOA)

doesn't look like we need to require the COCOA platform here. looks like everything should compile for other platform (but it may not work yet)

[chris fleizach]

Antti, could you also take a look at this?

[Ryosuke Niwa]

Comment on attachment 247638 [details]
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.

[Doug Russell]

Created attachment 248278 [details]
Patch

[Beth Dakin]

Comment on attachment 248278 [details]
Patch

It looks like you are allowing keyup to be sent still. You should consider if there are any websites that will be tripped up by this if they are expecting keydown to be sent before keyup. One possible solution to that would be to avoid sending keyup as well, but maybe it's not going to be an issue.

[Doug Russell]

Created attachment 248561 [details]
Updated to also stop propagation on key up

[Beth Dakin]

Comment on attachment 248561 [details]
Updated to also stop propagation on key up

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

> Source/WebCore/page/EventHandler.cpp:3252
> +    return false;

It looks like the EWS bot for EFL is failing because of the unused variable outside of the #if PLATFORM(COCOA). Build error:

../../Source/WebCore/page/EventHandler.cpp:3236:6: error: unused parameter 'event' [-Werror=unused-parameter]
 bool EventHandler::accessibilityPreventsEventPropogation(KeyboardEvent* event)

You can handle this with:

#else
UNUSED_PARAM(event);
#endif
return false;

[Doug Russell]

Created attachment 248576 [details]
Update for UNUSED_PARAM(event);

[Doug Russell]

Comment on attachment 248576 [details]
Update for UNUSED_PARAM(event);

>Subversion Revision: 181455
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index c86364bc9a4a0b5db026a46838c5cb7fbc8bbf64..96f9a946e01feed8c4b6543bf783c0b69cb23456 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,34 @@
>[phone]  Doug Russell  <[e-mail]>
>+
>+        AX: Provide API for assistive tech to ignore DOM key event handlers
>+        https://bugs.webkit.org/show_bug.cgi?id=142059
>+
>+        Reviewed by Chris Fleizach, Beth Dakin.
>+
>+        Assistive technology applications on the desktop are heavily dependent on keyboard navigation being reliable. This is greatly hindered by sites that handle key events without updating keyboard selection and then consume the event. It is important for assistive technology apps to allow users to decide to ignore these handlers that are incorrect for their purposes.
>+
>+        This can be fixed by exposing, via a new accessibility attribute, a way to decide, for a given WebCore::Frame, to pre-empt DOM dispatch and instead let accessibility caret browsing take place.
>+
>+        Test: platform/mac/accessibility/prevent-keyboard-event-dispatch.html
>+
>+        * accessibility/AccessibilityObject.cpp:
>+        (WebCore::AccessibilityObject::preventKeyboardDOMEventDispatch):
>+        (WebCore::AccessibilityObject::setPreventKeyboardDOMEventDispatch):
>+        * accessibility/AccessibilityObject.h:
>+        * accessibility/mac/WebAccessibilityObjectWrapperMac.mm:
>+        (-[WebAccessibilityObjectWrapper accessibilityAttributeNames]):
>+        (-[WebAccessibilityObjectWrapper accessibilityAttributeValue:]):
>+        (-[WebAccessibilityObjectWrapper accessibilityIsAttributeSettable:]):
>+        (-[WebAccessibilityObjectWrapper _accessibilitySetValue:forAttribute:]):
>+        * dom/Element.cpp:
>+        (WebCore::Element::dispatchKeyEvent):
>+        * page/EventHandler.cpp:
>+        (WebCore::EventHandler::keyEvent):
>+        (WebCore::handleKeyboardSelectionMovement):
>+        (WebCore::EventHandler::handleKeyboardSelectionMovementForAccessibility):
>+        * page/EventHandler.h:
>+        * page/Settings.in:
>+
> 2015-03-12  Dan Bernstein  <[e-mail]>
> 
>         Finish up <rdar://problem/20086546> [Cocoa] Add an option to treat certificate chains with SHA1-signed certificates as insecure
>diff --git a/Source/WebCore/accessibility/AccessibilityObject.cpp b/Source/WebCore/accessibility/AccessibilityObject.cpp
>index 8b4915ec7ad2cdc364a5fca255bad6512f6cde0b..258bf632fd1e118dcaf679f6ae175e8964ed53ab 100644
>--- a/Source/WebCore/accessibility/AccessibilityObject.cpp
>+++ b/Source/WebCore/accessibility/AccessibilityObject.cpp
>@@ -2605,4 +2605,20 @@ void AccessibilityObject::elementsFromAttribute(Vector<Element*>& elements, cons
>     }
> }
> 
>+#if PLATFORM(COCOA)
>+bool AccessibilityObject::preventKeyboardDOMEventDispatch() const
>+{
>+    Frame* frame = this->frame();
>+    return frame && frame->settings().preventKeyboardDOMEventDispatch();
>+}
>+
>+void AccessibilityObject::setPreventKeyboardDOMEventDispatch(bool on)
>+{
>+    Frame* frame = this->frame();
>+    if (!frame)
>+        return;
>+    frame->settings().setPreventKeyboardDOMEventDispatch(on);
>+}
>+#endif
>+
> } // namespace WebCore
>diff --git a/Source/WebCore/accessibility/AccessibilityObject.h b/Source/WebCore/accessibility/AccessibilityObject.h
>index 176fade31c449e8a1b67a7e78c0be83d0825677e..04188ee67a6a08935e3a4b75b193a8938180f63a 100644
>--- a/Source/WebCore/accessibility/AccessibilityObject.h
>+++ b/Source/WebCore/accessibility/AccessibilityObject.h
>@@ -978,6 +978,11 @@ public:
>     // other operations update type operations
>     void updateBackingStore();
>     
>+#if PLATFORM(COCOA)
>+    bool preventKeyboardDOMEventDispatch() const;
>+    void setPreventKeyboardDOMEventDispatch(bool);
>+#endif
>+    
> #if PLATFORM(COCOA) && !PLATFORM(IOS)
>     bool caretBrowsingEnabled() const;
>     void setCaretBrowsingEnabled(bool);
>diff --git a/Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm b/Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm
>index 74324f007864e3478622303cd6e7e478ce77eb03..801436539ff7cfb2b2d9a2f349a4d90338a90b1d 100644
>--- a/Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm
>+++ b/Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm
>@@ -464,6 +464,10 @@ using namespace HTMLNames;
> #define NSAccessibilityMathPrescriptsAttribute @"AXMathPrescripts"
> #define NSAccessibilityMathPostscriptsAttribute @"AXMathPostscripts"
> 
>+#ifndef NSAccessibilityPreventKeyboardDOMEventDispatchAttribute
>+#define NSAccessibilityPreventKeyboardDOMEventDispatchAttribute @"AXPreventKeyboardDOMEventDispatch"
>+#endif
>+
> #ifndef NSAccessibilityCaretBrowsingEnabledAttribute
> #define NSAccessibilityCaretBrowsingEnabledAttribute @"AXCaretBrowsingEnabled"
> #endif
>@@ -1359,6 +1363,7 @@ static id textMarkerRangeFromVisiblePositions(AXObjectCache *cache, VisiblePosit
>         [tempArray addObject:NSAccessibilityLoadingProgressAttribute];
>         [tempArray addObject:NSAccessibilityURLAttribute];
>         [tempArray addObject:NSAccessibilityCaretBrowsingEnabledAttribute];
>+        [tempArray addObject:NSAccessibilityPreventKeyboardDOMEventDispatchAttribute];
>         webAreaAttrs = [[NSArray alloc] initWithArray:tempArray];
>         [tempArray release];
>     }
>@@ -2958,6 +2963,9 @@ static NSString* roleValueToNSString(AccessibilityRole value)
>     if ([attributeName isEqualToString:@"AXDRTElementIdAttribute"])
>         return m_object->getAttribute(idAttr);
>     
>+    if (m_object->isWebArea() && [attributeName isEqualToString:NSAccessibilityPreventKeyboardDOMEventDispatchAttribute])
>+        return [NSNumber numberWithBool:m_object->preventKeyboardDOMEventDispatch()];
>+    
>     if (m_object->isWebArea() && [attributeName isEqualToString:NSAccessibilityCaretBrowsingEnabledAttribute])
>         return [NSNumber numberWithBool:m_object->caretBrowsingEnabled()];
>     
>@@ -3033,6 +3041,9 @@ static NSString* roleValueToNSString(AccessibilityRole value)
>     if ([attributeName isEqualToString:NSAccessibilityGrabbedAttribute])
>         return YES;
>     
>+    if (m_object->isWebArea() && [attributeName isEqualToString:NSAccessibilityPreventKeyboardDOMEventDispatchAttribute])
>+        return YES;
>+    
>     if (m_object->isWebArea() && [attributeName isEqualToString:NSAccessibilityCaretBrowsingEnabledAttribute])
>         return YES;
>     
>@@ -3352,6 +3363,8 @@ static NSString* roleValueToNSString(AccessibilityRole value)
>             m_object->setSelectedRows(selectedRows);
>     } else if ([attributeName isEqualToString:NSAccessibilityGrabbedAttribute])
>         m_object->setARIAGrabbed([number boolValue]);
>+    else if (m_object->isWebArea() && [attributeName isEqualToString:NSAccessibilityPreventKeyboardDOMEventDispatchAttribute])
>+        m_object->setPreventKeyboardDOMEventDispatch([number boolValue]);
>     else if (m_object->isWebArea() && [attributeName isEqualToString:NSAccessibilityCaretBrowsingEnabledAttribute])
>         m_object->setCaretBrowsingEnabled([number boolValue]);
> }
>diff --git a/Source/WebCore/dom/Element.cpp b/Source/WebCore/dom/Element.cpp
>index 5dd97aae0cd77fb839ad7d6f324feafa0bfef91c..222aebc767f621de094e2f751558c0b4593fe727 100644
>--- a/Source/WebCore/dom/Element.cpp
>+++ b/Source/WebCore/dom/Element.cpp
>@@ -39,6 +39,7 @@
> #include "ElementIterator.h"
> #include "ElementRareData.h"
> #include "EventDispatcher.h"
>+#include "EventHandler.h"
> #include "FlowThreadController.h"
> #include "FocusController.h"
> #include "FocusEvent.h"
>@@ -289,6 +290,10 @@ bool Element::dispatchWheelEvent(const PlatformWheelEvent& event)
> bool Element::dispatchKeyEvent(const PlatformKeyboardEvent& platformEvent)
> {
>     RefPtr<KeyboardEvent> event = KeyboardEvent::create(platformEvent, document().defaultView());
>+    if (Frame* frame = document().frame()) {
>+        if (frame->eventHandler().accessibilityPreventsEventPropogation(event.get()))
>+            event->stopPropagation();
>+    }
>     return EventDispatcher::dispatchEvent(this, event) && !event->defaultHandled();
> }
> 
>diff --git a/Source/WebCore/page/EventHandler.cpp b/Source/WebCore/page/EventHandler.cpp
>index de15d8208f6ec811c68bebe7a3f819ed31508b69..5647d3b01018a9ea6fb316e3d80daaeffb0490eb 100644
>--- a/Source/WebCore/page/EventHandler.cpp
>+++ b/Source/WebCore/page/EventHandler.cpp
>@@ -3087,6 +3087,9 @@ bool EventHandler::keyEvent(const PlatformKeyboardEvent& initialKeyEvent)
>         keydown->setTarget(element);
>         keydown->setDefaultHandled();
>     }
>+    
>+    if (accessibilityPreventsEventPropogation(keydown.get()))
>+        keydown->stopPropagation();
> 
>     element->dispatchEvent(keydown, IGNORE_EXCEPTION);
>     // If frame changed as a result of keydown dispatch, then return early to avoid sending a subsequent keypress message to the new frame.
>@@ -3230,6 +3233,27 @@ void EventHandler::handleKeyboardSelectionMovementForAccessibility(KeyboardEvent
>     }
> }
> 
>+bool EventHandler::accessibilityPreventsEventPropogation(KeyboardEvent* event)
>+{
>+#if PLATFORM(COCOA)
>+    if (!AXObjectCache::accessibilityEnhancedUserInterfaceEnabled())
>+        return false;
>+
>+    if (!m_frame.settings().preventKeyboardDOMEventDispatch())
>+        return false;
>+
>+    // Check for key events that are relevant to accessibility: tab and arrows keys that change focus
>+    if (event->keyIdentifier() == "U+0009")
>+        return true;
>+    FocusDirection direction = focusDirectionForKey(event->keyIdentifier());
>+    if (direction != FocusDirectionNone)
>+        return true;
>+#else
>+    UNUSED_PARAM(event);
>+#endif
>+    return false;
>+}
>+
> void EventHandler::defaultKeyboardEventHandler(KeyboardEvent* event)
> {
>     if (event->type() == eventNames().keydownEvent) {
>diff --git a/Source/WebCore/page/EventHandler.h b/Source/WebCore/page/EventHandler.h
>index 3720f6c1531a20a750da6146a4638cf2949d7d25..5df29d033285d7a59ce8a477c9623ee35a166c95 100644
>--- a/Source/WebCore/page/EventHandler.h
>+++ b/Source/WebCore/page/EventHandler.h
>@@ -239,6 +239,7 @@ public:
>     WEBCORE_EXPORT bool keyEvent(const PlatformKeyboardEvent&);
>     void defaultKeyboardEventHandler(KeyboardEvent*);
> 
>+    bool accessibilityPreventsEventPropogation(KeyboardEvent*);
>     WEBCORE_EXPORT void handleKeyboardSelectionMovementForAccessibility(KeyboardEvent*);
> 
>     bool handleTextInputEvent(const String& text, Event* underlyingEvent = 0, TextEventInputType = TextEventInputKeyboard);
>diff --git a/Source/WebCore/page/Settings.in b/Source/WebCore/page/Settings.in
>index f45314ca92f90cb2973827df415baeef2fe5aa48..22d15de260b08006caf57d0fbb603f6ebd9d6b52 100644
>--- a/Source/WebCore/page/Settings.in
>+++ b/Source/WebCore/page/Settings.in
>@@ -45,6 +45,7 @@ maximumHTMLParserDOMTreeDepth type=unsigned, initial=defaultMaximumHTMLParserDOM
> loadsSiteIconsIgnoringImageLoadingSetting initial=false
> 
> caretBrowsingEnabled initial=false
>+preventKeyboardDOMEventDispatch initial=false
> localStorageEnabled initial=false
> allowUniversalAccessFromFileURLs initial=true
> allowFileAccessFromFileURLs initial=true
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index d8f85bf758fd0fa27d71a12e472d13ebc3b098ec..d1dec88f01fd49e093e150071769d1f59f6f9c4a 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,19 @@
>[phone]  Doug Russell  <[e-mail]>
>+
>+        AX: Provide API for assistive tech to ignore DOM key event handlers
>+        https://bugs.webkit.org/show_bug.cgi?id=142059
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Assistive technology applications on the desktop are heavily dependent on keyboard navigation being reliable. This is greatly hindered by sites that handle key events without updating keyboard selection and then consume the event. It is important for assistive technology apps to allow users to decide to ignore these handlers that are incorrect for their purposes.
>+
>+        This can be fixed by exposing, via a new accessibility attribute, a way to decide, for a given WebCore::Frame, to pre-empt DOM dispatch and instead let accessibility caret browsing take place.
>+
>+        * accessibility/parent-delete-expected.txt:
>+        * platform/mac/accessibility/document-attributes-expected.txt:
>+        * platform/mac/accessibility/prevent-keyboard-event-dispatch-expected.txt: Added.
>+        * platform/mac/accessibility/prevent-keyboard-event-dispatch.html: Added.
>+
> 2015-03-12  Simon Fraser  <[e-mail]>
> 
>         These hidpi filter tests pass sometimes.
>diff --git a/LayoutTests/accessibility/parent-delete-expected.txt b/LayoutTests/accessibility/parent-delete-expected.txt
>index 42467546e244b3e95800137d2c05b2b7a47cfbf9..921ab78a11d9c07742eb9ae5b7d83495f4f07b7e 100644
>--- a/LayoutTests/accessibility/parent-delete-expected.txt
>+++ b/LayoutTests/accessibility/parent-delete-expected.txt
>@@ -28,5 +28,6 @@ AXLayoutCount: 2
> AXLoadingProgress: 1
> AXURL: LayoutTests/accessibility/parent-delete.html
> AXCaretBrowsingEnabled: 0
>+AXPreventKeyboardDOMEventDispatch: 0
> AXElementBusy: 0
> 
>diff --git a/LayoutTests/platform/mac/accessibility/document-attributes-expected.txt b/LayoutTests/platform/mac/accessibility/document-attributes-expected.txt
>index 8f4bae1671d8544c5f2167ee4d9410c03d69d732..b1958264df2aa05641c8628109b58325aa8c7321 100644
>--- a/LayoutTests/platform/mac/accessibility/document-attributes-expected.txt
>+++ b/LayoutTests/platform/mac/accessibility/document-attributes-expected.txt
>@@ -27,6 +27,7 @@ AXLayoutCount: 2
> AXLoadingProgress: 1
> AXURL: LayoutTests/platform/mac/accessibility/document-attributes.html
> AXCaretBrowsingEnabled: 0
>+AXPreventKeyboardDOMEventDispatch: 0
> AXElementBusy: 0
> 
> 
>diff --git a/LayoutTests/platform/mac/accessibility/prevent-keyboard-event-dispatch-expected.txt b/LayoutTests/platform/mac/accessibility/prevent-keyboard-event-dispatch-expected.txt
>new file mode 100644
>index 0000000000000000000000000000000000000000..b4d6b82e9c861e925db19f1f0962219b66f233da
>--- /dev/null
>+++ b/LayoutTests/platform/mac/accessibility/prevent-keyboard-event-dispatch-expected.txt
>@@ -0,0 +1,29 @@
>+1
>+
>+2
>+
>+2
>+
>+This tests ignoring javascript key handlers that consume key events.
>+
>+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
>+
>+
>+PASS webArea.role is 'AXRole: AXWebArea'
>+PASS caretBrowsingEnabled(webArea) is false
>+PASS accessibilityController.enhancedAccessibilityEnabled is false
>+PASS preventKeyboardDOMEventDispatch(webArea) is false
>+PASS elementAtStartMarkerOfSelectedTextMarkerRange(webArea).stringValue is 'AXValue: 1'
>+PASS elementAtStartMarkerOfSelectedTextMarkerRange(webArea).stringValue is 'AXValue: 1'
>+PASS elementAtStartMarkerOfSelectedTextMarkerRange(webArea).stringValue is 'AXValue: 1'
>+PASS keycount() is '2'
>+PASS accessibilityController.enhancedAccessibilityEnabled is true
>+PASS preventKeyboardDOMEventDispatch(webArea) is true
>+PASS elementAtStartMarkerOfSelectedTextMarkerRange(webArea).stringValue is 'AXValue: 1'
>+PASS elementAtStartMarkerOfSelectedTextMarkerRange(webArea).stringValue is 'AXValue: 1'
>+PASS elementAtStartMarkerOfSelectedTextMarkerRange(webArea).stringValue is 'AXValue: 2'
>+PASS keycount() is '2'
>+PASS successfullyParsed is true
>+
>+TEST COMPLETE
>+
>diff --git a/LayoutTests/platform/mac/accessibility/prevent-keyboard-event-dispatch.html b/LayoutTests/platform/mac/accessibility/prevent-keyboard-event-dispatch.html
>new file mode 100644
>index 0000000000000000000000000000000000000000..8c58f3de51acf3ef1b2a0e7c33b9fdea99514a03
>--- /dev/null
>+++ b/LayoutTests/platform/mac/accessibility/prevent-keyboard-event-dispatch.html
>@@ -0,0 +1,77 @@
>+<!DOCTYPE html>
>+<html>
>+<head>
>+    <script src="../../../resources/js-test-pre.js"></script>
>+    <script src="resources/accessibility-helper.js"></script>
>+</head>
>+<body id="body" onkeydown="return keydown();">
>+    <script>
>+    function preventKeyboardDOMEventDispatch(webArea) {
>+        return webArea.boolAttributeValue("AXPreventKeyboardDOMEventDispatch");
>+    }
>+    function setPreventKeyboardDOMEventDispatch(webArea, value) {
>+        webArea.setBoolAttributeValue("AXPreventKeyboardDOMEventDispatch", value);
>+    }
>+    function keydown(event) {
>+        var element = document.getElementById("keydowncount");
>+        element.innerHTML = parseInt(element.innerHTML) + 1;
>+        return false;
>+    }
>+    function keycount() {
>+        return document.getElementById("keydowncount").innerHTML;
>+    }
>+    </script>
>+    <div>
>+        <p>1</p>
>+        <p>2</p>
>+        <p id="keydowncount">0<p>
>+    </div>
>+    <div id="console"></div>
>+    <script>
>+    description("This tests ignoring javascript key handlers that consume key events.");
>+    if (window.testRunner) {
>+
>+        testRunner.dumpAsText();
>+
>+        if (window.accessibilityController && window.eventSender) {
>+
>+            var webArea = clearSelectionAndFocusOnWebArea();
>+            accessibilityController.enableEnhancedAccessibility(false);
>+            shouldBe("accessibilityController.enhancedAccessibilityEnabled", "false");
>+            setPreventKeyboardDOMEventDispatch(webArea, false);
>+            shouldBe("preventKeyboardDOMEventDispatch(webArea)", "false");
>+
>+            // Arrowing before enabling AX and ignore dom handlers won't move the caret
>+            shouldBe("elementAtStartMarkerOfSelectedTextMarkerRange(webArea).stringValue", "'AXValue: 1'");
>+            eventSender.keyDown("rightArrow");
>+            shouldBe("elementAtStartMarkerOfSelectedTextMarkerRange(webArea).stringValue", "'AXValue: 1'");
>+            eventSender.keyDown("rightArrow");
>+            shouldBe("elementAtStartMarkerOfSelectedTextMarkerRange(webArea).stringValue", "'AXValue: 1'");
>+
>+            // Validate that the handler received 2 keys events
>+            shouldBe("keycount()", "'2'");
>+
>+            // Enable enhanced accessibility (necessary for accessibility specific selection handling).
>+            accessibilityController.enableEnhancedAccessibility(true);
>+            shouldBe("accessibilityController.enhancedAccessibilityEnabled", "true");
>+
>+            // Enable IgnoreDOMKeyEventHandlers so that the javascript handler will be skipped
>+            setPreventKeyboardDOMEventDispatch(webArea, true);
>+            shouldBe("preventKeyboardDOMEventDispatch(webArea)", "true");
>+
>+            shouldBe("elementAtStartMarkerOfSelectedTextMarkerRange(webArea).stringValue", "'AXValue: 1'");
>+            eventSender.keyDown("rightArrow");
>+            shouldBe("elementAtStartMarkerOfSelectedTextMarkerRange(webArea).stringValue", "'AXValue: 1'");
>+            eventSender.keyDown("rightArrow");
>+            shouldBe("elementAtStartMarkerOfSelectedTextMarkerRange(webArea).stringValue", "'AXValue: 2'");
>+
>+            // Validate that the handler didn't receive any more key events
>+            shouldBe("keycount()", "'2'");
>+
>+            setPreventKeyboardDOMEventDispatch(webArea, false);
>+        }
>+    }
>+    </script>
>+    <script src="../../../resources/js-test-post.js"></script>
>+</body>
>+</html>

[WebKit Commit Bot]

Comment on attachment 248576 [details]
Update for UNUSED_PARAM(event);

Clearing flags on attachment: 248576

Committed r181484: <http://trac.webkit.org/changeset/181484>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.