CSS shouldn't cause an HTML table to not be exposed as a table

Bug: 1017535
Change-Id: I398c6870ac0f82452baee8aed9d0f273f1e7af1f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1894292
Commit-Queue: Aaron Leventhal <aleventhal@chromium.org>
Reviewed-by: Dominic Mazzoni <dmazzoni@chromium.org>
Cr-Commit-Position: refs/heads/master@{#712314}

content/browser/accessibility/dump_accessibility_tree_browsertest.cc

@@ -283,6 +283,11 @@ IN_PROC_BROWSER_TEST_P(DumpAccessibilityTreeTest,
   RunCSSTest(FILE_PATH_LITERAL("table-display.html"));
 }
 
+IN_PROC_BROWSER_TEST_P(DumpAccessibilityTreeTest,
+                       AccessibilityCSSTableDisplayOther) {
+  RunCSSTest(FILE_PATH_LITERAL("table-display-other.html"));
+}
+
 IN_PROC_BROWSER_TEST_P(DumpAccessibilityTreeTest, AccessibilityCSSTransform) {
   RunCSSTest(FILE_PATH_LITERAL("transform.html"));
 }

content/test/data/accessibility/css/table-display-other-expected-blink.txt

+rootWebArea
+++genericContainer ignored
+++++layoutTable
+++++++genericContainer ignored
+++++++++layoutTableRow
+++++++++++layoutTableCell name='cat'
+++++++++++++staticText name='cat'
+++++++++++++++inlineTextBox name='cat'

content/test/data/accessibility/css/table-display-other-expected-win.txt

+ROLE_SYSTEM_DOCUMENT READONLY FOCUSABLE
+++ROLE_SYSTEM_TABLE layout-guess:true table_rows=1 table_columns=1
+++++ROLE_SYSTEM_ROW
+++++++ROLE_SYSTEM_CELL name='cat' table-cell-index:0
+++++++++ROLE_SYSTEM_STATICTEXT name='cat'

content/test/data/accessibility/css/table-display-other.html

+<!--
+@WIN-ALLOW:table*
+-->
+<!-- Ensure that a table with display styles is still exposed as a table.. -->
+<style>
+table, tbody, tr, td { display: block; }
+</style>
+<table border="1">
+  <tbody>
+    <tr>
+      <td>
+        cat
+      </td>
+    </tr>
+  </tbody>
+</table>

content/test/data/accessibility/html/table-headers-on-all-sides.html

@@ -18,7 +18,7 @@
     <th>Fruit</th>
   </tr>
   <tr>
-    <th>Veggies</th>
+    <th id="veggies">Veggies</th>
     <td>radish</td>
     <td>spinach</td>
     <th>Veggies</th>

third_party/blink/renderer/modules/accessibility/ax_layout_object.cc

@@ -100,17 +100,6 @@
 #include "third_party/blink/renderer/platform/text/text_direction.h"
 #include "third_party/blink/renderer/platform/wtf/std_lib_extras.h"
 
-namespace {
-bool IsNeutralWithinTable(blink::AXObject* obj) {
-  if (!obj)
-    return false;
-  ax::mojom::Role role = obj->RoleValue();
-  return role == ax::mojom::Role::kGroup ||
-         role == ax::mojom::Role::kGenericContainer ||
-         role == ax::mojom::Role::kIgnored;
-}
-}  // namespace
-
 namespace blink {
 
 AXLayoutObject::AXLayoutObject(LayoutObject* layout_object,
@@ -203,14 +192,18 @@ ax::mojom::Role AXLayoutObject::NativeRoleIgnoringAria() const {
     return ax::mojom::Role::kLineBreak;
   if (layout_object_->IsText())
     return ax::mojom::Role::kStaticText;
-  if (layout_object_->IsTable() && node) {
-    return IsDataTable() ? ax::mojom::Role::kTable
-                         : ax::mojom::Role::kLayoutTable;
-  }
+
+  // Chrome exposes both table markup and table CSS as a tables, letting
+  // the screen reader determine what to do for CSS tables. If this line
+  // is reached, then it is not an HTML table, and therefore will only be
+  // considered a data table if ARIA markup indicates it is a table.
+  if (layout_object_->IsTable() && node)
+    return ax::mojom::Role::kLayoutTable;
   if (layout_object_->IsTableRow() && node)
     return DetermineTableRowRole();
   if (layout_object_->IsTableCell() && node)
     return DetermineTableCellRole();
+
   if (css_box && IsImageOrAltText(css_box, node)) {
     if (node && node->IsLink())
       return ax::mojom::Role::kImageMap;
@@ -2793,119 +2786,6 @@ ax::mojom::SortDirection AXLayoutObject::GetSortDirection() const {
   return ax::mojom::SortDirection::kOther;
 }
 
-static bool IsNonEmptyNonHeaderCell(const LayoutNGTableCellInterface* cell) {
-  if (!cell)
-    return false;
-
-  if (Node* node = cell->ToLayoutObject()->GetNode())
-    return node->hasChildren() && node->HasTagName(html_names::kTdTag);
-
-  return false;
-}
-
-static bool IsHeaderCell(const LayoutNGTableCellInterface* cell) {
-  if (!cell)
-    return false;
-
-  if (Node* node = cell->ToLayoutObject()->GetNode())
-    return node->HasTagName(html_names::kThTag);
-
-  return false;
-}
-
-static ax::mojom::Role DecideRoleFromSiblings(
-    LayoutNGTableCellInterface* cell) {
-  if (!IsHeaderCell(cell))
-    return ax::mojom::Role::kCell;
-
-  // If this header is only cell in its row, it is a column header.
-  // It is also a column header if it has a header on either side of it.
-  // If instead it has a non-empty td element next to it, it is a row header.
-
-  const LayoutNGTableCellInterface* next_cell = cell->NextCellInterface();
-  const LayoutNGTableCellInterface* previous_cell =
-      cell->PreviousCellInterface();
-  if (!next_cell && !previous_cell)
-    return ax::mojom::Role::kColumnHeader;
-  if (IsHeaderCell(next_cell) && IsHeaderCell(previous_cell))
-    return ax::mojom::Role::kColumnHeader;
-  if (IsNonEmptyNonHeaderCell(next_cell) ||
-      IsNonEmptyNonHeaderCell(previous_cell))
-    return ax::mojom::Role::kRowHeader;
-
-  const LayoutNGTableRowInterface* layout_row = cell->RowInterface();
-  DCHECK(layout_row);
-
-  // If this row's first or last cell is a non-empty td, this is a row header.
-  // Do the same check for the second and second-to-last cells because tables
-  // often have an empty cell at the intersection of the row and column headers.
-  const LayoutNGTableCellInterface* first_cell =
-      layout_row->FirstCellInterface();
-  DCHECK(first_cell);
-
-  const LayoutNGTableCellInterface* last_cell = layout_row->LastCellInterface();
-  DCHECK(last_cell);
-
-  if (IsNonEmptyNonHeaderCell(first_cell) || IsNonEmptyNonHeaderCell(last_cell))
-    return ax::mojom::Role::kRowHeader;
-
-  if (IsNonEmptyNonHeaderCell(first_cell->NextCellInterface()) ||
-      IsNonEmptyNonHeaderCell(last_cell->PreviousCellInterface()))
-    return ax::mojom::Role::kRowHeader;
-
-  // We have no evidence that this is not a column header.
-  return ax::mojom::Role::kColumnHeader;
-}
-
-ax::mojom::Role AXLayoutObject::DetermineTableRowRole() const {
-  AXObject* parent = ParentObject();
-  while (IsNeutralWithinTable(parent))
-    parent = parent->ParentObject();
-
-  if (!parent || !parent->IsTableLikeRole())
-    return ax::mojom::Role::kGenericContainer;
-
-  if (parent->RoleValue() == ax::mojom::Role::kLayoutTable)
-    return ax::mojom::Role::kLayoutTableRow;
-
-  if (parent->IsTableLikeRole())
-    return ax::mojom::Role::kRow;
-
-  return ax::mojom::Role::kGenericContainer;
-}
-
-ax::mojom::Role AXLayoutObject::DetermineTableCellRole() const {
-  DCHECK(layout_object_);
-
-  AXObject* parent = ParentObject();
-  if (!parent || !parent->IsTableRowLikeRole())
-    return ax::mojom::Role::kGenericContainer;
-
-  // Ensure table container.
-  AXObject* grandparent = parent->ParentObject();
-  while (IsNeutralWithinTable(grandparent))
-    grandparent = grandparent->ParentObject();
-  if (!grandparent || !grandparent->IsTableLikeRole())
-    return ax::mojom::Role::kGenericContainer;
-
-  if (parent->RoleValue() == ax::mojom::Role::kLayoutTableRow)
-    return ax::mojom::Role::kLayoutTableCell;
-
-  if (!GetNode() || !GetNode()->HasTagName(html_names::kThTag))
-    return ax::mojom::Role::kCell;
-
-  const AtomicString& scope = GetAttribute(html_names::kScopeAttr);
-  if (EqualIgnoringASCIICase(scope, "row") ||
-      EqualIgnoringASCIICase(scope, "rowgroup"))
-    return ax::mojom::Role::kRowHeader;
-  if (EqualIgnoringASCIICase(scope, "col") ||
-      EqualIgnoringASCIICase(scope, "colgroup"))
-    return ax::mojom::Role::kColumnHeader;
-
-  return DecideRoleFromSiblings(
-      ToInterface<LayoutNGTableCellInterface>(layout_object_));
-}
-
 AXObject* AXLayoutObject::CellForColumnAndRow(unsigned target_column_index,
                                               unsigned target_row_index) const {
   LayoutObject* layout_object = GetLayoutObject();

third_party/blink/renderer/modules/accessibility/ax_layout_object.h

@@ -224,8 +224,6 @@ class MODULES_EXPORT AXLayoutObject : public AXNodeObject {
   void AddRemoteSVGChildren();
   void AddTableChildren();
   void AddValidationMessageChild();
-  ax::mojom::Role DetermineTableCellRole() const;
-  ax::mojom::Role DetermineTableRowRole() const;
   bool FindAllTableCellsWithRole(ax::mojom::Role, AXObjectVector&) const;
 
   LayoutRect ComputeElementRect() const;

third_party/blink/renderer/modules/accessibility/ax_node_object.cc

@@ -83,6 +83,17 @@
 #include "third_party/blink/renderer/platform/weborigin/kurl.h"
 #include "third_party/blink/renderer/platform/wtf/text/string_builder.h"
 
+namespace {
+bool IsNeutralWithinTable(blink::AXObject* obj) {
+  if (!obj)
+    return false;
+  ax::mojom::Role role = obj->RoleValue();
+  return role == ax::mojom::Role::kGroup ||
+         role == ax::mojom::Role::kGenericContainer ||
+         role == ax::mojom::Role::kIgnored;
+}
+}  // namespace
+
 namespace blink {
 
 using html_names::kAltAttr;
@@ -487,6 +498,100 @@ bool AXNodeObject::IsDescendantOfElementType(
   return false;
 }
 
+static bool IsNonEmptyNonHeaderCell(const Node* cell) {
+  return cell && cell->hasChildren() && cell->HasTagName(html_names::kTdTag);
+}
+
+static bool IsHeaderCell(const Node* cell) {
+  return cell && cell->HasTagName(html_names::kThTag);
+}
+
+static ax::mojom::Role DecideRoleFromSiblings(Element* cell) {
+  // If this header is only cell in its row, it is a column header.
+  // It is also a column header if it has a header on either side of it.
+  // If instead it has a non-empty td element next to it, it is a row header.
+
+  const Node* next_cell = LayoutTreeBuilderTraversal::NextSibling(*cell);
+  const Node* previous_cell =
+      LayoutTreeBuilderTraversal::PreviousSibling(*cell);
+  if (!next_cell && !previous_cell)
+    return ax::mojom::Role::kColumnHeader;
+  if (IsHeaderCell(next_cell) && IsHeaderCell(previous_cell))
+    return ax::mojom::Role::kColumnHeader;
+  if (IsNonEmptyNonHeaderCell(next_cell) ||
+      IsNonEmptyNonHeaderCell(previous_cell))
+    return ax::mojom::Role::kRowHeader;
+
+  const Element* row = ToElement(cell->parentNode());
+  if (!row || !row->HasTagName(html_names::kTrTag))
+    return ax::mojom::Role::kColumnHeader;
+
+  // If this row's first or last cell is a non-empty td, this is a row header.
+  // Do the same check for the second and second-to-last cells because tables
+  // often have an empty cell at the intersection of the row and column headers.
+  const Element* first_cell = ElementTraversal::FirstChild(*row);
+  DCHECK(first_cell);
+
+  const Element* last_cell = ElementTraversal::LastChild(*row);
+  DCHECK(last_cell);
+
+  if (IsNonEmptyNonHeaderCell(first_cell) || IsNonEmptyNonHeaderCell(last_cell))
+    return ax::mojom::Role::kRowHeader;
+
+  if (IsNonEmptyNonHeaderCell(ElementTraversal::NextSibling(*first_cell)) ||
+      IsNonEmptyNonHeaderCell(ElementTraversal::PreviousSibling(*last_cell)))
+    return ax::mojom::Role::kRowHeader;
+
+  // We have no evidence that this is not a column header.
+  return ax::mojom::Role::kColumnHeader;
+}
+
+ax::mojom::Role AXNodeObject::DetermineTableRowRole() const {
+  AXObject* parent = ParentObject();
+  while (IsNeutralWithinTable(parent))
+    parent = parent->ParentObject();
+
+  if (!parent || !parent->IsTableLikeRole())
+    return ax::mojom::Role::kGenericContainer;
+
+  if (parent->RoleValue() == ax::mojom::Role::kLayoutTable)
+    return ax::mojom::Role::kLayoutTableRow;
+
+  if (parent->IsTableLikeRole())
+    return ax::mojom::Role::kRow;
+
+  return ax::mojom::Role::kGenericContainer;
+}
+
+ax::mojom::Role AXNodeObject::DetermineTableCellRole() const {
+  AXObject* parent = ParentObject();
+  if (!parent || !parent->IsTableRowLikeRole())
+    return ax::mojom::Role::kGenericContainer;
+
+  // Ensure table container.
+  AXObject* grandparent = parent->ParentObject();
+  while (IsNeutralWithinTable(grandparent))
+    grandparent = grandparent->ParentObject();
+  if (!grandparent || !grandparent->IsTableLikeRole())
+    return ax::mojom::Role::kGenericContainer;
+
+  if (parent->RoleValue() == ax::mojom::Role::kLayoutTableRow)
+    return ax::mojom::Role::kLayoutTableCell;
+
+  if (!GetElement() || !GetNode()->HasTagName(html_names::kThTag))
+    return ax::mojom::Role::kCell;
+
+  const AtomicString& scope = GetAttribute(html_names::kScopeAttr);
+  if (EqualIgnoringASCIICase(scope, "row") ||
+      EqualIgnoringASCIICase(scope, "rowgroup"))
+    return ax::mojom::Role::kRowHeader;
+  if (EqualIgnoringASCIICase(scope, "col") ||
+      EqualIgnoringASCIICase(scope, "colgroup"))
+    return ax::mojom::Role::kColumnHeader;
+
+  return DecideRoleFromSiblings(GetElement());
+}
+
 // TODO(accessibility) Needs a new name as it does check ARIA, including
 // checking the @role for an iframe, and @aria-haspopup/aria-pressed via
 // ButtonType().
@@ -526,6 +631,17 @@ ax::mojom::Role AXNodeObject::NativeRoleIgnoringAria() const {
     return ax::mojom::Role::kUnknown;
   }
 
+  // Chrome exposes both table markup and table CSS as a tables, letting
+  // the screen reader determine what to do for CSS tables.
+  if (IsHTMLTableElement(*GetNode())) {
+    return IsDataTable() ? ax::mojom::Role::kTable
+                         : ax::mojom::Role::kLayoutTable;
+  }
+  if (IsHTMLTableRowElement(*GetNode()))
+    return DetermineTableRowRole();
+  if (IsHTMLTableCellElement(*GetNode()))
+    return DetermineTableCellRole();
+
   if (const auto* input = ToHTMLInputElementOrNull(*GetNode())) {
     const AtomicString& type = input->type();
     if (input->DataList() && type != input_type_names::kColor)

third_party/blink/renderer/modules/accessibility/ax_node_object.h

@@ -59,6 +59,8 @@ class MODULES_EXPORT AXNodeObject : public AXObject {
       IgnoredReasons* = nullptr) const;
   bool ComputeAccessibilityIsIgnored(IgnoredReasons* = nullptr) const override;
   const AXObject* InheritsPresentationalRoleFrom() const override;
+  ax::mojom::Role DetermineTableCellRole() const;
+  ax::mojom::Role DetermineTableRowRole() const;
   ax::mojom::Role DetermineAccessibilityRole() override;
   virtual ax::mojom::Role NativeRoleIgnoringAria() const;
   void AlterSliderOrSpinButtonValue(bool increase);