Bug 1492393 - Make our table index methods aware of cells spanning multiple columns, r=surkov
authorMarco Zehe <mzehe@mozilla.com>
Fri, 19 Oct 2018 08:18:04 +0000
changeset 442128 295644de04d2b1abbccd102473cbf3023dfce21a
parent 442105 1030155f208e08a4a4673655c8b6b343c1b920a1
child 442129 fbb784c460e5fdc034e1e734fe5d5cfcd3c18c33
push id109116
push userdvarga@mozilla.com
push dateSat, 20 Oct 2018 10:33:59 +0000
treeherdermozilla-inbound@8784cdbf16d3 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssurkov
bugs1492393
milestone64.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1492393 - Make our table index methods aware of cells spanning multiple columns, r=surkov Sometimes, when cells have display:block, and a different cell in the same row has a column span, our index methods did not take these into account. Also, when regular tables encounter such a cell, index calculation failed. Differential Revision: https://phabricator.services.mozilla.com/D7867
accessible/generic/ARIAGridAccessible.cpp
accessible/generic/TableAccessible.cpp
accessible/html/HTMLTableAccessible.cpp
accessible/tests/mochitest/table/test_indexes_table.html
--- a/accessible/generic/ARIAGridAccessible.cpp
+++ b/accessible/generic/ARIAGridAccessible.cpp
@@ -65,18 +65,20 @@ ARIAGridAccessible::ColCount() const
   Accessible* row = rowIter.Next();
   if (!row)
     return 0;
 
   AccIterator cellIter(row, filters::GetCell);
   Accessible* cell = nullptr;
 
   uint32_t colCount = 0;
-  while ((cell = cellIter.Next()))
-    colCount++;
+  while ((cell = cellIter.Next())) {
+    MOZ_ASSERT(cell->IsTableCell(), "No table or grid cell!");
+    colCount += cell->AsTableCell()->ColExtent();
+  }
 
   return colCount;
 }
 
 uint32_t
 ARIAGridAccessible::RowCount()
 {
   uint32_t rowCount = 0;
@@ -597,20 +599,19 @@ ARIAGridCellAccessible::ColIdx() const
   Accessible* row = Row();
   if (!row)
     return 0;
 
   int32_t indexInRow = IndexInParent();
   uint32_t colIdx = 0;
   for (int32_t idx = 0; idx < indexInRow; idx++) {
     Accessible* cell = row->GetChildAt(idx);
-    roles::Role role = cell->Role();
-    if (role == roles::CELL || role == roles::GRID_CELL ||
-        role == roles::ROWHEADER || role == roles::COLUMNHEADER)
-      colIdx++;
+    if (cell->IsTableCell()) {
+      colIdx += cell->AsTableCell()->ColExtent();
+    }
   }
 
   return colIdx;
 }
 
 uint32_t
 ARIAGridCellAccessible::RowIdx() const
 {
--- a/accessible/generic/TableAccessible.cpp
+++ b/accessible/generic/TableAccessible.cpp
@@ -256,15 +256,17 @@ TableAccessible::RowAt(int32_t aRow)
 }
 
 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--;
+  Accessible* cell = nullptr;
+  
+  while (colIdx >= 0 && (cell = cellIter.Next())) {
+    MOZ_ASSERT(cell->IsTableCell(), "No table or grid cell!");
+    colIdx -= cell->AsTableCell()->ColExtent();
   }
 
   return cell;
 }
--- a/accessible/html/HTMLTableAccessible.cpp
+++ b/accessible/html/HTMLTableAccessible.cpp
@@ -633,60 +633,104 @@ HTMLTableAccessible::CellAt(uint32_t aRo
 
 int32_t
 HTMLTableAccessible::CellIndexAt(uint32_t aRowIdx, uint32_t aColIdx)
 {
   nsTableWrapperFrame* tableFrame = do_QueryFrame(mContent->GetPrimaryFrame());
   if (!tableFrame)
     return -1;
 
-  return tableFrame->GetIndexByRowAndColumn(aRowIdx, aColIdx);
+  int32_t cellIndex = tableFrame->GetIndexByRowAndColumn(aRowIdx, aColIdx);
+  if (cellIndex == -1) {
+    // 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.
+    nsIContent* cellContent = tableFrame->GetCellAt(aRowIdx, aColIdx);
+    Accessible* cell = mDoc->GetAccessible(cellContent);
+    if (cell && cell->IsTableRow()) {
+      return TableAccessible::CellIndexAt(aRowIdx, aColIdx);
+    }
+  }
+
+  return cellIndex;
 }
 
 int32_t
 HTMLTableAccessible::ColIndexAt(uint32_t aCellIdx)
 {
   nsTableWrapperFrame* tableFrame = do_QueryFrame(mContent->GetPrimaryFrame());
   if (!tableFrame)
     return -1;
 
   int32_t rowIdx = -1, colIdx = -1;
   tableFrame->GetRowAndColumnByIndex(aCellIdx, &rowIdx, &colIdx);
+
+  if (colIdx == -1) {
+    // Sometimes, the index returned indicates that this is not a regular
+    // cell, for example when a cell has CSS display:block; set.
+    // In such cases, try the super class method to find it.
+    return TableAccessible::ColIndexAt(aCellIdx);
+  }
+
   return colIdx;
 }
 
 int32_t
 HTMLTableAccessible::RowIndexAt(uint32_t aCellIdx)
 {
   nsTableWrapperFrame* tableFrame = do_QueryFrame(mContent->GetPrimaryFrame());
   if (!tableFrame)
     return -1;
 
   int32_t rowIdx = -1, colIdx = -1;
   tableFrame->GetRowAndColumnByIndex(aCellIdx, &rowIdx, &colIdx);
+
+  if (rowIdx == -1) {
+    // Sometimes, the index returned indicates that this is not a regular
+    // cell, for example when a cell has CSS display:block; set.
+    // In such cases, try the super class method to find it.
+    return TableAccessible::RowIndexAt(aCellIdx);
+  }
+
   return rowIdx;
 }
 
 void
 HTMLTableAccessible::RowAndColIndicesAt(uint32_t aCellIdx, int32_t* aRowIdx,
                                         int32_t* aColIdx)
 {
   nsTableWrapperFrame* tableFrame = do_QueryFrame(mContent->GetPrimaryFrame());
-  if (tableFrame)
+  if (tableFrame) {
     tableFrame->GetRowAndColumnByIndex(aCellIdx, aRowIdx, aColIdx);
+    if (*aRowIdx == -1 || *aColIdx == -1) {
+      // Sometimes, the index returned indicates that this is not a regular
+      // cell, for example when a cell has CSS display:block; set.
+      // In such cases, try the super class method to find it.
+      TableAccessible::RowAndColIndicesAt(aCellIdx, aRowIdx, aColIdx);
+    }
+  }
 }
 
 uint32_t
 HTMLTableAccessible::ColExtentAt(uint32_t aRowIdx, uint32_t aColIdx)
 {
   nsTableWrapperFrame* tableFrame = do_QueryFrame(mContent->GetPrimaryFrame());
   if (!tableFrame)
     return 0;
 
-  return tableFrame->GetEffectiveColSpanAt(aRowIdx, aColIdx);
+  uint32_t colExtent = tableFrame->GetEffectiveColSpanAt(aRowIdx, aColIdx);
+  if (colExtent == 0) {
+    nsIContent* cellContent = tableFrame->GetCellAt(aRowIdx, aColIdx);
+    Accessible* cell = mDoc->GetAccessible(cellContent);
+    if (cell && cell->IsTableRow()) {
+      return TableAccessible::ColExtentAt(aRowIdx, aColIdx);
+    }
+  }
+
+  return colExtent;
 }
 
 uint32_t
 HTMLTableAccessible::RowExtentAt(uint32_t aRowIdx, uint32_t aColIdx)
 {
   nsTableWrapperFrame* tableFrame = do_QueryFrame(mContent->GetPrimaryFrame());
   if (!tableFrame)
     return 0;
--- a/accessible/tests/mochitest/table/test_indexes_table.html
+++ b/accessible/tests/mochitest/table/test_indexes_table.html
@@ -129,16 +129,24 @@ https://bugzilla.mozilla.org/show_bug.cg
 
       // ////////////////////////////////////////////////////////////////////////
       // Table with a cell that has display: block; style
       idxes = [
         [0, 1]
       ];
       testTableIndexes("tablewithcelldisplayblock", idxes);
 
+      // ////////////////////////////////////////////////////////////////////////
+      // A table with a cell that has display: block; and a cell with colspan.
+      // This makes us fall back to the ARIAGridCellAccessible implementation.
+      idxes = [
+        [0, 0, 1]
+      ];
+      testTableIndexes("tablewithcolspanandcelldisplayblock", idxes);
+
       SimpleTest.finish();
     }
 
     SimpleTest.waitForExplicitFinish();
     addA11yLoadEvent(doTest);
   </script>
 </head>
 <body>
@@ -414,10 +422,17 @@ https://bugzilla.mozilla.org/show_bug.cg
 
   <table id="tablewithcelldisplayblock">
     <tr>
       <th>a</th>
       <td style="display: block;">b</td>
     </tr>
   </table>
 
+  <table id="tablewithcolspanandcelldisplayblock">
+    <tr>
+      <th colspan="2">a</th>
+      <td style="display: block;" >b</td>
+    </tr>
+  </table>
+
 </body>
 </html>