Bug 260278 – AX: aria-selected-menu-items test is flaky in ITM mode

• Status: RESOLVED
• Resolution: FIXED
• Component: Accessibility

[Joshua Hoffman]

The accessibility/aria-selected-menu-items test is flaky when run in ITM mode.

[Radar WebKit Bug Importer]

<rdar://problem/113979223>

[Joshua Hoffman]

Created attachment 467304 [details]
Patch

[Joshua Hoffman]

Created attachment 467336 [details]
Patch

[Andres Gonzalez]

(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.

[Joshua Hoffman]

Created attachment 467376 [details]
Patch

[chris fleizach]

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)

[Joshua Hoffman]

(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.

[Joshua Hoffman]

Created attachment 467389 [details]
Patch

[Joshua Hoffman]

Created attachment 467400 [details]
Patch

[EWS]

Commit message contains (OOPS!) and no reviewer found, blocking PR #None

[Joshua Hoffman]

Created attachment 467409 [details]
Patch

[EWS]

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].