Bug 1461244 - Take into account row groups when creating ARIAGridRowAccessibles, r=Jamie a=lizzard
authorMarco Zehe <mzehe@mozilla.com>
Mon, 04 Feb 2019 06:17:33 +0000
changeset 515956 a854fbac4d464b76780eea790ef5cee0fb28432a
parent 515955 177c9f1a39c7f159ce08a440e226af6f07bc8309
child 515957 b9933573c6a8d5f14dcba178603e82d054fec936
push id1953
push userffxbld-merge
push dateMon, 11 Mar 2019 12:10:20 +0000
treeherdermozilla-release@9c35dcbaa899 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersJamie, lizzard
bugs1461244
milestone66.0
Bug 1461244 - Take into account row groups when creating ARIAGridRowAccessibles, r=Jamie a=lizzard If all parts of a table are non-standard display types, like all elements being display:block;, we weren't properly determining table cell indices because we weren't always taking into account thead, tbody, or tfoot elements. This patch: * Exposes non-standard tbody, tfoot and thead elements as groupings, similar to ARIA rowgroup. * Adjusts the one instance in nsAccessibilityService::CreateAccessible that didn't account for the table not being the direct parent of the row node, but the grandparent instead. Differential Revision: https://phabricator.services.mozilla.com/D18333
accessible/base/MarkupMap.h
accessible/base/nsAccessibilityService.cpp
accessible/tests/mochitest/table/test_indexes_table.html
accessible/tests/mochitest/tree/test_table.html
accessible/tests/mochitest/tree/test_table_3.html
--- a/accessible/base/MarkupMap.h
+++ b/accessible/base/MarkupMap.h
@@ -346,16 +346,27 @@ MARKUPMAP(table,
             }
             return nullptr;
           },
           0)
 
 MARKUPMAP(time, New_HyperText, 0, Attr(xmlroles, time),
           AttrFromDOM(datetime, datetime))
 
+MARKUPMAP(tbody,
+          [](Element* aElement, Accessible* aContext) -> Accessible* {
+            // Expose this as a grouping if its frame type is non-standard.
+            if (aElement->GetPrimaryFrame() &&
+                aElement->GetPrimaryFrame()->IsTableRowGroupFrame()) {
+              return nullptr;
+            }
+            return new HyperTextAccessibleWrap(aElement, aContext->Document());
+          },
+          roles::GROUPING)
+
 MARKUPMAP(td,
           [](Element* aElement, Accessible* aContext) -> Accessible* {
             if (aContext->IsTableRow() &&
                 aContext->GetContent() == aElement->GetParent()) {
               // If HTML:td element is part of its HTML:table, which has CSS
               // 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. The same is
@@ -371,31 +382,53 @@ MARKUPMAP(td,
                 return new HTMLTableHeaderCellAccessibleWrap(
                     aElement, aContext->Document());
               }
             }
             return nullptr;
           },
           0)
 
+MARKUPMAP(tfoot,
+          [](Element* aElement, Accessible* aContext) -> Accessible* {
+            // Expose this as a grouping if its frame type is non-standard.
+            if (aElement->GetPrimaryFrame() &&
+                aElement->GetPrimaryFrame()->IsTableRowGroupFrame()) {
+              return nullptr;
+            }
+            return new HyperTextAccessibleWrap(aElement, aContext->Document());
+          },
+          roles::GROUPING)
+
 MARKUPMAP(th,
           [](Element* aElement, Accessible* aContext) -> Accessible* {
             if (aContext->IsTableRow() &&
                 aContext->GetContent() == aElement->GetParent()) {
               if (!aContext->IsHTMLTableRow()) {
                 return new ARIAGridCellAccessibleWrap(aElement,
                                                       aContext->Document());
               }
               return new HTMLTableHeaderCellAccessibleWrap(
                   aElement, aContext->Document());
             }
             return nullptr;
           },
           0)
 
+MARKUPMAP(tfoot,
+          [](Element* aElement, Accessible* aContext) -> Accessible* {
+            // Expose this as a grouping if its frame type is non-standard.
+            if (aElement->GetPrimaryFrame() &&
+                aElement->GetPrimaryFrame()->IsTableRowGroupFrame()) {
+              return nullptr;
+            }
+            return new HyperTextAccessibleWrap(aElement, aContext->Document());
+          },
+          roles::GROUPING)
+
 MARKUPMAP(tr,
           [](Element* aElement, Accessible* aContext) -> Accessible* {
             // If HTML:tr element is part of its HTML:table, which has CSS
             // display style other than 'table', then create a generic table row
             // accessible, because there's no underlying table layout and thus
             // native HTML table row class doesn't work. Refer to
             // CreateAccessibleByFrameType dual logic.
             Accessible* table = aContext->IsTable() ? aContext : nullptr;
--- a/accessible/base/nsAccessibilityService.cpp
+++ b/accessible/base/nsAccessibilityService.cpp
@@ -1036,18 +1036,20 @@ Accessible* nsAccessibilityService::Crea
     // In case of ARIA grid or table use table-specific classes if it's not
     // native table based.
     if (isARIATablePart && (!newAcc || newAcc->IsGenericHyperText())) {
       if ((roleMapEntry->accTypes & eTableCell)) {
         if (aContext->IsTableRow())
           newAcc = new ARIAGridCellAccessibleWrap(content, document);
 
       } else if (roleMapEntry->IsOfType(eTableRow)) {
-        if (aContext->IsTable())
+        if (aContext->IsTable() ||
+            (aContext->Parent() && aContext->Parent()->IsTable())) {
           newAcc = new ARIARowAccessible(content, document);
+        }
 
       } else if (roleMapEntry->IsOfType(eTable)) {
         newAcc = new ARIAGridAccessibleWrap(content, document);
       }
     }
 
     // If table has strong ARIA role then all table descendants shouldn't
     // expose their native roles.
--- a/accessible/tests/mochitest/table/test_indexes_table.html
+++ b/accessible/tests/mochitest/table/test_indexes_table.html
@@ -137,16 +137,25 @@ https://bugzilla.mozilla.org/show_bug.cg
       // ////////////////////////////////////////////////////////////////////////
       // 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);
 
+      // ////////////////////////////////////////////////////////////////////////
+      // A table with all elements being display:block, including a row group.
+      // This makes us fall back to the ARIAGridRowAccessible, and we must
+      // make sure the index is 0. Crazy example from Gmail.
+      idxes = [
+        [0],
+      ];
+      testTableIndexes("tablealldisplayblock", idxes);
+
       SimpleTest.finish();
     }
 
     SimpleTest.waitForExplicitFinish();
     addA11yLoadEvent(doTest);
   </script>
 </head>
 <body>
@@ -429,10 +438,18 @@ https://bugzilla.mozilla.org/show_bug.cg
 
   <table id="tablewithcolspanandcelldisplayblock">
     <tr>
       <th colspan="2">a</th>
       <td style="display: block;" >b</td>
     </tr>
   </table>
 
+  <table id="tablealldisplayblock" style="display:block;">
+    <tbody style="display:block;">
+      <tr style="display:block;">
+        <td style="display:block;">text</td>
+      </tr>
+    </tbody>
+  </table>
+  
 </body>
 </html>
--- a/accessible/tests/mochitest/tree/test_table.html
+++ b/accessible/tests/mochitest/tree/test_table.html
@@ -165,17 +165,17 @@
           ] } ],
         };
       testAccessibleTree("table4", accTree);
 
       // ///////////////////////////////////////////////////////////////////////
       // table5 (intermediate accessible for tbody)
       accTree =
         { TABLE: [
-          { TEXT_CONTAINER: [
+          { GROUPING: [
             { ROW: [
               { CELL: [
                 { TEXT_LEAF: [ ] },
               ] },
             ] },
           ] } ],
         };
       testAccessibleTree("table5", accTree);
@@ -247,16 +247,32 @@
             ] },
             { CELL: [
               { TEXT_LEAF: [ ] },
             ] },
           ] },
         ] };
       testAccessibleTree("table_containing_block_cell", accTree);
 
+      // ///////////////////////////////////////////////////////////////////////
+      // A table with all elements being display:block, including a row group.
+      // This makes us fall back to the ARIAGridRowAccessible.
+      // Crazy example from Gmail.
+      accTree =
+        { TABLE: [
+          { GROUPING: [
+            { ROW: [
+              { CELL: [
+                { TEXT_LEAF: [ ] },
+              ] },
+            ] },
+          ] },
+        ] };
+        testAccessibleTree("table_all_display_block", accTree);
+
       SimpleTest.finish();
     }
 
     SimpleTest.waitForExplicitFinish();
     addA11yLoadEvent(doTest);
   </script>
 </head>
 <body>
@@ -385,10 +401,17 @@
    </td></tr></table>
 
   <table id="table_containing_block_cell">
     <tr>
       <td>Normal cell</td>
       <td style="display: block;">Block cell</td>
     </tr>
   </table>
+  <table id="table_all_display_block" style="display:block;">
+    <tbody style="display:block;">
+      <tr style="display:block;">
+        <td style="display:block;">text</td>
+      </tr>
+    </tbody>
+  </table>
 </body>
 </html>
--- a/accessible/tests/mochitest/tree/test_table_3.html
+++ b/accessible/tests/mochitest/tree/test_table_3.html
@@ -101,17 +101,17 @@
 
 <script type="application/javascript">
 
 const COLHEADER = ROLE_COLUMNHEADER;
 const ROWHEADER = ROLE_ROWHEADER;
 const CELL = ROLE_CELL;
 const STATICTEXT = ROLE_STATICTEXT;
 const TEXT_LEAF = ROLE_TEXT_LEAF;
-const TEXT_CONTAINER = ROLE_TEXT_CONTAINER;
+const GROUPING = ROLE_GROUPING;
 
 function doTest() {
   let accTree =
     { TABLE: [
       { CAPTION: [
         {
           role: ROLE_TEXT_LEAF,
           name: "Top 10 Grossing Animated Films of All Time",
@@ -124,17 +124,17 @@ function doTest() {
         { CELL: [ { role: TEXT_LEAF, name: "Worldwide Gross" } ] },
         { CELL: [ { role: TEXT_LEAF, name: "Domestic Gross" } ] },
         { CELL: [ { role: TEXT_LEAF, name: "Foreign Gross" } ] },
         { CELL: [ { role: TEXT_LEAF, name: "Budget" } ] },
       ] },
       { ROW: [
         { role: CELL },
       ] },
-      { TEXT_CONTAINER: [
+      { GROUPING: [
         { ROW: [
           { CELL: [ { role: TEXT_LEAF, name: "Toy Story 3" } ] },
           { CELL: [
             { role: STATICTEXT, name: "Released" },
             { role: TEXT_LEAF, name: "2010" },
           ] },
           { CELL: [
             { role: STATICTEXT, name: "Studio" },