Use DOM/style interfaces to determine if table is for data/layout

Currently table layout interfaces are used in IsDataTable(), but
unfortunately these interfaces will not work if a table is styled using
another display type, e.g. display: block / flex, etc.

This CL does not change how ordinary <table> elements are exposed.
It only makes the display style irrelevant in how a table is exposed.

Bug: 1011067

Change-Id: I2044015e1ec1345a9bc636f867526c7b75a78041
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2083602
Auto-Submit: Aaron Leventhal <aleventhal@chromium.org>
Reviewed-by: Dominic Mazzoni <dmazzoni@chromium.org>
Commit-Queue: Aaron Leventhal <aleventhal@chromium.org>
Cr-Commit-Position: refs/heads/master@{#747428}

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

@@ -2464,47 +2464,29 @@ bool AXLayoutObject::IsDataTable() const {
   if (Traversal<HTMLTableColElement>::FirstChild(*table_element))
     return true;
 
-  // Everything from here forward uses cell style info, but only if a CSS table.
-  // If this code is reached for a <table> with another display type, consider
-  // this to be a layout table.
-  // TODO(accessibility) consider rewriting the following cell inspection code
-  // using purely DOM methods, such as table->rows()->Item(index)->cells().
-  if (!layout_object_->IsTable())
-    return false;
-
-  // If there's no node, it's definitely a layout table. This happens
-  // when table CSS styles are used without a complete table DOM structure.
-  LayoutNGTableInterface* table =
-      ToInterface<LayoutNGTableInterface>(layout_object_);
-  table->RecalcSectionsIfNeeded();
-  Node* table_node = layout_object_->GetNode();
-  if (!table_node)
-    return false;
-
-  // go through the cell's and check for tell-tale signs of "data" table status
-  // cells have borders, or use attributes like headers, abbr, scope or axis
-  table->RecalcSectionsIfNeeded();
-  LayoutNGTableSectionInterface* first_body = table->FirstBodyInterface();
-  if (!first_body)
+  // If there are at least 20 rows, we'll call it a data table.
+  HTMLTableRowsCollection* rows = table_element->rows();
+  int num_rows = rows->length();
+  if (num_rows >= 20)
+    return true;
+  if (num_rows <= 0)
     return false;
 
-  int num_cols_in_first_body = first_body->NumEffectiveColumns();
-  int num_rows = first_body->NumRows();
+  int num_cols_in_first_body = rows->Item(0)->cells()->length();
   // If there's only one cell, it's not a good AXTable candidate.
   if (num_rows == 1 && num_cols_in_first_body == 1)
     return false;
 
-  // If there are at least 20 rows, we'll call it a data table.
-  if (num_rows >= 20)
-    return true;
-
   // Store the background color of the table to check against cell's background
   // colors.
-  const ComputedStyle* table_style = table->ToLayoutObject()->Style();
+  const ComputedStyle* table_style = layout_object_->Style();
   if (!table_style)
     return false;
+
   Color table_bg_color =
       table_style->VisitedDependentColor(GetCSSPropertyBackgroundColor());
+  bool has_cell_spacing = table_style->HorizontalBorderSpacing() &&
+                          table_style->VerticalBorderSpacing();
 
   // check enough of the cells to find if the table matches our criteria
   // Criteria:
@@ -2523,38 +2505,37 @@ bool AXLayoutObject::IsDataTable() const {
   Color alternating_row_colors[5];
   int alternating_row_color_count = 0;
   for (int row = 0; row < num_rows; ++row) {
-    int n_cols = first_body->NumCols(row);
+    HTMLTableRowElement* row_element = rows->Item(row);
+    int n_cols = row_element->cells()->length();
     for (int col = 0; col < n_cols; ++col) {
-      const LayoutNGTableCellInterface* cell =
-          first_body->PrimaryCellInterfaceAt(row, col);
+      const Element* cell = row_element->cells()->item(col);
       if (!cell)
         continue;
-      const LayoutBlock* cell_layout_block =
-          To<LayoutBlock>(cell->ToLayoutObject());
-      Node* cell_node = cell_layout_block->GetNode();
-      if (!cell_node)
+      // Any <th> tag -> treat as data table.
+      if (cell->HasTagName(html_names::kThTag))
+        return true;
+
+      // Check for an explicitly assigned a "data" table attribute.
+      auto* cell_elem = DynamicTo<HTMLTableCellElement>(*cell);
+      if (cell_elem) {
+        if (!cell_elem->Headers().IsEmpty() || !cell_elem->Abbr().IsEmpty() ||
+            !cell_elem->Axis().IsEmpty() ||
+            !cell_elem->FastGetAttribute(html_names::kScopeAttr).IsEmpty())
+          return true;
+      }
+
+      LayoutObject* cell_layout_object = cell->GetLayoutObject();
+      if (!cell_layout_object)
         continue;
 
+      const LayoutBlock* cell_layout_block =
+          To<LayoutBlock>(cell_layout_object);
       if (cell_layout_block->Size().Width() < 1 ||
           cell_layout_block->Size().Height() < 1)
         continue;
 
       valid_cell_count++;
 
-      // Any <th> tag -> treat as data table.
-      if (cell_node->HasTagName(html_names::kThTag))
-        return true;
-
-      // In this case, the developer explicitly assigned a "data" table
-      // attribute.
-      if (auto* cell_element = DynamicTo<HTMLTableCellElement>(*cell_node)) {
-        if (!cell_element->Headers().IsEmpty() ||
-            !cell_element->Abbr().IsEmpty() ||
-            !cell_element->Axis().IsEmpty() ||
-            !cell_element->FastGetAttribute(html_names::kScopeAttr).IsEmpty())
-          return true;
-      }
-
       const ComputedStyle* computed_style = cell_layout_block->Style();
       if (!computed_style)
         continue;
@@ -2586,8 +2567,8 @@ bool AXLayoutObject::IsDataTable() const {
       // the place of borders).
       Color cell_color = computed_style->VisitedDependentColor(
           GetCSSPropertyBackgroundColor());
-      if (table->HBorderSpacing() > 0 && table->VBorderSpacing() > 0 &&
-          table_bg_color != cell_color && cell_color.Alpha() != 1)
+      if (has_cell_spacing && table_bg_color != cell_color &&
+          cell_color.Alpha() != 1)
         background_difference_cell_count++;
 
       // If we've found 10 "good" cells, we don't need to keep searching.

third_party/blink/web_tests/accessibility/table-detection-display-flex-expected.txt

+
+
+This should be a table because it has a thead.
+
+AXRole: AXTable
+
+
+asdf	asdf
+asdf	asdf
+
+
+This should be a table because cells have borders.
+
+AXRole: AXTable
+
+
+asdf	asdf
+
+
+This should not be a table because its cells do not have borders.
+
+AXRole: AXLayoutTable
+
+
+asdf	asdf
+
+
+This should be a table because a cell has a special attribute
+
+AXRole: AXTable
+
+
+asdf	asdf
+
+
+This should be a table because a cell has a special attribute.
+
+AXRole: AXTable
+
+
+asdf	asdf
+
+
+This should be a table because a cell has a special attribute.
+
+AXRole: AXTable
+
+
+asdf	asdf
+asdf	asdf
+
+
+This should be a table because cells have different colors.
+
+AXRole: AXTable
+
+
+asdf	asdf
+
+
+This should not be a table because cells have different but no spacing.
+
+AXRole: AXLayoutTable
+
+
+asdf	asdf
+
+
+This should not be a table because cells have the same colors even though there is spacing.
+
+AXRole: AXLayoutTable
+
+
+asdf	asdf
+
+
+This should be a table because it has the "rules" attr.
+
+AXRole: AXTable
+
+
+asdf	asdf
+
+
+This should not be a table because it only has one valid cell (need more than one).
+
+AXRole: AXLayoutTable
+
+
+Contributions
+
+
+This should not be a table because it does not have enough cell borders or background colors
+
+AXRole: AXLayoutTable
+
+
+Politics
+Decision '08
+The debates
+The White House
+Capitol Hill
+National Journal
+New York Times
+
+
+This should be a table because it's editable.
+
+AXRole: AXTable
+
+
+asdf	asdf
+
+
+This should be a table because most cells have a top border.
+
+AXRole: AXTable
+
+
+asdf	asdf
+asdf	asdf
+
+
+This should not be a table because cells have different borders.
+
+AXRole: AXLayoutTable
+
+
+asdf	asdf
+asdf	asdf
+
+
+This should be a table because it sets empty-cells: hide on the table.
+
+AXRole: AXTable
+
+
+asdf	asdf
+	asdf
+asdf	asdf
+
+
+This should be a table because it sets empty-cells: hide on a cell.
+
+AXRole: AXTable
+
+
+asdf	asdf
+	asdf
+asdf	asdf
+
+
+This should be a table because it has a col.
+
+AXRole: AXTable
+
+
+asdf	asdf
+asdf	asdf
+
+
+This should be a table because it has at least 20 rows
+
+AXRole: AXTable
+
+
+asdf
+asdf
+asdf
+asdf
+asdf
+asdf
+asdf
+asdf
+asdf
+asdf
+asdf
+asdf
+asdf
+asdf
+asdf
+asdf
+asdf
+asdf
+asdf
+asdf
+asdf
+asdf
+asdf
+asdf