Skip to content

Commit

Permalink
AX: aria-selected-menu-items test is flaky in ITM mode
Browse files Browse the repository at this point in the history
https://bugs.webkit.org/show_bug.cgi?id=260278
rdar://113979223

Reviewed by Chris Fleizach.

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 its SelectedChildren property.
The traversal happens during a new case in AXObjectCache::updateIsolatedTree where the notification AXMenuListItemSelected is now handled.

* LayoutTests/accessibility-isolated-tree/TestExpectations

* LayoutTests/accessibility/aria-selected-menu-items.html
* LayoutTests/platform/mac/accessibility/aria-selected-menu-items-expected.txt
* LayoutTests/platform/glib/accessibility/aria-selected-menu-items-expected.txt

Tests updated to be async for ITM.

* Source/WebCore/accessibility/AXObjectCache.cpp
(WebCore::AXObjectCache::updateIsolatedTree):

Logic to handle AXMenuListItemSelected notifications.

* Source/WebCore/accessibility/isolatedtree/AXIsolatedTree.cpp

Handle AXPropertyName::SelectedChildren in updateNodeProperties.

Canonical link: https://commits.webkit.org/267221@main
  • Loading branch information
hoffmanjoshua authored and twilco committed Aug 24, 2023
1 parent 51e8e5e commit 5e54c0b
Show file tree
Hide file tree
Showing 6 changed files with 29 additions and 20 deletions.
1 change: 0 additions & 1 deletion LayoutTests/accessibility-isolated-tree/TestExpectations
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,5 @@ accessibility/mac/grid-selected-cells.html [ Failure ]
accessibility/table-cells.html [ Failure ]
accessibility/mac/document-links.html [ Failure ]

accessibility/aria-selected-menu-items.html [ Failure ]
accessibility/aria-owns-hierarchy-remap.html [ Failure ]
accessibility/element-reflection-ariaactivedescendant.html [ Failure ]
25 changes: 14 additions & 11 deletions LayoutTests/accessibility/aria-selected-menu-items.html
Original file line number Diff line number Diff line change
Expand Up @@ -30,37 +30,40 @@
</div>

<script>
description("This tests that focused/active menu items are reported as selected children of the parent container.");
testOutput = "This tests that focused/active menu items are reported as selected children of the parent container.\n";

if (window.accessibilityController) {
window.jsTestIsAsync = true;

document.getElementById("item2").focus();
setTimeout(async () => {
await focus("item2");
selectedChildInfo(window.accessibilityController.accessibleElementById("menu1"));
selectedChildInfo(window.accessibilityController.accessibleElementById("menu2"));
testOutput += await selectedChildInfo(accessibilityController.accessibleElementById("menu1"), "item2");
testOutput += await selectedChildInfo(accessibilityController.accessibleElementById("menu2"), "item5");
await focus("item8");
selectedChildInfo(window.accessibilityController.accessibleElementById("menubar1"));
selectedChildInfo(window.accessibilityController.accessibleElementById("menubar2"));
testOutput += await selectedChildInfo(accessibilityController.accessibleElementById("menubar1"), "item8");
testOutput += await selectedChildInfo(accessibilityController.accessibleElementById("menubar2"), "item12");
document.getElementById("content").style.visibility = "hidden";

debug(testOutput);
finishJSTest();
}, 0);
}

function selectedChildInfo(axElement) {
async function selectedChildInfo(axElement, expectedChildId) {
result = ""
if (!axElement)
debug("Element not exposed");
result += "Element not exposed";

var count = axElement.selectedChildrenCount;
debug(platformValueForW3CName(axElement) + " has " + count + " selected child(ren)");
result += `${platformValueForW3CName(axElement)} has ${count} selected child(ren)\n`;
for (var i = 0; i < count; i++) {
await waitFor(() => axElement.selectedChildAtIndex(i).domIdentifier == expectedChildId)
var child = axElement.selectedChildAtIndex(i);
result = "\t" + platformValueForW3CName(child) + " (" + child.role + ")";
result += " isSelectable: " + child.isSelectable + " isSelected: " + child.isSelected;
debug(result);
result += `\t${platformValueForW3CName(child)} (${child.role})`;
result += ` isSelectable: ${child.isSelectable} isSelected: ${child.isSelected}\n`;
}
return result;
}

async function focus(id) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,4 @@
This tests that focused/active menu items are reported as selected children of the parent container.

On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".


First menu has 1 selected child(ren)
item 2 (AXRole: AXMenuItem) isSelectable: true isSelected: true
Second menu has 1 selected child(ren)
Expand All @@ -11,6 +7,7 @@ First menubar has 1 selected child(ren)
Edit (AXRole: AXMenuItem) isSelectable: true isSelected: true
Second menubar has 1 selected child(ren)
View (AXRole: AXMenuItem) isSelectable: true isSelected: true

PASS successfullyParsed is true

TEST COMPLETE
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,4 @@
This tests that focused/active menu items are reported as selected children of the parent container.

On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".


First menu has 1 selected child(ren)
item 2 (AXRole: AXMenuItem) isSelectable: true isSelected: true
Second menu has 1 selected child(ren)
Expand All @@ -11,6 +7,7 @@ First menubar has 1 selected child(ren)
Edit (AXRole: AXMenuItem) isSelectable: true isSelected: true
Second menubar has 1 selected child(ren)
View (AXRole: AXMenuItem) isSelectable: true isSelected: true

PASS successfullyParsed is true

TEST COMPLETE
Expand Down
10 changes: 10 additions & 0 deletions Source/WebCore/accessibility/AXObjectCache.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4031,6 +4031,16 @@ void AXObjectCache::updateIsolatedTree(const Vector<std::pair<RefPtr<Accessibili
case AXMaximumValueChanged:
tree->updateNodeProperties(*notification.first, { AXPropertyName::MaxValueForRange, AXPropertyName::ValueForRange });
break;
case AXMenuListItemSelected: {
RefPtr ancestor = Accessibility::findAncestor<AccessibilityObject>(*notification.first, false, [] (const auto& object) {
return object.isMenu() || object.isMenuBar();
});
if (ancestor) {
tree->updateNodeProperty(*ancestor, AXPropertyName::SelectedChildren);
tree->updateNodeProperty(*notification.first, AXPropertyName::IsSelected);
}
break;
}
case AXMinimumValueChanged:
tree->updateNodeProperties(*notification.first, { AXPropertyName::MinValueForRange, AXPropertyName::ValueForRange });
break;
Expand Down
3 changes: 3 additions & 0 deletions Source/WebCore/accessibility/isolatedtree/AXIsolatedTree.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -574,6 +574,9 @@ void AXIsolatedTree::updateNodeProperties(AXCoreObject& axObject, const Vector<A
propertyMap.set(AXPropertyName::SupportsKeyShortcuts, axObject.supportsKeyShortcuts());
propertyMap.set(AXPropertyName::KeyShortcuts, axObject.keyShortcuts().isolatedCopy());
break;
case AXPropertyName::SelectedChildren:
propertyMap.set(AXPropertyName::SelectedChildren, axIDs(axObject.selectedChildren()));
break;
case AXPropertyName::SupportsPosInSet:
propertyMap.set(AXPropertyName::SupportsPosInSet, axObject.supportsPosInSet());
break;
Expand Down

0 comments on commit 5e54c0b

Please sign in to comment.