Skip to content

Commit d7e2051

Browse files
twilcotwilco
committed
AX: Tables with show/hide rows report wrong counts and block access to some rows in VoiceOver
https://bugs.webkit.org/show_bug.cgi?id=275366 rdar://129612387 Reviewed by Chris Fleizach. This happened because fundamentally, `AccessibilityTableRow::computeAccessibilityIsIgnored` did not respect hidden states at all (https://www.w3.org/TR/wai-aria/#dfn-hidden — display:none or visibility:hidden). We only got away with it for so long because `AXObjectCache::getOrCreate(Node& node, IsPartOfRelation isPartOfRelation)` limits the types of objects that can be created without a renderer, effectively blocking these hidden objects. Except it unconditionally creates objects that are part of a relantionship (like `aria-controls`): ``` // If node is the target of a relationship or a descendant of one, create an AX object unconditionally. if (isPartOfRelation == IsPartOfRelation::No && !isDescendantOfRelatedNode(node)) { ...strict criteria to create a renderer-less object... } ``` After this commit, `AccessibilityTableRow::computeAccessibilityIsIgnored` now properly respects hidden status, fixing the bug. * LayoutTests/accessibility/aria-controlled-table-row-visibility-expected.txt: Added. * LayoutTests/accessibility/aria-controlled-table-row-visibility.html: Added. * LayoutTests/platform/ios/TestExpectations: Enable new test. * LayoutTests/platform/ios/accessibility/aria-controlled-table-row-visibility-expected.txt: Added. * LayoutTests/platform/mac-wk1/TestExpectations: Skip new test. * Source/WebCore/accessibility/AccessibilityTableRow.cpp: (WebCore::AccessibilityTableRow::computeAccessibilityIsIgnored const): Canonical link: https://commits.webkit.org/282165@main
1 parent 65d48e5 commit d7e2051

File tree

7 files changed

+174
-4
lines changed

7 files changed

+174
-4
lines changed

‎LayoutTests/accessibility/aria-controlled-table-row-visibility-expected.txt‎

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
This test ensures the accessibility tree is correct after table rows with an aria-controls relationship dynamically change their hidden status.
2+
3+
4+
{#table AXRole: AXTable}
5+
6+
{#r0 AXRole: AXRow}
7+
8+
{#r0c0 AXRole: AXCell}
9+
10+
{AXRole: AXStaticText AXValue: Author}
11+
12+
{#r0c1 AXRole: AXCell}
13+
14+
{AXRole: AXStaticText AXValue: Title}
15+
16+
{#r1 AXRole: AXRow}
17+
18+
{#r1c0 AXRole: AXCell}
19+
20+
{AXRole: AXButton}
21+
22+
{#r1c1 AXRole: AXCell}
23+
24+
{AXRole: AXStaticText AXValue: A Brief History of Time}
25+
26+
PASS: output.includes('Carl Sagan') === false
27+
PASS: table.rowCount === 2 === true
28+
29+
30+
Traversal after un-hiding #r2:
31+
32+
{#table AXRole: AXTable}
33+
34+
{#r0 AXRole: AXRow}
35+
36+
{#r0c0 AXRole: AXCell}
37+
38+
{AXRole: AXStaticText AXValue: Author}
39+
40+
{#r0c1 AXRole: AXCell}
41+
42+
{AXRole: AXStaticText AXValue: Title}
43+
44+
{#r1 AXRole: AXRow}
45+
46+
{#r1c0 AXRole: AXCell}
47+
48+
{AXRole: AXButton}
49+
50+
{#r1c1 AXRole: AXCell}
51+
52+
{AXRole: AXStaticText AXValue: A Brief History of Time}
53+
54+
{#r2 AXRole: AXRow}
55+
56+
{#r2c0 AXRole: AXCell}
57+
58+
{AXRole: AXStaticText AXValue: Carl Sagan}
59+
60+
{#r2c1 AXRole: AXCell}
61+
62+
{AXRole: AXStaticText AXValue: Cosmos}
63+
64+
PASS: table.rowCount === 3 === true
65+
66+
PASS successfullyParsed is true
67+
68+
TEST COMPLETE
69+
This is a table caption
70+
Author Title
71+
Toggle second row A Brief History of Time
72+
Carl Sagan Cosmos

‎LayoutTests/accessibility/aria-controlled-table-row-visibility.html‎

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">
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+
<table id="table">
10+
<caption>This is a table caption</caption>
11+
<thead>
12+
<tr id="r0">
13+
<th id="r0c0">Author</th>
14+
<th id="r0c1">Title</th>
15+
</tr>
16+
</thead>
17+
<tbody>
18+
<tr id="r1">
19+
<td id="r1c0"><button aria-controls="r2">Toggle second row</button></td>
20+
<td id="r1c1">A Brief History of Time</td>
21+
</tr>
22+
<tr id="r2" style="display:none">
23+
<td id="r2c0">Carl Sagan</td>
24+
<td id="r2c1">Cosmos</td>
25+
</tr>
26+
</tbody>
27+
</table>
28+
29+
<script>
30+
var output = "This test ensures the accessibility tree is correct after table rows with an aria-controls relationship dynamically change their hidden status.\n\n";
31+
32+
if (window.accessibilityController) {
33+
window.jsTestIsAsync = true;
34+
35+
const isiOS = accessibilityController.platformName === "ios";
36+
var webarea = accessibilityController.rootElement.childAtIndex(0);
37+
var table = !isiOS ? accessibilityController.accessibleElementById("table") : null;
38+
output += `${dumpAXSearchTraversal(webarea)}\n`;
39+
output += expect("output.includes('Carl Sagan')", "false");
40+
if (!isiOS)
41+
output += expect("table.rowCount === 2", "true");
42+
43+
document.getElementById("r2").removeAttribute("style");
44+
var searchOutput = "";
45+
setTimeout(async function() {
46+
await waitFor(() => {
47+
searchOutput = dumpAXSearchTraversal(webarea);
48+
return searchOutput.includes("Carl Sagan");
49+
});
50+
output += `\n\nTraversal after un-hiding #r2:\n${searchOutput}\n`;
51+
if (!isiOS)
52+
output += expect("table.rowCount === 3", "true");
53+
54+
debug(output);
55+
finishJSTest();
56+
}, 0);
57+
}
58+
</script>
59+
</body>
60+
</html>
61+

‎LayoutTests/platform/glib/TestExpectations‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -584,6 +584,7 @@ accessibility/dynamically-ignored-canvas.html [ Skip ]
584584
accessibility/basic-focusability.html [ Skip ]
585585

586586
# Missing AccessibilityUIElement::uiElementForSearchPredicate implementation.
587+
accessibility/aria-controlled-table-row-visibility.html [ Skip ]
587588
accessibility/aria-modal-with-text-crash.html [ Skip ]
588589
accessibility/button-inside-label-ax-text.html [ Skip ]
589590
accessibility/display-contents/dynamically-added-children.html [ Skip ]

‎LayoutTests/platform/ios/TestExpectations‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2312,6 +2312,7 @@ accessibility/accessibility-node-reparent.html [ Pass ]
23122312
accessibility/aria-braillelabel.html [ Pass ]
23132313
accessibility/aria-brailleroledescription.html [ Pass ]
23142314
accessibility/aria-checked-mixed-value.html [ Pass ]
2315+
accessibility/aria-controlled-table-row-visibility.html [ Pass ]
23152316
accessibility/aria-describedby-on-input.html [ Pass ]
23162317
accessibility/body-element-aria-hidden.html [ Pass ]
23172318
accessibility/display-contents/aria-hidden.html [ Pass ]

‎LayoutTests/platform/ios/accessibility/aria-controlled-table-row-visibility-expected.txt‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
This test ensures the accessibility tree is correct after table rows with an aria-controls relationship dynamically change their hidden status.
2+
3+
4+
{StaticText AXLabel: Author}
5+
6+
{StaticText AXLabel: Title}
7+
8+
{Button}
9+
10+
{StaticText AXLabel: A Brief History of Time}
11+
12+
PASS: output.includes('Carl Sagan') === false
13+
14+
15+
Traversal after un-hiding #r2:
16+
17+
{StaticText AXLabel: Author}
18+
19+
{StaticText AXLabel: Title}
20+
21+
{Button}
22+
23+
{StaticText AXLabel: A Brief History of Time}
24+
25+
{StaticText AXLabel: Carl Sagan}
26+
27+
{StaticText AXLabel: Cosmos}
28+
29+
30+
PASS successfullyParsed is true
31+
32+
TEST COMPLETE
33+
This is a table caption
34+
Author Title
35+
Toggle second row A Brief History of Time
36+
Carl Sagan Cosmos

‎LayoutTests/platform/mac-wk1/TestExpectations‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -879,6 +879,8 @@ accessibility/mac/lazy-spellchecking.html [ Skip ]
879879
accessibility/mac/spellcheck-with-voiceover.html [ Skip ]
880880
accessibility/text-marker/text-marker-range-with-unordered-markers.html
881881

882+
accessibility/aria-controlled-table-row-visibility.html [ Skip ]
883+
882884
# AccessibilityController::setForceInitialFrameCaching not supported on WK1.
883885
accessibility/mac/initial-relative-frame-cached.html [ Skip ]
884886

‎Source/WebCore/accessibility/AccessibilityTableRow.cpp‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -95,10 +95,7 @@ bool AccessibilityTableRow::computeAccessibilityIsIgnored() const
9595
if (!isTableRow())
9696
return AccessibilityRenderObject::computeAccessibilityIsIgnored();
9797

98-
if (ignoredFromPresentationalRole())
99-
return true;
100-
101-
return false;
98+
return isDOMHidden() || ignoredFromPresentationalRole();
10299
}
103100

104101
AccessibilityTable* AccessibilityTableRow::parentTable() const

0 commit comments

Comments
 (0)