Skip to content

Commit cab0b7e

Browse files
twilcotwilco
committed
AX: VoiceOver isn't able to access aria-owned rows and their cells in grids / tables
https://bugs.webkit.org/show_bug.cgi?id=306130 rdar://168770938 Reviewed by Joshua Hoffman. This commit updates AccessibilityNodeObject::computeCellSlots() to look for and handle aria-owned elements of the table we are computing cell slots for, mostly fixing the bug. Even after this though, I noticed VoiceOver still wouldn't navigate into the table testcases in new test aria-owns-grid-parts.html. The reason for that was because we were returning an empty frame for the grid, and VoiceOver generally ignores things with an empty frame. This makes sense — the isolated tree gets geometry from paint, and these grids have no actual DOM children, just aria-owns children, so they don't have any meaningful geometry from non-ARIA means. Fix this by adding more robust frame fallbacks in AXIsolatedObject::relativeFrame. The tests verify we don't return an empty frame for any of these grids. * LayoutTests/accessibility/aria-owns-grid-parts-expected.txt: Added. * LayoutTests/accessibility/aria-owns-grid-parts.html: Added. * Source/WebCore/accessibility/AccessibilityNodeObject.cpp: (WebCore::AccessibilityNodeObject::computeCellSlots): * Source/WebCore/accessibility/isolatedtree/AXIsolatedObject.cpp: (WebCore::AXIsolatedObject::relativeFrame const): Canonical link: https://commits.webkit.org/308087@main
1 parent fa3c94e commit cab0b7e

File tree

4 files changed

+194
-2
lines changed

4 files changed

+194
-2
lines changed

‎LayoutTests/accessibility/aria-owns-grid-parts-expected.txt‎

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
This tests that ARIA grids correctly recognize rows and cells via aria-owns, including after dynamic attribute changes.
2+
3+
Testing grid with aria-owned rows:
4+
PASS: ownedRowsGrid.rowCount === 2
5+
PASS: ownedRowsGrid.columnCount === 2
6+
PASS: ownedRowsGrid.width >= 2 === true
7+
PASS: ownedRowsGrid.height >= 2 === true
8+
PASS: ownedRowsGrid.cellForColumnAndRow(0, 0).domIdentifier === 'owned-row-cell-1-1'
9+
PASS: ownedRowsGrid.cellForColumnAndRow(1, 1).domIdentifier === 'owned-row-cell-2-2'
10+
11+
Testing grid with aria-owned cells:
12+
PASS: ownedCellsGrid.rowCount === 1
13+
PASS: ownedCellsGrid.columnCount === 2
14+
PASS: ownedCellsGrid.width >= 2 === true
15+
PASS: ownedCellsGrid.height >= 2 === true
16+
PASS: ownedCellsGrid.cellForColumnAndRow(0, 0).domIdentifier === 'owned-cell-1'
17+
PASS: ownedCellsGrid.cellForColumnAndRow(1, 0).domIdentifier === 'owned-cell-2'
18+
19+
Testing grid with DOM children that are also aria-owned (ordering test):
20+
PASS: domAndOwnedGrid.rowCount === 3
21+
PASS: domAndOwnedGrid.width >= 2 === true
22+
PASS: domAndOwnedGrid.height >= 2 === true
23+
PASS: domAndOwnedGrid.cellForColumnAndRow(0, 0).domIdentifier === 'dom-row-C-cell'
24+
PASS: domAndOwnedGrid.cellForColumnAndRow(0, 1).domIdentifier === 'dom-row-B-cell'
25+
PASS: domAndOwnedGrid.cellForColumnAndRow(0, 2).domIdentifier === 'dom-row-A-cell'
26+
27+
Adding row-3 to aria-owns...
28+
PASS: ownedRowsGrid.rowCount === 3
29+
PASS: ownedRowsGrid.cellForColumnAndRow(0, 2).domIdentifier === 'owned-row-cell-3-1'
30+
31+
Adding owned-cell-3 to row...
32+
PASS: ownedCellsGrid.cellForColumnAndRow(2, 0).domIdentifier === 'owned-cell-3'
33+
34+
Removing owned-row-1 from aria-owns...
35+
PASS: ownedRowsGrid.rowCount === 2
36+
PASS: ownedRowsGrid.cellForColumnAndRow(0, 0).domIdentifier === 'owned-row-cell-2-1'
37+
38+
PASS successfullyParsed is true
39+
40+
TEST COMPLETE
41+
Owned Row 1 Cell 1
42+
Owned Row 1 Cell 2
43+
Owned Row 2 Cell 1
44+
Owned Row 2 Cell 2
45+
Owned Row 3 Cell 1
46+
Owned Row 3 Cell 2
47+
Owned Cell 1
48+
Owned Cell 2
49+
Owned Cell 3
50+
Row A Cell
51+
Row B Cell
52+
Row C Cell

‎LayoutTests/accessibility/aria-owns-grid-parts.html‎

Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,108 @@
1+
<!DOCTYPE html>
2+
<html>
3+
<head>
4+
<script src="../resources/accessibility-helper.js"></script>
5+
<script src="../resources/js-test.js"></script>
6+
</head>
7+
<body>
8+
9+
<!-- Grid with aria-owned rows -->
10+
<div id="grid-with-owned-rows" role="grid" aria-label="Grid with owned rows" aria-owns="owned-row-1 owned-row-2"></div>
11+
<div id="owned-row-1" role="row">
12+
<div id="owned-row-cell-1-1" role="gridcell">Owned Row 1 Cell 1</div>
13+
<div id="owned-row-cell-1-2" role="gridcell">Owned Row 1 Cell 2</div>
14+
</div>
15+
<div id="owned-row-2" role="row">
16+
<div id="owned-row-cell-2-1" role="gridcell">Owned Row 2 Cell 1</div>
17+
<div id="owned-row-cell-2-2" role="gridcell">Owned Row 2 Cell 2</div>
18+
</div>
19+
<div id="owned-row-3" role="row">
20+
<div id="owned-row-cell-3-1" role="gridcell">Owned Row 3 Cell 1</div>
21+
<div id="owned-row-cell-3-2" role="gridcell">Owned Row 3 Cell 2</div>
22+
</div>
23+
24+
<!-- Grid with aria-owned cells in a row -->
25+
<div id="grid-with-owned-cells" role="grid" aria-label="Grid with owned cells">
26+
<div id="row-with-owned-cells" role="row" aria-owns="owned-cell-1 owned-cell-2"></div>
27+
</div>
28+
<div id="owned-cell-1" role="gridcell">Owned Cell 1</div>
29+
<div id="owned-cell-2" role="gridcell">Owned Cell 2</div>
30+
<div id="owned-cell-3" role="gridcell">Owned Cell 3</div>
31+
32+
<!-- Grid where rows are both DOM children AND aria-owned (tests ordering) -->
33+
<div id="grid-with-dom-and-owned-rows" role="grid" aria-label="Grid with DOM and owned rows" aria-owns="dom-row-B dom-row-A">
34+
<div id="dom-row-A" role="row">
35+
<div id="dom-row-A-cell" role="gridcell">Row A Cell</div>
36+
</div>
37+
<div id="dom-row-B" role="row">
38+
<div id="dom-row-B-cell" role="gridcell">Row B Cell</div>
39+
</div>
40+
<div id="dom-row-C" role="row">
41+
<div id="dom-row-C-cell" role="gridcell">Row C Cell</div>
42+
</div>
43+
</div>
44+
45+
<script>
46+
var output = "This tests that ARIA grids correctly recognize rows and cells via aria-owns, including after dynamic attribute changes.\n\n";
47+
48+
if (window.accessibilityController) {
49+
window.jsTestIsAsync = true;
50+
51+
output += "Testing grid with aria-owned rows:\n";
52+
var ownedRowsGrid = accessibilityController.accessibleElementById("grid-with-owned-rows");
53+
output += expect("ownedRowsGrid.rowCount", "2");
54+
output += expect("ownedRowsGrid.columnCount", "2");
55+
output += expect("ownedRowsGrid.width >= 2", "true");
56+
output += expect("ownedRowsGrid.height >= 2", "true");
57+
output += expect("ownedRowsGrid.cellForColumnAndRow(0, 0).domIdentifier", "'owned-row-cell-1-1'");
58+
output += expect("ownedRowsGrid.cellForColumnAndRow(1, 1).domIdentifier", "'owned-row-cell-2-2'");
59+
60+
output += "\nTesting grid with aria-owned cells:\n";
61+
var ownedCellsGrid = accessibilityController.accessibleElementById("grid-with-owned-cells");
62+
output += expect("ownedCellsGrid.rowCount", "1");
63+
output += expect("ownedCellsGrid.columnCount", "2");
64+
output += expect("ownedCellsGrid.width >= 2", "true");
65+
output += expect("ownedCellsGrid.height >= 2", "true");
66+
output += expect("ownedCellsGrid.cellForColumnAndRow(0, 0).domIdentifier", "'owned-cell-1'");
67+
output += expect("ownedCellsGrid.cellForColumnAndRow(1, 0).domIdentifier", "'owned-cell-2'");
68+
69+
// Test grid where rows are both DOM children AND aria-owned.
70+
// aria-owns="dom-row-B dom-row-A" should reorder: C (not owned, DOM order), then B, then A.
71+
output += "\nTesting grid with DOM children that are also aria-owned (ordering test):\n";
72+
var domAndOwnedGrid = accessibilityController.accessibleElementById("grid-with-dom-and-owned-rows");
73+
output += expect("domAndOwnedGrid.rowCount", "3");
74+
output += expect("domAndOwnedGrid.width >= 2", "true");
75+
output += expect("domAndOwnedGrid.height >= 2", "true");
76+
// Row C is not aria-owned, so it comes first (DOM order for non-owned children).
77+
output += expect("domAndOwnedGrid.cellForColumnAndRow(0, 0).domIdentifier", "'dom-row-C-cell'");
78+
// Row B is first in aria-owns, so it comes second.
79+
output += expect("domAndOwnedGrid.cellForColumnAndRow(0, 1).domIdentifier", "'dom-row-B-cell'");
80+
// Row A is second in aria-owns, so it comes third.
81+
output += expect("domAndOwnedGrid.cellForColumnAndRow(0, 2).domIdentifier", "'dom-row-A-cell'");
82+
83+
output += "\nAdding row-3 to aria-owns...\n";
84+
document.getElementById("grid-with-owned-rows").setAttribute("aria-owns", "owned-row-1 owned-row-2 owned-row-3");
85+
86+
setTimeout(async function() {
87+
output += await expectAsync("ownedRowsGrid.rowCount", "3");
88+
output += expect("ownedRowsGrid.cellForColumnAndRow(0, 2).domIdentifier", "'owned-row-cell-3-1'");
89+
90+
output += "\nAdding owned-cell-3 to row...\n";
91+
document.getElementById("row-with-owned-cells").setAttribute("aria-owns", "owned-cell-1 owned-cell-2 owned-cell-3");
92+
93+
await waitFor(() => ownedCellsGrid.cellForColumnAndRow(2, 0) !== null);
94+
output += expect("ownedCellsGrid.cellForColumnAndRow(2, 0).domIdentifier", "'owned-cell-3'");
95+
96+
output += "\nRemoving owned-row-1 from aria-owns...\n";
97+
document.getElementById("grid-with-owned-rows").setAttribute("aria-owns", "owned-row-2 owned-row-3");
98+
99+
output += await expectAsync("ownedRowsGrid.rowCount", "2");
100+
output += expect("ownedRowsGrid.cellForColumnAndRow(0, 0).domIdentifier", "'owned-row-cell-2-1'");
101+
102+
debug(output);
103+
finishJSTest();
104+
}, 0);
105+
}
106+
</script>
107+
</body>
108+
</html>

‎Source/WebCore/accessibility/AccessibilityNodeObject.cpp‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2603,13 +2603,29 @@ unsigned AccessibilityNodeObject::computeCellSlots()
26032603
if (descendantIsRowGroup)
26042604
processRowGroup(*element);
26052605
};
2606+
// Collect aria-owned children upfront. aria-owned elements are placed after natural DOM
2607+
// children, so skip them if we encounter them in the DOM traversal.
2608+
auto ownedChildren = ownedObjects();
2609+
HashSet<Node*> ownedChildNodes;
2610+
for (const auto& ownedChild : ownedChildren) {
2611+
if (SUPPRESS_UNCOUNTED_LOCAL auto* node = ownedChild->node())
2612+
ownedChildNodes.add(node);
2613+
}
2614+
26062615
// Step 7: Let the current element be the first element child of the table element.
26072616
// Use composedTreeChildren to traverse shadow DOM children too.
26082617
for (Ref child : composedTreeChildren<0>(*tableElement)) {
2618+
// Skip children that are aria-owned; they'll be processed in aria-owns order below.
2619+
if (ownedChildNodes.contains(&child.get()))
2620+
continue;
26092621
processTableDescendant(&child.get());
26102622
// Step 17 + 18: Advance the current element to the next child of the table.
26112623
}
26122624

2625+
// Process any aria-owned children that may be rows or rowgroups.
2626+
for (const auto& ownedChild : ownedChildren)
2627+
processTableDescendant(ownedChild->node());
2628+
26132629
// Step 19: For each tfoot element in the list of pending tfoot elements, in tree order,
26142630
// run the algorithm for processing row groups.
26152631
for (const auto& tfootElement : pendingTfootElements)

‎Source/WebCore/accessibility/isolatedtree/AXIsolatedObject.cpp‎

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1246,6 +1246,15 @@ FloatRect AXIsolatedObject::relativeFrame() const
12461246
// until we cache the necessary information let's go to the main-thread.
12471247
} else if (role() == AccessibilityRole::Column || role() == AccessibilityRole::TableHeaderContainer)
12481248
relativeFrame = exposedTableAncestor() ? relativeFrameFromChildren() : FloatRect();
1249+
else if (isExposableTable()) {
1250+
// If we are an exposable-to-accessibility table, we must have at least one valid row, so see if
1251+
// our row(s) have cached geometry we can use. For tables, this will probably be more accurate
1252+
// than the ancestor bounding-box fallback below.
1253+
for (const auto& child : const_cast<AXIsolatedObject*>(this)->unignoredChildren()) {
1254+
if (std::optional cachedFrame = downcast<AXIsolatedObject>(child)->cachedRelativeFrame())
1255+
relativeFrame.unite(*cachedFrame);
1256+
}
1257+
}
12491258

12501259
// Mock objects and SVG objects need use the main thread since they do not have render nodes and are not painted with layers, respectively.
12511260
// FIXME: Remove isNonLayerSVGObject when LBSE is enabled & SVG frames are cached.
@@ -1297,8 +1306,15 @@ FloatRect AXIsolatedObject::relativeFrame() const
12971306
return ancestorRelativeFrame;
12981307
});
12991308

1300-
if (ancestorRelativeFrame)
1301-
relativeFrame.setLocation(ancestorRelativeFrame->location());
1309+
if (ancestorRelativeFrame) {
1310+
if (relativeFrame.isEmpty() && !isIgnored()) {
1311+
// It's possible our initial frame rect was empty too. For things exposed in the accessibility
1312+
// tree (i.e. they aren't ignored), it's important to expose a non-empty frame, as some ATs
1313+
// like VoiceOver will ignore elements with empty frames.
1314+
relativeFrame = *ancestorRelativeFrame;
1315+
} else
1316+
relativeFrame.setLocation(ancestorRelativeFrame->location());
1317+
}
13021318
}
13031319

13041320
// If an assistive technology is requesting the frame for something,

0 commit comments

Comments
 (0)