Bug 213875 – AX: Implement user action spec for Escape action

• Component: Accessibility
• Status: RESOLVED FIXED

[chris fleizach]

User action events from Assistive Technology

To preserve the privacy of assistive technology users, events from assistive technology will typically cause a synthesised DOM event to be triggered. The events are determined by platform conventions and partially documented in the ARIA Authoring Practices Guide (APG).

dismiss or escape	all elements		Escape KeyboardEvent

https://github.com/WICG/aom/blob/gh-pages/explainer.md#user-action-events-from-assistive-technology

[Radar WebKit Bug Importer]

<rdar://problem/65022980>

[chris fleizach]

Created attachment 403350 [details]
Patch

[chris fleizach]

Created attachment 403352 [details]
patch

[chris fleizach]

Created attachment 403373 [details]
patch

[chris fleizach]

Created attachment 403393 [details]
patch

[chris fleizach]

Created attachment 403402 [details]
patch

[chris fleizach]

Created attachment 403442 [details]
patch

[Darin Adler]

Comment on attachment 403442 [details]
patch

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

> Source/WebCore/accessibility/AccessibilityNodeObject.cpp:1100
> +    // In case the keyboard event causes this element to be removed.
> +    Ref<AccessibilityObject> protectedThis(*this);

This should be moved int dispatchSimulatedKeyboardUpDownEvent; that’s where the protection is needed, not here.

> Source/WebCore/accessibility/AccessibilityNodeObject.cpp:1110
> +bool AccessibilityNodeObject::dispatchSimulatedKeyboardUpDownEvent(KeyboardEvent::Init& keyInit)

Should take a const&

> Source/WebCore/accessibility/AccessibilityNodeObject.cpp:1132
>      Ref<AccessibilityObject> protectedThis(*this);

Can move this into dispatchSimulatedKeyboardUpDownEvent and remove it here.

> Source/WebCore/accessibility/AccessibilityNodeObject.h:32
> +#include "KeyboardEvent.h"

This dependency is really unfortunate, but I suppose there’s no way to forward declare something from inside a base class?

> Source/WebCore/accessibility/AccessibilityNodeObject.h:195
> +    bool performEscape() override;

final instead of override

> Source/WebCore/accessibility/AccessibilityNodeObject.h:196
> +    bool dispatchSimulatedKeyboardUpDownEvent(KeyboardEvent::Init&);

Argument should be const&, not non-const&.

> Source/WebCore/accessibility/AccessibilityObject.h:481
> +    bool performEscape() override { return false; }

final instead of override

> Source/WebCore/accessibility/AccessibilityObjectInterface.h:910
> +    virtual bool performEscape() = 0;

Seems like more of this should be named performDismissAction, and not use the word "escape".

[chris fleizach]

(In reply to Darin Adler from comment #8)
> Comment on attachment 403442 [details]
> patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=403442&action=review
> 
> > Source/WebCore/accessibility/AccessibilityNodeObject.cpp:1100
> > +    // In case the keyboard event causes this element to be removed.
> > +    Ref<AccessibilityObject> protectedThis(*this);
> 
> This should be moved int dispatchSimulatedKeyboardUpDownEvent; that’s where
> the protection is needed, not here.
> 
> > Source/WebCore/accessibility/AccessibilityNodeObject.cpp:1110
> > +bool AccessibilityNodeObject::dispatchSimulatedKeyboardUpDownEvent(KeyboardEvent::Init& keyInit)
> 
> Should take a const&
> 
> > Source/WebCore/accessibility/AccessibilityNodeObject.cpp:1132
> >      Ref<AccessibilityObject> protectedThis(*this);
> 
> Can move this into dispatchSimulatedKeyboardUpDownEvent and remove it here.
> 
> > Source/WebCore/accessibility/AccessibilityNodeObject.h:32
> > +#include "KeyboardEvent.h"

I think I can make this a static function inside the .cpp file so it doesn't have to be in the header

> 
> This dependency is really unfortunate, but I suppose there’s no way to
> forward declare something from inside a base class?
> 
> > Source/WebCore/accessibility/AccessibilityNodeObject.h:195
> > +    bool performEscape() override;
> 
> final instead of override
> 
> > Source/WebCore/accessibility/AccessibilityNodeObject.h:196
> > +    bool dispatchSimulatedKeyboardUpDownEvent(KeyboardEvent::Init&);
> 
> Argument should be const&, not non-const&.
> 
> > Source/WebCore/accessibility/AccessibilityObject.h:481
> > +    bool performEscape() override { return false; }
> 
> final instead of override
> 
> > Source/WebCore/accessibility/AccessibilityObjectInterface.h:910
> > +    virtual bool performEscape() = 0;
> 
> Seems like more of this should be named performDismissAction, and not use
> the word "escape".

[chris fleizach]

Created attachment 403609 [details]
patch

[chris fleizach]

Created attachment 403610 [details]
patch

[Darin Adler]

Comment on attachment 403610 [details]
patch

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

> Source/WebCore/accessibility/AccessibilityObjectInterface.h:910
> +    virtual bool performDismissAction() { return false; };

No need for the trailing semicolon here.

[chris fleizach]

Created attachment 403615 [details]
patch

thanks!

[chris fleizach]

Created attachment 403623 [details]
Patch

[chris fleizach]

Created attachment 403625 [details]
patch

[EWS]

Committed r264000: <https://trac.webkit.org/changeset/264000>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 403625 [details].