Bug 1486668 - <td> with display: block doesn't get table cell semantics, r=surkov

Differential Revision: https://phabricator.services.mozilla.com/D6158

--HG--
extra : moz-landing-system : lando

accessible/base/MarkupMap.h

@@ -358,7 +358,10 @@ MARKUPMAP(
        // display style other than 'table', then create a generic table cell
        // accessible, because there's no underlying table layout and thus native
        // HTML table cell class doesn't work.
-       if (!aContext->IsHTMLTableRow()) {
+       // The same is true if the cell itself has CSS display:block;.
+       if (!aContext->IsHTMLTableRow() ||
+           (aElement->GetPrimaryFrame() &&
+            aElement->GetPrimaryFrame()->AccessibleType() != eHTMLTableCellType)) {
          return new ARIAGridCellAccessibleWrap(aElement, aContext->Document());
        }
        if (aElement->HasAttr(kNameSpaceID_None, nsGkAtoms::scope)) {

accessible/generic/ARIAGridAccessible.cpp

@@ -90,11 +90,11 @@ ARIAGridAccessible::RowCount()
 Accessible*
 ARIAGridAccessible::CellAt(uint32_t aRowIndex, uint32_t aColumnIndex)
 {
-  Accessible* row = GetRowAt(aRowIndex);
+  Accessible* row = RowAt(aRowIndex);
   if (!row)
     return nullptr;
 
-  return GetCellInRowAt(row, aColumnIndex);
+  return CellInRowAt(row, aColumnIndex);
 }
 
 bool
@@ -110,7 +110,7 @@ ARIAGridAccessible::IsColSelected(uint32_t aColIdx)
 
   do {
     if (!nsAccUtils::IsARIASelected(row)) {
-      Accessible* cell = GetCellInRowAt(row, aColIdx);
+      Accessible* cell = CellInRowAt(row, aColIdx);
       if (!cell || !nsAccUtils::IsARIASelected(cell))
         return false;
     }
@@ -125,7 +125,7 @@ ARIAGridAccessible::IsRowSelected(uint32_t aRowIdx)
   if (IsARIARole(nsGkAtoms::table))
     return false;
 
-  Accessible* row = GetRowAt(aRowIdx);
+  Accessible* row = RowAt(aRowIdx);
   if(!row)
     return false;
 
@@ -147,12 +147,12 @@ ARIAGridAccessible::IsCellSelected(uint32_t aRowIdx, uint32_t aColIdx)
   if (IsARIARole(nsGkAtoms::table))
     return false;
 
-  Accessible* row = GetRowAt(aRowIdx);
+  Accessible* row = RowAt(aRowIdx);
   if(!row)
     return false;
 
   if (!nsAccUtils::IsARIASelected(row)) {
-    Accessible* cell = GetCellInRowAt(row, aColIdx);
+    Accessible* cell = CellInRowAt(row, aColIdx);
     if (!cell || !nsAccUtils::IsARIASelected(cell))
       return false;
   }
@@ -416,7 +416,7 @@ ARIAGridAccessible::SelectCol(uint32_t aColIdx)
     NS_ASSERTION(NS_SUCCEEDED(rv), "SetARIASelected() Shouldn't fail!");
 
     // Select cell at the column index.
-    Accessible* cell = GetCellInRowAt(row, aColIdx);
+    Accessible* cell = CellInRowAt(row, aColIdx);
     if (cell)
       SetARIASelected(cell, true);
   }
@@ -428,7 +428,7 @@ ARIAGridAccessible::UnselectRow(uint32_t aRowIdx)
   if (IsARIARole(nsGkAtoms::table))
     return;
 
-  Accessible* row = GetRowAt(aRowIdx);
+  Accessible* row = RowAt(aRowIdx);
   if (row)
     SetARIASelected(row, false);
 }
@@ -443,7 +443,7 @@ ARIAGridAccessible::UnselectCol(uint32_t aColIdx)
 
   Accessible* row = nullptr;
   while ((row = rowIter.Next())) {
-    Accessible* cell = GetCellInRowAt(row, aColIdx);
+    Accessible* cell = CellInRowAt(row, aColIdx);
     if (cell)
       SetARIASelected(cell, false);
   }
@@ -452,33 +452,6 @@ ARIAGridAccessible::UnselectCol(uint32_t aColIdx)
 ////////////////////////////////////////////////////////////////////////////////
 // Protected
 
-Accessible*
-ARIAGridAccessible::GetRowAt(int32_t aRow)
-{
-  int32_t rowIdx = aRow;
-
-  AccIterator rowIter(this, filters::GetRow);
-
-  Accessible* row = rowIter.Next();
-  while (rowIdx != 0 && (row = rowIter.Next()))
-    rowIdx--;
-
-  return row;
-}
-
-Accessible*
-ARIAGridAccessible::GetCellInRowAt(Accessible* aRow, int32_t aColumn)
-{
-  int32_t colIdx = aColumn;
-
-  AccIterator cellIter(aRow, filters::GetCell);
-  Accessible* cell = cellIter.Next();
-  while (colIdx != 0 && (cell = cellIter.Next()))
-    colIdx--;
-
-  return cell;
-}
-
 nsresult
 ARIAGridAccessible::SetARIASelected(Accessible* aAccessible,
                                     bool aIsSelected, bool aNotify)

accessible/generic/ARIAGridAccessible.h

@@ -52,16 +52,6 @@ public:
 protected:
   virtual ~ARIAGridAccessible() {}
 
-  /**
-   * Return row accessible at the given row index.
-   */
-  Accessible* GetRowAt(int32_t aRow);
-
-  /**
-   * Return cell accessible at the given column index in the row.
-   */
-  Accessible* GetCellInRowAt(Accessible* aRow, int32_t aColumn);
-
   /**
    * Set aria-selected attribute value on DOM node of the given accessible.
    *

accessible/generic/TableAccessible.cpp

@@ -7,6 +7,7 @@
 #include "TableAccessible.h"
 
 #include "Accessible-inl.h"
+#include "AccIterator.h"
 
 #include "nsTableCellFrame.h"
 #include "nsTableWrapperFrame.h"
@@ -238,3 +239,32 @@ TableAccessible::IsProbablyLayoutTable()
     false, "No layout factor strong enough, so will guess data"
   );
 }
+
+Accessible*
+TableAccessible::RowAt(int32_t aRow)
+{
+  int32_t rowIdx = aRow;
+
+  AccIterator rowIter(this->AsAccessible(), filters::GetRow);
+
+  Accessible* row = rowIter.Next();
+  while (rowIdx != 0 && (row = rowIter.Next())) {
+    rowIdx--;
+  }
+
+  return row;
+}
+
+Accessible*
+TableAccessible::CellInRowAt(Accessible* aRow, int32_t aColumn)
+{
+  int32_t colIdx = aColumn;
+
+  AccIterator cellIter(aRow, filters::GetCell);
+  Accessible* cell = cellIter.Next();
+  while (colIdx != 0 && (cell = cellIter.Next())) {
+    colIdx--;
+  }
+
+  return cell;
+}

accessible/generic/TableAccessible.h

@@ -179,6 +179,18 @@ public:
    * Convert the table to an Accessible*.
    */
   virtual Accessible* AsAccessible() = 0;
+
+protected:
+
+  /**
+   * Return row accessible at the given row index.
+   */
+  Accessible* RowAt(int32_t aRow);
+
+  /**
+   * Return cell accessible at the given column index in the row.
+   */
+  Accessible* CellInRowAt(Accessible* aRow, int32_t aColumn);
 };
 
 } // namespace a11y

accessible/html/HTMLTableAccessible.cpp

@@ -619,6 +619,14 @@ HTMLTableAccessible::CellAt(uint32_t aRowIdx, uint32_t aColIdx)
   nsIContent* cellContent = tableFrame->GetCellAt(aRowIdx, aColIdx);
   Accessible* cell = mDoc->GetAccessible(cellContent);
 
+  // Sometimes, the accessible returned here is a row accessible instead of
+  // a cell accessible, for example when a cell has CSS display:block; set.
+  // In such cases, iterate through the cells in this row differently to find it.
+  if (cell && cell->IsTableRow()) {
+    Accessible* row = RowAt(aRowIdx);
+    return CellInRowAt(row, aColIdx);
+  }
+
   // XXX bug 576838: crazy tables (like table6 in tables/test_table2.html) may
   // return itself as a cell what makes Orca hang.
   return cell == this ? nullptr : cell;
@@ -762,6 +770,9 @@ HTMLTableAccessible::UnselectCol(uint32_t aColIdx)
   RemoveRowsOrColumnsFromSelection(aColIdx, TableSelection::Column, false);
 }
 
+////////////////////////////////////////////////////////////////////////////////
+// HTMLTableAccessible: protected implementation
+
 nsresult
 HTMLTableAccessible::AddRowOrColumnToSelection(int32_t aIndex, TableSelection aTarget)
 {

accessible/tests/mochitest/table/test_indexes_table.html

@@ -127,6 +127,13 @@ https://bugzilla.mozilla.org/show_bug.cgi?id=410052
       ];
       testTableIndexes("tableinsane6", idxes);
 
+      // ////////////////////////////////////////////////////////////////////////
+      // Table with a cell that has display: block; style
+      idxes = [
+        [0, 1]
+      ];
+      testTableIndexes("tablewithcelldisplayblock", idxes);
+
       SimpleTest.finish();
     }
 
@@ -405,5 +412,12 @@ https://bugzilla.mozilla.org/show_bug.cgi?id=410052
     </tbody>
   </table>
 
+  <table id="tablewithcelldisplayblock">
+    <tr>
+      <th>a</th>
+      <td style="display: block;">b</td>
+    </tr>
+  </table>
+
 </body>
 </html>

accessible/tests/mochitest/table/test_struct_table.html

@@ -66,6 +66,15 @@
 
       testTableStruct("table4", cellsArray);
 
+      // ////////////////////////////////////////////////////////////////////////
+      // Table with a cell that has display: block; style
+
+      cellsArray = [
+        [kRowHeaderCell, kDataCell]
+      ];
+
+      testTableStruct("table5", cellsArray);
+
       SimpleTest.finish();
     }
 
@@ -198,5 +207,12 @@
     </tbody>
   </table>
 
+  <table id="table5">
+    <tr>
+      <th>a</th>
+      <td style="display: block;">b</td>
+    </tr>
+  </table>
+
 </body>
 </html>

accessible/tests/mochitest/tree/test_table.html

@@ -218,6 +218,21 @@
         ] };
       testAccessibleTree("table_containing_inlinetable", accTree);
 
+      // ///////////////////////////////////////////////////////////////////////
+      // table with a cell that has display:block
+      accTree =
+        { TABLE: [
+          { ROW: [
+            { CELL: [
+              { TEXT_LEAF: [ ] }
+            ] },
+            { CELL: [
+              { TEXT_LEAF: [ ] }
+            ] }
+          ] }
+        ] };
+      testAccessibleTree("table_containing_block_cell", accTree);
+
       SimpleTest.finish();
     }
 
@@ -343,5 +358,12 @@
        </tr>
      </table>
    </td></tr></table>
+
+  <table id="table_containing_block_cell">
+    <tr>
+      <td>Normal cell</td>
+      <td style="display: block;">Block cell</td>
+    </tr>
+  </table>
 </body>
 </html>