Bug 183023 - AX: AOM: More accessibility events support
Summary: AX: AOM: More accessibility events support
Status: RESOLVED FIXED
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:
 
Reported: 2018-02-21 15:44 PST by Nan Wang
Modified: 2018-03-09 19:37 PST (History)
14 users (show)

See Also:


Attachments
patch (17.84 KB, patch)
2018-03-05 11:57 PST, Nan Wang
no flags Details | Formatted Diff | Diff
patch (17.84 KB, patch)
2018-03-05 12:24 PST, Nan Wang
ews-watchlist: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from ews102 for mac-sierra (2.41 MB, application/zip)
2018-03-05 13:14 PST, EWS Watchlist
no flags Details
Archive of layout-test-results from ews106 for mac-sierra-wk2 (3.44 MB, application/zip)
2018-03-05 13:18 PST, EWS Watchlist
no flags Details
patch (18.77 KB, patch)
2018-03-05 13:38 PST, Nan Wang
ews-watchlist: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from ews100 for mac-sierra (2.22 MB, application/zip)
2018-03-05 14:47 PST, EWS Watchlist
no flags Details
Archive of layout-test-results from ews115 for mac-sierra (3.05 MB, application/zip)
2018-03-05 15:38 PST, EWS Watchlist
no flags Details
patch (19.41 KB, patch)
2018-03-05 16:55 PST, Nan Wang
cfleizach: review+
Details | Formatted Diff | Diff
patch (19.58 KB, patch)
2018-03-05 18:19 PST, Nan Wang
no flags Details | Formatted Diff | Diff
patch to fix crash (5.30 KB, patch)
2018-03-08 18:40 PST, Nan Wang
no flags Details | Formatted Diff | Diff
patch to fix test crash (1.43 KB, patch)
2018-03-09 12:29 PST, Nan Wang
no flags Details | Formatted Diff | Diff
patch to fix test crash (2.68 KB, patch)
2018-03-09 17:49 PST, Nan Wang
no flags Details | Formatted Diff | Diff
patch to fix test crash (2.64 KB, patch)
2018-03-09 17:59 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 2018-02-21 15:44:19 PST
Spec: https://wicg.github.io/aom/spec/phase2.html

Add support for:
accessiblecontextmenu
accessibledecrement
accessibledismiss
accessiblefocus
accessibleincrement
accessiblescrollintoview
accessibleselect
Comment 1 Radar WebKit Bug Importer 2018-02-21 15:44:38 PST
<rdar://problem/37764380>
Comment 2 Nan Wang 2018-03-05 11:57:38 PST
Created attachment 335012 [details]
patch
Comment 3 Nan Wang 2018-03-05 12:24:01 PST
Created attachment 335015 [details]
patch

This patch should work
Comment 4 EWS Watchlist 2018-03-05 13:14:53 PST
Comment on attachment 335015 [details]
patch

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

New failing tests:
accessibility/mac/AOM-events-all.html
js/dom/dom-static-property-for-in-iteration.html
Comment 5 EWS Watchlist 2018-03-05 13:14:54 PST
Created attachment 335019 [details]
Archive of layout-test-results from ews102 for mac-sierra

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews102  Port: mac-sierra  Platform: Mac OS X 10.12.6
Comment 6 EWS Watchlist 2018-03-05 13:18:14 PST
Comment on attachment 335015 [details]
patch

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

New failing tests:
js/dom/dom-static-property-for-in-iteration.html
Comment 7 EWS Watchlist 2018-03-05 13:18:16 PST
Created attachment 335020 [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 8 Nan Wang 2018-03-05 13:38:33 PST
Created attachment 335021 [details]
patch

fix the test
Comment 9 EWS Watchlist 2018-03-05 14:47:37 PST
Comment on attachment 335021 [details]
patch

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

New failing tests:
accessibility/mac/AOM-events-all.html
Comment 10 EWS Watchlist 2018-03-05 14:47:38 PST
Created attachment 335031 [details]
Archive of layout-test-results from ews100 for mac-sierra

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews100  Port: mac-sierra  Platform: Mac OS X 10.12.6
Comment 11 EWS Watchlist 2018-03-05 15:38:28 PST
Comment on attachment 335021 [details]
patch

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

New failing tests:
accessibility/mac/AOM-events-all.html
accessibility/scroll-to-make-visible-iframe.html
Comment 12 EWS Watchlist 2018-03-05 15:38:30 PST
Created attachment 335039 [details]
Archive of layout-test-results from ews115 for mac-sierra

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews115  Port: mac-sierra  Platform: Mac OS X 10.12.6
Comment 13 Nan Wang 2018-03-05 16:55:16 PST
Created attachment 335053 [details]
patch
Comment 14 chris fleizach 2018-03-05 17:08:40 PST
Comment on attachment 335053 [details]
patch

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

> Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm:3542
> +    if (m_object->dispatchAccessibilityEventWithType(AccessibilityEventType::ContextMenu))

Are we gonna implement escape action for iOS now?
Comment 15 Nan Wang 2018-03-05 17:19:08 PST
(In reply to chris fleizach from comment #14)
> Comment on attachment 335053 [details]
> patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=335053&action=review
> 
> > Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm:3542
> > +    if (m_object->dispatchAccessibilityEventWithType(AccessibilityEventType::ContextMenu))
> 
> Are we gonna implement escape action for iOS now?

I think I'll file a new bug for that.
Comment 16 Nan Wang 2018-03-05 17:22:09 PST
(In reply to Nan Wang from comment #15)
> (In reply to chris fleizach from comment #14)
> > Comment on attachment 335053 [details]
> > patch
> > 
> > View in context:
> > https://bugs.webkit.org/attachment.cgi?id=335053&action=review
> > 
> > > Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm:3542
> > > +    if (m_object->dispatchAccessibilityEventWithType(AccessibilityEventType::ContextMenu))
> > 
> > Are we gonna implement escape action for iOS now?
> 
> I think I'll file a new bug for that.

filed:
https://bugs.webkit.org/show_bug.cgi?id=183352
Comment 17 Nan Wang 2018-03-05 18:19:15 PST
Created attachment 335064 [details]
patch

Fixed a crashing test
Comment 18 WebKit Commit Bot 2018-03-06 00:19:16 PST
Comment on attachment 335064 [details]
patch

Clearing flags on attachment: 335064

Committed r229310: <https://trac.webkit.org/changeset/229310>
Comment 19 WebKit Commit Bot 2018-03-06 00:19:18 PST
All reviewed patches have been landed.  Closing bug.
Comment 21 Nan Wang 2018-03-06 10:23:57 PST
(In reply to Ryan Haddad from comment #20)
> LayoutTest accessibility/mac/AOM-events.html is consistently crashing on
> High Sierra and timing out on Sierra after this change:
> 
> https://build.webkit.org/results/
> Apple%20High%20Sierra%20Release%20WK2%20(Tests)/r229323%20(3270)/results.html
> 
> https://webkit-test-results.webkit.org/dashboards/flakiness_dashboard.
> html#showAllRuns=true&tests=accessibility%2Fmac%2FAOM-events.html

I'm going to look at this soon
Comment 22 Nan Wang 2018-03-06 11:48:35 PST
Going to fix the test in https://bugs.webkit.org/show_bug.cgi?id=183376
Comment 23 Nan Wang 2018-03-08 18:36:30 PST
Reopen this since it's causing a crash

>  1 com.apple.WebCore              0x00d6b0ab WebCore::EventDispatcher::dispatchEvent(WTF::Vector<WebCore::Element*, 0ul, WTF::CrashOnOverflow, 16ul, WTF::FastMalloc> const&, WebCore::Event&) + 43
   2 com.apple.WebCore              0x00add5eb WebCore::AccessibilityObject::dispatchAccessibilityEvent(WebCore::Event&) const + 651
   3 com.apple.WebCore              0x00acec32 WebCore::AccessibilityObject::dispatchAccessibilityEventWithType(WebCore::AccessibilityEventType) const + 290
   4 com.apple.WebCore              0x00ae7ac3 WebCore::AccessibilityRenderObject::setFocused(bool) + 35
   5 libdispatch.dylib              0x0000848b _dispatch_call_block_and_release + 12
   6 libdispatch.dylib              0x0000185f _dispatch_client_callout + 8
   7 libdispatch.dylib              0x0000c6ae _dispatch_main_queue_callback_4CF + 1199
   8 com.apple.CoreFoundation       0x000c2959 __CFRUNLOOP_IS_SERVICING_THE_MAIN_DISPATCH_QUEUE__ + 9 (/BuildRoot/Library/Caches/com.apple.xbs/Sources/CoreFoundation/Foundation-1524.10/CoreFoundation/RunLoop.subproj/CFRunLoop.c:1815)
   9 com.apple.CoreFoundation       0x000845dc __CFRunLoopRun + 2556 (/BuildRoot/Library/Caches/com.apple.xbs/Sources/CoreFoundation/Foundation-1524.10/CoreFoundation/RunLoop.subproj/CFRunLoop.c:3115)
  10 com.apple.CoreFoundation       0x00083940 CFRunLoopRunSpecific + 736 (/BuildRoot/Library/Caches/com.apple.xbs/Sources/CoreFoundation/Foundation-1524.10/CoreFoundation/RunLoop.subproj/CFRunLoop.c:3249)
  11 com.apple.Foundation           0x000215ea -[NSRunLoop(NSRunLoop) runMode:beforeDate:] + 277 (/BuildRoot/Library/Caches/com.apple.xbs/Sources/Foundation/Foundation-1524.10/Foundation/Soil.subproj/NSRunLoop.m:367)
  12 com.apple.Foundation           0x000214c2 -[NSRunLoop(NSRunLoop) run] + 76 (/BuildRoot/Library/Caches/com.apple.xbs/Sources/Foundation/Foundation-1524.10/Foundation/Soil.subproj/NSRunLoop.m:389)
  13 libxpc.dylib                   0x0001068d _xpc_objc_main + 547
  14 libxpc.dylib                   0x0000f306 xpc_main + 417
  15 com.apple.WebKit.WebContent    0x000016b5  + 0
  16 libdyld.dylib                  0x00000f09 start + 1
Comment 24 Nan Wang 2018-03-08 18:40:21 PST
Created attachment 335381 [details]
patch to fix crash
Comment 25 WebKit Commit Bot 2018-03-08 23:35:57 PST
Comment on attachment 335381 [details]
patch to fix crash

Clearing flags on attachment: 335381

Committed r229452: <https://trac.webkit.org/changeset/229452>
Comment 26 WebKit Commit Bot 2018-03-08 23:35:59 PST
All reviewed patches have been landed.  Closing bug.
Comment 27 Ryan Haddad 2018-03-09 09:42:38 PST
(In reply to WebKit Commit Bot from comment #25)
> Comment on attachment 335381 [details]
> patch to fix crash
> 
> Clearing flags on attachment: 335381
> 
> Committed r229452: <https://trac.webkit.org/changeset/229452>

The test added with this change seems to be a flaky crash. In this test run, the test results blame accessibility/mac/AOM-events.html, but the crashlog blames ccessibility/mac/AOM-events-webarea-crash.html:
https://build.webkit.org/results/Apple%20High%20Sierra%20Release%20WK2%20(Tests)/r229462%20(3354)/results.html

Thread 0 Crashed:: Dispatch queue: com.apple.main-thread
0   com.apple.WebCore             	0x00000004a88ac8a4 WTF::KeyValuePair<WebCore::RenderObject*, unsigned int>* WTF::HashTable<WebCore::RenderObject*, WTF::KeyValuePair<WebCore::RenderObject*, unsigned int>, WTF::KeyValuePairKeyExtractor<WTF::KeyValuePair<WebCore::RenderObject*, unsigned int> >, WTF::PtrHash<WebCore::RenderObject*>, WTF::HashMap<WebCore::RenderObject*, unsigned int, WTF::PtrHash<WebCore::RenderObject*>, WTF::HashTraits<WebCore::RenderObject*>, WTF::HashTraits<unsigned int> >::KeyValuePairTraits, WTF::HashTraits<WebCore::RenderObject*> >::lookup<WTF::HashMapTranslatorAdapter<WTF::HashMap<WebCore::RenderObject*, unsigned int, WTF::PtrHash<WebCore::RenderObject*>, WTF::HashTraits<WebCore::RenderObject*>, WTF::HashTraits<unsigned int> >::KeyValuePairTraits, WTF::IdentityHashTranslator<WTF::HashMap<WebCore::RenderObject*, unsigned int, WTF::PtrHash<WebCore::RenderObject*>, WTF::HashTraits<WebCore::RenderObject*>, WTF::HashTraits<unsigned int> >::KeyValuePairTraits, WTF::PtrHash<WebCore::RenderObject*> > >, WebCore::RenderObject*>(WebCore::RenderObject* const&) + 4 (HashTable.h:601)
1   com.apple.WebCore             	0x00000004a889d8c3 WebCore::AXObjectCache::get(WebCore::RenderObject*) + 35 (AXObjectCache.cpp:386)
2   com.apple.WebCore             	0x00000004a889dfdf WebCore::AXObjectCache::getOrCreate(WebCore::RenderObject*) + 47 (AXObjectCache.cpp:621)
3   com.apple.WebCore             	0x00000004a88d6380 WebCore::AccessibilityRenderObject::addChildren() + 48 (AccessibilityRenderObject.cpp:3262)
4   com.apple.WebCore             	0x00000004a88d5d3e WebCore::AccessibilityRenderObject::updateChildrenIfNecessary() + 94 (AccessibilityRenderObject.cpp:3044)
5   com.apple.WebCore             	0x00000004a88c0401 WebCore::AccessibilityObject::updateBackingStore() + 97 (RefCounted.h:98)
6   com.apple.WebCore             	0x00000004a94f73fd -[WebAccessibilityObjectWrapperBase updateObjectBackingStore] + 61 (WebAccessibilityObjectWrapperBase.mm:294)
7   com.apple.WebCore             	0x00000004a9509377 -[WebAccessibilityObjectWrapper _accessibilitySetValue:forAttribute:] + 39 (WebAccessibilityObjectWrapperMac.mm:3620)
8   libdispatch.dylib             	0x00007fff76964591 _dispatch_call_block_and_release + 12
9   libdispatch.dylib             	0x00007fff7695cd50 _dispatch_client_callout + 8
10  libdispatch.dylib             	0x00007fff7696832d _dispatch_main_queue_callback_4CF + 1148
11  com.apple.CoreFoundation      	0x00007fff4f02f839 __CFRUNLOOP_IS_SERVICING_THE_MAIN_DISPATCH_QUEUE__ + 9
12  com.apple.CoreFoundation      	0x00007fff4eff208a __CFRunLoopRun + 2586
13  com.apple.CoreFoundation      	0x00007fff4eff13d7 CFRunLoopRunSpecific + 487
14  com.apple.HIToolbox           	0x00007fff4e2fee26 RunCurrentEventLoopInMode + 286
15  com.apple.HIToolbox           	0x00007fff4e2feb96 ReceiveNextEventCommon + 613
16  com.apple.HIToolbox           	0x00007fff4e2fe914 _BlockUntilNextEventMatchingListInModeWithFilter + 64
17  com.apple.AppKit              	0x00007fff4c5c9f5f _DPSNextEvent + 2085
18  com.apple.AppKit              	0x00007fff4cd5fb4c -[NSApplication(NSEvent) _nextEventMatchingEventMask:untilDate:inMode:dequeue:] + 3044
19  com.apple.AppKit              	0x00007fff4c5bed6d -[NSApplication run] + 764
20  com.apple.AppKit              	0x00007fff4c58df1a NSApplicationMain + 804
21  libxpc.dylib                  	0x00007fff76c6342f _xpc_objc_main + 580
22  libxpc.dylib                  	0x00007fff76c62082 xpc_main + 417
23  com.apple.WebKit.WebContent   	0x000000010eba26a1 main + 490
24  libdyld.dylib                 	0x00007fff76996115 start + 1
Comment 28 Nan Wang 2018-03-09 12:20:23 PST
Ok I think the problem is that takeFocus() is async and the webpage closes before it finishes calling updateObjectBackingStore
Comment 29 Nan Wang 2018-03-09 12:29:41 PST
Created attachment 335449 [details]
patch to fix test crash

Fixed the test crash
Comment 30 chris fleizach 2018-03-09 13:20:52 PST
Comment on attachment 335449 [details]
patch to fix test crash

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

> LayoutTests/ChangeLog:9
> +        The test is crashing during updateObjectBackingStore. The problem

is it a crash or an assert
Comment 31 Nan Wang 2018-03-09 13:40:17 PST
Comment on attachment 335449 [details]
patch to fix test crash

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

>> LayoutTests/ChangeLog:9
>> +        The test is crashing during updateObjectBackingStore. The problem
> 
> is it a crash or an assert

It's a crash during hash table lookup
Comment 32 chris fleizach 2018-03-09 14:08:41 PST
seems like we should have an assert so we don't get into that state. don't think we should hit a crash for those cases.
Comment 33 Nan Wang 2018-03-09 16:10:42 PST
(In reply to chris fleizach from comment #32)
> seems like we should have an assert so we don't get into that state. don't
> think we should hit a crash for those cases.

I can reproduce this crash on a release build. And this patch fixed the crash.
Comment 34 chris fleizach 2018-03-09 16:11:31 PST
(In reply to Nan Wang from comment #33)
> (In reply to chris fleizach from comment #32)
> > seems like we should have an assert so we don't get into that state. don't
> > think we should hit a crash for those cases.
> 
> I can reproduce this crash on a release build. And this patch fixed the
> crash.

I understand that the test change can fix the patch, but I'm more interesting in fixing the actual crash
Comment 35 Nan Wang 2018-03-09 16:12:42 PST
(In reply to chris fleizach from comment #34)
> (In reply to Nan Wang from comment #33)
> > (In reply to chris fleizach from comment #32)
> > > seems like we should have an assert so we don't get into that state. don't
> > > think we should hit a crash for those cases.
> > 
> > I can reproduce this crash on a release build. And this patch fixed the
> > crash.
> 
> I understand that the test change can fix the patch, but I'm more
> interesting in fixing the actual crash

Ok I can take some further look at it
Comment 36 Nan Wang 2018-03-09 17:49:14 PST
Created attachment 335491 [details]
patch to fix test crash
Comment 37 chris fleizach 2018-03-09 17:54:33 PST
Comment on attachment 335491 [details]
patch to fix test crash

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

> Source/WebCore/ChangeLog:9
> +        The test is crashing that we call updateBackingStore when 

test is crashing "when" we

> Source/WebCore/ChangeLog:12
> +        Modified the test by using the right format of setTimtout and extended the delay.

setTimeout

> Source/WebCore/accessibility/AccessibilityObject.cpp:1780
> +    AXObjectCache* cache = this->axObjectCache();

if (!axObjectCache())
   return;
Comment 38 Nan Wang 2018-03-09 17:59:20 PST
Created attachment 335492 [details]
patch to fix test crash

update from review
Comment 39 WebKit Commit Bot 2018-03-09 19:37:18 PST
Comment on attachment 335492 [details]
patch to fix test crash

Clearing flags on attachment: 335492

Committed r229499: <https://trac.webkit.org/changeset/229499>
Comment 40 WebKit Commit Bot 2018-03-09 19:37:20 PST
All reviewed patches have been landed.  Closing bug.