| Summary: | AX: aria-selected-menu-items test is flaky in ITM mode | ||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Joshua Hoffman <jhoffman23> | ||||||||||||||
| Component: | Accessibility | Assignee: | Nobody <webkit-unassigned> | ||||||||||||||
| Status: | RESOLVED FIXED | ||||||||||||||||
| Severity: | Normal | CC: | aboxhall, andresg_22, apinheiro, cfleizach, dmazzoni, ews-watchlist, jcraig, jdiggs, samuel_white, tyler_w, webkit-bug-importer | ||||||||||||||
| Priority: | P2 | Keywords: | InRadar | ||||||||||||||
| Version: | WebKit Nightly Build | ||||||||||||||||
| Hardware: | Unspecified | ||||||||||||||||
| OS: | Unspecified | ||||||||||||||||
| Attachments: |
|
||||||||||||||||
|
Description
Joshua Hoffman
2023-08-16 11:30:34 PDT
Created attachment 467304 [details]
Patch
Created attachment 467336 [details]
Patch
(In reply to Joshua Hoffman from comment #3) > Created attachment 467336 [details] > Patch """ This change fixes aria-selected-menu-items in isolated tree mode by adding a traversal up for the newly selected child so that its ancestor also updates. """ so that it's ancestor also updates. -> so that its ancestor also updates. --- a/Source/WebCore/accessibility/AXObjectCache.cpp +++ b/Source/WebCore/accessibility/AXObjectCache.cpp + case AXMenuListItemSelected: + if (RefPtr ancestor = Accessibility::findAncestor<AccessibilityObject>(*notification.first, false, [] (const auto& object) { + return object.canHaveSelectedChildren(); + })) { I think that declaring ancestor = Accessibility::findAncestor above the if statement is more legible. --- a/LayoutTests/accessibility/aria-selected-menu-items.html +++ b/LayoutTests/accessibility/aria-selected-menu-items.html - selectedChildInfo(window.accessibilityController.accessibleElementById("menu1")); - selectedChildInfo(window.accessibilityController.accessibleElementById("menu2")); + testOutput += await selectedChildInfo(window.accessibilityController.accessibleElementById("menu1"), "item2"); + testOutput += await selectedChildInfo(window.accessibilityController.accessibleElementById("menu2"), "item5"); You can get rid of the window. in window.accessibilityController. Created attachment 467376 [details]
Patch
Comment on attachment 467376 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=467376&action=review > Source/WebCore/accessibility/AXObjectCache.cpp:4019 > + return object.canHaveSelectedChildren(); should this try to find a matching role instead of canHaveSelectedChildren? there may possibly be a mismatch in what is found > Source/WebCore/accessibility/AXObjectCache.cpp:4022 > + updateNode(ancestor); are we able to update just a single property for the ancestor? (I'm not sure what's happening in updateNode) (In reply to chris fleizach from comment #6) > Comment on attachment 467376 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=467376&action=review > > > Source/WebCore/accessibility/AXObjectCache.cpp:4019 > > + return object.canHaveSelectedChildren(); > > should this try to find a matching role instead of canHaveSelectedChildren? > there may possibly be a mismatch in what is found Definitely—we can check for menu or menubar roles instead. > > Source/WebCore/accessibility/AXObjectCache.cpp:4022 > > + updateNode(ancestor); > > are we able to update just a single property for the ancestor? (I'm not sure > what's happening in updateNode) After some more investigation we can do this via an update to AXPropertyName::SelectedChildren. Created attachment 467389 [details]
Patch
Created attachment 467400 [details]
Patch
Commit message contains (OOPS!) and no reviewer found, blocking PR #None Created attachment 467409 [details]
Patch
Committed 267221@main (5e54c0b0e654): <https://commits.webkit.org/267221@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 467409 [details]. |