Commit c4633477 authored by Aaron Leventhal's avatar Aaron Leventhal Committed by Chromium LUCI CQ

ARIA grids is not mixing well with aria-owns

Remove RemapAriaRolesDueToParent:
- It's harmless to have cells, rows etc. exist without a parent table.
  Having code in Blink to remap these roles causes complexity and
  issues with aria-owns. Adding any complexity to do this is more
  trouble than it's worth
- Remapping kListBoxOption to a kMenuItem when it's in a kMenu is
  related to old code inherited from WebKit. WebKit made efforts
  to try to change role=option or <option> elements inside a
  role=menu to menuitems, but this is just not worth it.

Also, 2 old tests that failed now pass, so the -expected.txt files
are removed -- the expectations were only there to allow the failures.

Bug: 1150481
Change-Id: I7342e3c15bf2dd3d877f90e682db7d68bf8c9142
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2568365Reviewed-by: default avatarDominic Mazzoni <dmazzoni@chromium.org>
Commit-Queue: Aaron Leventhal <aleventhal@chromium.org>
Cr-Commit-Position: refs/heads/master@{#834008}
parent dd92dd21
...@@ -998,6 +998,10 @@ IN_PROC_BROWSER_TEST_P(DumpAccessibilityTreeTest, AccessibilityAriaOwnsCrash2) { ...@@ -998,6 +998,10 @@ IN_PROC_BROWSER_TEST_P(DumpAccessibilityTreeTest, AccessibilityAriaOwnsCrash2) {
RunAriaTest(FILE_PATH_LITERAL("aria-owns-crash-2.html")); RunAriaTest(FILE_PATH_LITERAL("aria-owns-crash-2.html"));
} }
IN_PROC_BROWSER_TEST_P(DumpAccessibilityTreeTest, AccessibilityAriaOwnsGrid) {
RunAriaTest(FILE_PATH_LITERAL("aria-owns-grid.html"));
}
IN_PROC_BROWSER_TEST_P(DumpAccessibilityTreeTest, IN_PROC_BROWSER_TEST_P(DumpAccessibilityTreeTest,
AccessibilityAriaOwnsIgnored) { AccessibilityAriaOwnsIgnored) {
RunAriaTest(FILE_PATH_LITERAL("aria-owns-ignored.html")); RunAriaTest(FILE_PATH_LITERAL("aria-owns-ignored.html"));
...@@ -1183,11 +1187,6 @@ IN_PROC_BROWSER_TEST_P(DumpAccessibilityTreeTest, AccessibilityAriaTable) { ...@@ -1183,11 +1187,6 @@ IN_PROC_BROWSER_TEST_P(DumpAccessibilityTreeTest, AccessibilityAriaTable) {
RunAriaTest(FILE_PATH_LITERAL("aria-table.html")); RunAriaTest(FILE_PATH_LITERAL("aria-table.html"));
} }
IN_PROC_BROWSER_TEST_P(DumpAccessibilityTreeTest,
AccessibilityAriaTableIllegalRoles) {
RunAriaTest(FILE_PATH_LITERAL("aria-table-illegal-roles.html"));
}
IN_PROC_BROWSER_TEST_P(DumpAccessibilityTreeTest, IN_PROC_BROWSER_TEST_P(DumpAccessibilityTreeTest,
AccessibilityAriaTabNestedInLists) { AccessibilityAriaTabNestedInLists) {
RunAriaTest(FILE_PATH_LITERAL("aria-tab-nested-in-lists.html")); RunAriaTest(FILE_PATH_LITERAL("aria-tab-nested-in-lists.html"));
......
rootWebArea rootWebArea
++genericContainer ignored ++genericContainer ignored
++++genericContainer ignored ++++genericContainer ignored
++++++grid ++++++grid tableRowCount=2 tableColumnCount=2
++++++++row ++++++++row tableRowIndex=0
++++++++++columnHeader name='Browser' ++++++++++columnHeader name='Browser' tableCellColumnIndex=0 tableCellColumnSpan=1 tableCellRowIndex=0 tableCellRowSpan=1
++++++++++++staticText name='Browser' ++++++++++++staticText name='Browser'
++++++++++++++inlineTextBox name='Browser' ++++++++++++++inlineTextBox name='Browser'
++++++++++columnHeader name='Rendering Engine' ++++++++++columnHeader name='Rendering Engine' tableCellColumnIndex=1 tableCellColumnSpan=1 tableCellRowIndex=0 tableCellRowSpan=1
++++++++++++staticText name='Rendering Engine' ++++++++++++staticText name='Rendering Engine'
++++++++++++++inlineTextBox name='Rendering Engine' ++++++++++++++inlineTextBox name='Rendering Engine'
++++++++row ++++++++row tableRowIndex=1
++++++++++cell name='Chrome' ++++++++++cell name='Chrome' tableCellColumnIndex=0 tableCellColumnSpan=1 tableCellRowIndex=1 tableCellRowSpan=1
++++++++++++staticText name='Chrome' ++++++++++++staticText name='Chrome'
++++++++++++++inlineTextBox name='Chrome' ++++++++++++++inlineTextBox name='Chrome'
++++++++++cell name='Blink' ++++++++++cell name='Blink' tableCellColumnIndex=1 tableCellColumnSpan=1 tableCellRowIndex=1 tableCellRowSpan=1
++++++++++++staticText name='Blink' ++++++++++++staticText name='Blink'
++++++++++++++inlineTextBox name='Blink' ++++++++++++++inlineTextBox name='Blink'
<!-- <!--
@BLINK-ALLOW:table*
@MAC-ALLOW:AXRoleDescription @MAC-ALLOW:AXRoleDescription
@WIN-ALLOW:*SELECTABLE @WIN-ALLOW:*SELECTABLE
@WIN-ALLOW:xml-roles* @WIN-ALLOW:xml-roles*
......
rootWebArea
++genericContainer ignored
++++genericContainer ignored
++++++grid tableRowCount=2 tableColumnCount=2
++++++++row tableRowIndex=0
++++++++++columnHeader name='Browser' tableCellColumnIndex=0 tableCellColumnSpan=1 tableCellRowIndex=0 tableCellRowSpan=1
++++++++++++staticText name='Browser'
++++++++++++++inlineTextBox name='Browser'
++++++++++columnHeader name='Rendering Engine' tableCellColumnIndex=1 tableCellColumnSpan=1 tableCellRowIndex=0 tableCellRowSpan=1
++++++++++++staticText name='Rendering Engine'
++++++++++++++inlineTextBox name='Rendering Engine'
++++++++row tableRowIndex=1
++++++++++cell name='Chrome' tableCellColumnIndex=0 tableCellColumnSpan=1 tableCellRowIndex=1 tableCellRowSpan=1
++++++++++++staticText name='Chrome'
++++++++++++++inlineTextBox name='Chrome'
++++++++++cell name='Blink' tableCellColumnIndex=1 tableCellColumnSpan=1 tableCellRowIndex=1 tableCellRowSpan=1
++++++++++++staticText name='Blink'
++++++++++++++inlineTextBox name='Blink'
<!--
@BLINK-ALLOW:table*
-->
<div role="grid" aria-owns="row1 row2">
</div>
<div id="row1" role="row">
<span role="columnheader">Browser</span>
<span role="columnheader">Rendering Engine</span>
</div>
<div id="row2" role="row">
<span role="gridcell">Chrome</span>
<span role="gridcell">Blink</span>
</div>
rootWebArea
++genericContainer ignored
++++genericContainer ignored
++++++genericContainer
++++++++staticText name='illegal row'
++++++++++inlineTextBox name='illegal row'
++++++genericContainer
++++++++staticText name='illegal cell'
++++++++++inlineTextBox name='illegal cell'
++++++genericContainer
++++++++staticText name='illegal rowheader'
++++++++++inlineTextBox name='illegal rowheader'
++++++genericContainer
++++++++staticText name='illegal columnheader'
++++++++++inlineTextBox name='illegal columnheader'
++++++genericContainer
++++++++staticText name='illegal rowgroup'
++++++++++inlineTextBox name='illegal rowgroup'
++++++table
++++++++row
++++++++++cell name='legal cell'
++++++++++++genericContainer
++++++++++++++staticText name='illegal cell inside another cell'
++++++++++++++++inlineTextBox name='illegal cell inside another cell'
<!--
Rows, cells, etc. require containing table, grid or treegrid.
-->
<!DOCTYPE html>
<html>
<body>
<div role="row">illegal row</div>
<div role="cell">illegal cell</div>
<div role="rowheader">illegal rowheader</div>
<div role="columnheader">illegal columnheader</div>
<div role="rowgroup">illegal rowgroup</div>
<div role="table">
<div role="row">
<div role="cell" aria-label="legal cell">
<div role="cell">
illegal cell inside another cell
</div>
</div>
</div>
</div>
</body>
</html>
...@@ -2984,8 +2984,6 @@ ax::mojom::blink::Role AXObject::DetermineAriaRoleAttribute() const { ...@@ -2984,8 +2984,6 @@ ax::mojom::blink::Role AXObject::DetermineAriaRoleAttribute() const {
if (role == ax::mojom::blink::Role::kButton) if (role == ax::mojom::blink::Role::kButton)
role = ButtonRoleType(); role = ButtonRoleType();
role = RemapAriaRoleDueToParent(role);
// Distinguish between different uses of the "combobox" role: // Distinguish between different uses of the "combobox" role:
// //
// ax::mojom::blink::Role::kComboBoxGrouping: // ax::mojom::blink::Role::kComboBoxGrouping:
...@@ -3007,60 +3005,6 @@ ax::mojom::blink::Role AXObject::DetermineAriaRoleAttribute() const { ...@@ -3007,60 +3005,6 @@ ax::mojom::blink::Role AXObject::DetermineAriaRoleAttribute() const {
return ax::mojom::blink::Role::kUnknown; return ax::mojom::blink::Role::kUnknown;
} }
ax::mojom::blink::Role AXObject::RemapAriaRoleDueToParent(
ax::mojom::blink::Role role) const {
// Some objects change their role based on their parent.
// However, asking for the unignoredParent calls accessibilityIsIgnored(),
// which can trigger a loop. While inside the call stack of creating an
// element, we need to avoid accessibilityIsIgnored().
// https://bugs.webkit.org/show_bug.cgi?id=65174
// Don't return table roles unless inside a table-like container.
switch (role) {
case ax::mojom::blink::Role::kRow:
case ax::mojom::blink::Role::kRowGroup:
case ax::mojom::blink::Role::kCell:
case ax::mojom::blink::Role::kRowHeader:
case ax::mojom::blink::Role::kColumnHeader:
for (AXObject* ancestor = ParentObjectUnignored(); ancestor;
ancestor = ancestor->ParentObjectUnignored()) {
ax::mojom::blink::Role ancestor_aria_role =
ancestor->AriaRoleAttribute();
if (ancestor_aria_role == ax::mojom::blink::Role::kCell)
return ax::mojom::blink::Role::kGenericContainer; // In another cell,
// illegal.
if (ancestor->IsTableLikeRole())
return role; // Inside a table: ARIA role is legal.
}
return ax::mojom::blink::Role::kGenericContainer; // Not in a table.
default:
break;
}
if (role != ax::mojom::blink::Role::kListBoxOption &&
role != ax::mojom::blink::Role::kMenuItem)
return role;
for (AXObject* parent = ParentObject();
parent && !parent->AccessibilityIsIgnored();
parent = parent->ParentObject()) {
ax::mojom::blink::Role parent_aria_role = parent->AriaRoleAttribute();
// Selects and listboxes both have options as child roles, but they map to
// different roles within WebCore.
if (role == ax::mojom::blink::Role::kListBoxOption &&
parent_aria_role == ax::mojom::blink::Role::kMenu)
return ax::mojom::blink::Role::kMenuItem;
// If the parent had a different role, then we don't need to continue
// searching up the chain.
if (parent_aria_role != ax::mojom::blink::Role::kUnknown)
break;
}
return role;
}
bool AXObject::IsEditableRoot() const { bool AXObject::IsEditableRoot() const {
UpdateCachedAttributeValuesIfNeeded(); UpdateCachedAttributeValuesIfNeeded();
return cached_is_editable_root_; return cached_is_editable_root_;
......
This is a testharness.js-based test.
FAIL Aria-owns works on ARIA grids. assert_equals: expected 3 but got 0
Harness: the test ran to completion.
This is a testharness.js-based test.
FAIL An ARIA cell without a layout object does not crash assert_equals: expected "AXRole: AXCell" but got "AXRole: AXGenericContainer"
PASS An ARIA row in a select in a canvas does not crash
Harness: the test ran to completion.
...@@ -84,7 +84,7 @@ AXRole: AXGenericContainer ...@@ -84,7 +84,7 @@ AXRole: AXGenericContainer
AXRole: AXInlineTextBox "The row for "Explicit th" has a row role even if table has a presentation role because it has an explicit role." AXRole: AXInlineTextBox "The row for "Explicit th" has a row role even if table has a presentation role because it has an explicit role."
AXRole: AXPresentational AXRole: AXPresentational
AXRole: AXGenericContainer AXRole: AXGenericContainer
AXRole: AXGenericContainer AXRole: AXRow
AXRole: AXGenericContainer AXRole: AXGenericContainer
AXRole: AXStaticText "Explicit th" AXRole: AXStaticText "Explicit th"
AXRole: AXInlineTextBox "Explicit th" AXRole: AXInlineTextBox "Explicit th"
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment