Bug 180898 - AX: AOM: Dispatch accessibleclick event
Summary: AX: AOM: Dispatch accessibleclick event
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: Accessibility (show other bugs)
Version: WebKit Nightly Build
Hardware: All All
: P2 Normal
Assignee: Nobody
URL:
Keywords: InRadar
Depends on:
Blocks: 179502
  Show dependency treegraph
 
Reported: 2017-12-15 18:02 PST by Nan Wang
Modified: 2018-02-20 11:02 PST (History)
18 users (show)

See Also:


Attachments
patch (18.13 KB, patch)
2017-12-15 18:14 PST, Nan Wang
no flags Details | Formatted Diff | Diff
patch (15.97 KB, patch)
2018-02-14 12:30 PST, Nan Wang
ews: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from ews106 for mac-sierra-wk2 (3.32 MB, application/zip)
2018-02-14 14:06 PST, Build Bot
no flags Details
Archive of layout-test-results from ews116 for mac-sierra (2.91 MB, application/zip)
2018-02-14 14:07 PST, Build Bot
no flags Details
patch (16.56 KB, patch)
2018-02-14 14:11 PST, Nan Wang
rniwa: review-
Details | Formatted Diff | Diff
patch (18.47 KB, patch)
2018-02-19 18:23 PST, Nan Wang
rniwa: review+
ews: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from ews113 for mac-sierra (3.13 MB, application/zip)
2018-02-19 19:41 PST, Build Bot
no flags Details
patch for landing (18.60 KB, patch)
2018-02-20 10:11 PST, Nan Wang
no flags Details | Formatted Diff | Diff
patch for landing (18.60 KB, patch)
2018-02-20 10:17 PST, Nan Wang
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Nan Wang 2017-12-15 18:02:10 PST
Phase 2 of AOM:
Add support for custom ax events. And implement onaccessibleclick
Comment 1 Radar WebKit Bug Importer 2017-12-15 18:02:44 PST
<rdar://problem/36086710>
Comment 2 Nan Wang 2017-12-15 18:14:58 PST
Created attachment 329552 [details]
patch
Comment 3 chris fleizach 2017-12-18 18:16:31 PST
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
Comment 4 Nan Wang 2018-02-14 12:30:11 PST
Created attachment 333830 [details]
patch
Comment 5 Build Bot 2018-02-14 14:06:43 PST
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
Comment 6 Build Bot 2018-02-14 14:06:45 PST
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
Comment 7 Build Bot 2018-02-14 14:07:14 PST
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
Comment 8 Build Bot 2018-02-14 14:07:16 PST
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
Comment 9 Nan Wang 2018-02-14 14:11:40 PST
Created attachment 333841 [details]
patch

fixed test
Comment 10 chris fleizach 2018-02-14 15:36:45 PST
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?
Comment 11 Nan Wang 2018-02-14 15:59:05 PST
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.
Comment 12 Ryosuke Niwa 2018-02-16 20:15:28 PST
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?
Comment 13 Nan Wang 2018-02-19 18:23:15 PST
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
Comment 14 Nan Wang 2018-02-19 18:23:38 PST
Created attachment 334221 [details]
patch

update from review
Comment 15 Ryosuke Niwa 2018-02-19 19:15:51 PST
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.
Comment 16 Build Bot 2018-02-19 19:41:47 PST
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
Comment 17 Build Bot 2018-02-19 19:41:49 PST
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
Comment 18 Nan Wang 2018-02-20 10:11:58 PST
Created attachment 334282 [details]
patch for landing

fixed the test
Comment 19 Build Bot 2018-02-20 10:14:37 PST
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.
Comment 20 Nan Wang 2018-02-20 10:17:59 PST
Created attachment 334284 [details]
patch for landing

fixed style
Comment 21 WebKit Commit Bot 2018-02-20 11:02:30 PST
Comment on attachment 334284 [details]
patch for landing

Clearing flags on attachment: 334284

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