Bug 180898 – AX: AOM: Dispatch accessibleclick event

• Status: RESOLVED
• Resolution: WONTFIX
• Component: Accessibility

[Nan Wang]

Phase 2 of AOM:
Add support for custom ax events. And implement onaccessibleclick

[Radar WebKit Bug Importer]

<rdar://problem/36086710>

[Nan Wang]

Created attachment 329552 [details]
patch

[chris fleizach]

Comment on attachment 329552 [details]
patch

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

> Source/WebCore/accessibility/AccessibilityObject.cpp:2247
> +    for (size_t i = 1; i < eventPath.size(); ++i) {

should take eventPath.size() out of the loop

[Nan Wang]

Created attachment 333830 [details]
patch

[EWS Watchlist]

Comment on attachment 333830 [details]
patch

Attachment 333830 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/6505405

New failing tests:
js/dom/dom-static-property-for-in-iteration.html

[EWS Watchlist]

Created attachment 333837 [details]
Archive of layout-test-results from ews106 for mac-sierra-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews106  Port: mac-sierra-wk2  Platform: Mac OS X 10.12.6

[EWS Watchlist]

Comment on attachment 333830 [details]
patch

Attachment 333830 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/6505138

New failing tests:
js/dom/dom-static-property-for-in-iteration.html

[EWS Watchlist]

Created attachment 333838 [details]
Archive of layout-test-results from ews116 for mac-sierra

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews116  Port: mac-sierra  Platform: Mac OS X 10.12.6

[Nan Wang]

Created attachment 333841 [details]
patch

fixed test

[chris fleizach]

Comment on attachment 333830 [details]
patch

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

> Source/WebCore/accessibility/AccessibilityObject.cpp:2166
> +    for (auto* parentObject = this; parentObject; parentObject = parentObject->parentObject()) {

should we follow the Element->parentNode() hierarchy here instead of ax object hierarchy?

[Nan Wang]

Comment on attachment 333830 [details]
patch

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

>> Source/WebCore/accessibility/AccessibilityObject.cpp:2166
>> +    for (auto* parentObject = this; parentObject; parentObject = parentObject->parentObject()) {
> 
> should we follow the Element->parentNode() hierarchy here instead of ax object hierarchy?

This is specified in the previous spec, and I think this would keep the same:
Accessibility input events go through capture and bubble phases, just like DOM events. The only difference is that the capture and bubble phases happen entirely in the accessibility tree.

[Ryosuke Niwa]

Comment on attachment 333841 [details]
patch

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

> Source/WebCore/ChangeLog:14
> +

Need a URL for the spec.

> Source/WebCore/accessibility/AccessibilityObject.cpp:2163
> +bool AccessibilityObject::dispatchAccessibilityEvent(Event& event)

I don't think we want to duplicate all this code here.
We should templatize the one in EventDispatcher.cpp and re-use it for regular DOM events and accessibility tree instead.
r- due to this code duplication.

> Source/WebCore/accessibility/AccessibilityObject.cpp:2166
> +    for (auto* parentObject = this; parentObject; parentObject = parentObject->parentObject()) {

Where this event path specified?
Are we sure we want to use the accessibility tree to dispatch events?

[Nan Wang]

Comment on attachment 333841 [details]
patch

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

>> Source/WebCore/accessibility/AccessibilityObject.cpp:2166
>> +    for (auto* parentObject = this; parentObject; parentObject = parentObject->parentObject()) {
> 
> Where this event path specified?
> Are we sure we want to use the accessibility tree to dispatch events?

It's specified here:
https://wicg.github.io/aom/spec/phase2.html

[Nan Wang]

Created attachment 334221 [details]
patch

update from review

[Ryosuke Niwa]

Comment on attachment 334221 [details]
patch

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

> Source/WebCore/accessibility/AccessibilityObject.cpp:999
> +            Ref<Event> event = Event::create(eventNames().accessibleclickEvent, true, true);

Use auto.

> Source/WebCore/accessibility/AccessibilityObject.cpp:2164
> +    EventDispatcher::dispatchEvent(eventPath, event);

The definition of the event dispatching isn't detailed enough in the current spec...
Filed https://github.com/WICG/aom/issues/103.

[EWS Watchlist]

Comment on attachment 334221 [details]
patch

Attachment 334221 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/6582409

New failing tests:
media/media-controls-accessibility.html

[EWS Watchlist]

Created attachment 334229 [details]
Archive of layout-test-results from ews113 for mac-sierra

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews113  Port: mac-sierra  Platform: Mac OS X 10.12.6

[Nan Wang]

Created attachment 334282 [details]
patch for landing

fixed the test

[EWS Watchlist]

Attachment 334282 [details] did not pass style-queue:


ERROR: Source/WebCore/dom/EventPath.cpp:277:  Declaration has space between type name and * in Node *origin  [whitespace/declaration] [3]
Total errors found: 1 in 19 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Nan Wang]

Created attachment 334284 [details]
patch for landing

fixed style

[WebKit Commit Bot]

Comment on attachment 334284 [details]
patch for landing

Clearing flags on attachment: 334284

Committed r228827: <https://trac.webkit.org/changeset/228827>

[James Craig]

These features are no longer in the specs. Closing out this bug.