Backed out 5 changesets (bug 1726124, bug 1786086) for failures on br…

…owser_table.js. CLOSED TREE Backed out changeset 169c9576cb7b (bug 1786086) Backed out changeset 76dd155c66b8 (bug 1786086) Backed out changeset 25a5c81eded3 (bug 1786086) Backed out changeset 1c5f2f67382b (bug 1726124) Backed out changeset 8c82b5407ff3 (bug 1726124)
dothq · Aug 29, 2022 · 3840d81 · 3840d81
1 parent 960e452
commit 3840d81
Show file tree

Hide file tree

Showing 9 changed files with 77 additions and 67 deletions.
diff --git a/accessible/base/CachedTableAccessible.cpp b/accessible/base/CachedTableAccessible.cpp
@@ -72,8 +72,19 @@ void CachedTableAccessible::Invalidate(Accessible* aAcc) {
   if (!sCachedTables) {
     return;
   }
-
-  if (Accessible* table = nsAccUtils::TableFor(aAcc)) {
+  Accessible* table = nullptr;
+  if (aAcc->IsTable()) {
+    table = aAcc;
+  } else if (aAcc->IsTableCell()) {
+    for (table = aAcc->Parent(); table; table = table->Parent()) {
+      if (table->IsTable()) {
+        break;
+      }
+    }
+  } else {
+    MOZ_ASSERT_UNREACHABLE("Should only be called on a table or a cell");
+  }
+  if (table) {
     // Destroy the instance (if any). We'll create a new one the next time it
     // is requested.
     sCachedTables->Remove(table);

diff --git a/accessible/base/nsAccUtils.cpp b/accessible/base/nsAccUtils.cpp
@@ -185,19 +185,27 @@ bool nsAccUtils::IsDOMAttrTrue(const LocalAccessible* aAccessible,
                                eCaseMatters);
 }
 
-Accessible* nsAccUtils::TableFor(Accessible* aAcc) {
-  if (!aAcc ||
-      (!aAcc->IsTable() && !aAcc->IsTableRow() && !aAcc->IsTableCell())) {
-    return nullptr;
-  }
-  Accessible* table = aAcc;
-  for (; table && !table->IsTable(); table = table->Parent()) {
+Accessible* nsAccUtils::TableFor(Accessible* aRow) {
+  if (aRow) {
+    Accessible* table = aRow->Parent();
+    if (table) {
+      roles::Role tableRole = table->Role();
+      const nsRoleMapEntry* roleMapEntry = table->ARIARoleMap();
+      if (tableRole == roles::GROUPING ||  // if there's a rowgroup.
+          (table->IsGenericHyperText() && !roleMapEntry &&
+           !table->IsTable())) {  // or there is a wrapping text container
+        table = table->Parent();
+        if (table) tableRole = table->Role();
+      }
+
+      return (tableRole == roles::TABLE || tableRole == roles::TREE_TABLE ||
+              tableRole == roles::MATHML_TABLE)
+                 ? table
+                 : nullptr;
+    }
   }
-  // We don't assert (table && table->IsTable()) here because
-  // it's possible for this tree walk to yield no table at all
-  // ex. because a table part has been moved in the tree
-  // using aria-owns.
-  return table;
+
+  return nullptr;
 }
 
 LocalAccessible* nsAccUtils::TableFor(LocalAccessible* aRow) {

diff --git a/accessible/base/nsAccessibilityService.cpp b/accessible/base/nsAccessibilityService.cpp
@@ -567,11 +567,9 @@ void nsAccessibilityService::ContentRemoved(PresShell* aPresShell,
 void nsAccessibilityService::TableLayoutGuessMaybeChanged(
     PresShell* aPresShell, nsIContent* aContent) {
   if (DocAccessible* document = GetDocAccessible(aPresShell)) {
-    if (LocalAccessible* acc = document->GetAccessible(aContent)) {
-      LocalAccessible* table = nsAccUtils::TableFor(acc);
+    if (LocalAccessible* accessible = document->GetAccessible(aContent)) {
       document->FireDelayedEvent(
-          nsIAccessibleEvent::EVENT_TABLE_STYLING_CHANGED, table);
-      document->QueueCacheUpdate(table, CacheDomain::Table);
+          nsIAccessibleEvent::EVENT_TABLE_STYLING_CHANGED, accessible);
     }
   }
 }

diff --git a/accessible/generic/DocAccessible.cpp b/accessible/generic/DocAccessible.cpp
@@ -1318,13 +1318,21 @@ bool DocAccessible::PruneOrInsertSubtree(nsIContent* aRoot) {
 
     // If the accessible is a table, or table part, its layout table
     // status may have changed. We need to invalidate the associated
-    // table cache, which listens for the following event.
+    // cache, which listens for the following event.
     if (acc->IsTable() || acc->IsTableRow() || acc->IsTableCell()) {
-      LocalAccessible* table = nsAccUtils::TableFor(acc);
+      FireDelayedEvent(nsIAccessibleEvent::EVENT_TABLE_STYLING_CHANGED, acc);
+      LocalAccessible* table;
+      if (acc->IsTable()) {
+        table = acc;
+      } else {
+        for (table = acc->LocalParent(); table; table = table->LocalParent()) {
+          if (table->IsTable()) {
+            break;
+          }
+        }
+      }
       if (table && table->IsTable()) {
-        FireDelayedEvent(nsIAccessibleEvent::EVENT_TABLE_STYLING_CHANGED,
-                         table);
-        QueueCacheUpdate(table, CacheDomain::Table);
+        QueueCacheUpdate(acc, CacheDomain::Table);
       }
     }
 

diff --git a/accessible/generic/LocalAccessible.cpp b/accessible/generic/LocalAccessible.cpp
@@ -2489,6 +2489,14 @@ void LocalAccessible::BindToParent(LocalAccessible* aParent,
         table->GetHeaderCache().Clear();
       }
     }
+  } else if (IsTableRow() && aParent->IsTable() &&
+             StaticPrefs::accessibility_cache_enabled_AtStartup()) {
+    // This table might have previously been treated as a layout table. Now that
+    // a row has been added, it might have sufficient rows to be considered a
+    // data table. We do this here rather than when handling the reorder event
+    // because queuing a cache update once we start firing events means waiting
+    // for the next tick before the cache update is sent.
+    mDoc->QueueCacheUpdate(aParent, CacheDomain::Table);
   }
 }
 

diff --git a/accessible/mac/mozTableAccessible.mm b/accessible/mac/mozTableAccessible.mm
@@ -139,7 +139,8 @@ - (void)handleAccessibleEvent:(uint32_t)eventType {
     // invalidate the mIsLayoutTable cache on our parent
     // table.
     if (eventType == nsIAccessibleEvent::EVENT_REORDER ||
-        eventType == nsIAccessibleEvent::EVENT_OBJECT_ATTRIBUTE_CHANGED) {
+        eventType == nsIAccessibleEvent::EVENT_OBJECT_ATTRIBUTE_CHANGED ||
+        eventType == nsIAccessibleEvent::EVENT_TABLE_STYLING_CHANGED) {
       // Invalidate the cache on our parent table
       [self invalidateLayoutTableCache];
     }

diff --git a/accessible/tests/browser/e10s/browser_caching_table.js b/accessible/tests/browser/e10s/browser_caching_table.js
@@ -208,9 +208,11 @@ addAccessibleTask(
 <table id="mutate"><tr><td>a</td><td>b</td></tr></table>
   `,
   async function(browser, docAcc) {
-    const layout = findAccessibleChildByID(docAcc, "layout");
+    const layout = findAccessibleChildByID(docAcc, "layout", [
+      nsIAccessibleTable,
+    ]);
     testAttrs(layout, { "layout-guess": "true" }, true);
-    const data = findAccessibleChildByID(docAcc, "data");
+    const data = findAccessibleChildByID(docAcc, "data", [nsIAccessibleTable]);
     testAbsentAttrs(data, { "layout-guess": "true" });
     const mutate = findAccessibleChildByID(docAcc, "mutate");
     testAttrs(mutate, { "layout-guess": "true" }, true);
@@ -230,36 +232,9 @@ addAccessibleTask(
   },
   {
     chrome: true,
-    topLevel: true,
-    iframe: true,
-    remoteIframe: true,
-  }
-);
-
-/**
- * Test table layout guess with border styling changes.
- */
-addAccessibleTask(
-  `
-  <table id="layout"><tr><td id="cell">a</td><td>b</td></tr>
-  <tr><td>c</td><td>d</td></tr><tr><td>c</td><td>d</td></tr></table>
-  `,
-  async function(browser, docAcc) {
-    const layout = findAccessibleChildByID(docAcc, "layout");
-    testAttrs(layout, { "layout-guess": "true" }, true);
-    info("changing border style on table cell");
-    let styleChanged = waitForEvent(EVENT_TABLE_STYLING_CHANGED, layout);
-    await invokeContentTask(browser, [], () => {
-      content.document.getElementById("cell").style.border = "1px solid black";
-    });
-    await styleChanged;
-    testAbsentAttrs(layout, { "layout-guess": "true" });
-  },
-  {
-    chrome: true,
-    topLevel: true,
-    iframe: true,
-    remoteIframe: true,
+    topLevel: isCacheEnabled,
+    iframe: isCacheEnabled,
+    remoteIframe: isCacheEnabled,
   }
 );
 

diff --git a/accessible/tests/browser/mac/browser_table.js b/accessible/tests/browser/mac/browser_table.js
@@ -297,10 +297,9 @@ addAccessibleTask(
 );
 
 /*
- * After executing function 'change' which operates on 'elem', verify the specified
- * 'event' is fired on the test's table (assumed id="table"). After the event, check
- * if the given native accessible 'table' is a layout or data table by role
- * using 'isLayout'.
+ * After executing function 'change', verify that the element identified by the
+ * id 'elem' recieves the event 'event'. After the event, check if the given
+ * native accessible 'table' is a layout or data table by role using 'isLayout'.
  */
 async function testIsLayout(table, elem, event, change, isLayout) {
   info(
@@ -309,7 +308,7 @@ async function testIsLayout(table, elem, event, change, isLayout) {
       ", expecting table change to " +
       (isLayout ? "AXGroup" : "AXTable")
   );
-  const toWait = waitForEvent(event, "table");
+  const toWait = waitForEvent(event, elem);
   await change();
   await toWait;
   is(
@@ -327,7 +326,7 @@ async function testIsLayout(table, elem, event, change, isLayout) {
  * appropriately. Attrs: summary, abbr, scope, headers
  */
 addAccessibleTask(
-  `<table id="table" summary="example summary">
+  `<table id="sampleTable" summary="example summary">
     <tr role="presentation">
       <td id="cellOne">cell1</td>
       <td>cell2</td>
@@ -338,7 +337,7 @@ addAccessibleTask(
     </tr>
   </table>`,
   async (browser, accDoc) => {
-    let table = getNativeInterface(accDoc, "table");
+    let table = getNativeInterface(accDoc, "sampleTable");
     // summary attr should take precedence over role="presentation" to make this
     // a data table
     is(table.getAttributeValue("AXRole"), "AXTable", "Table is data table");
@@ -347,11 +346,13 @@ addAccessibleTask(
     // after summary is removed, we should have a layout table
     await testIsLayout(
       table,
-      "table",
+      "sampleTable",
       EVENT_OBJECT_ATTRIBUTE_CHANGED,
       async () => {
         await SpecialPowers.spawn(browser, [], () => {
-          content.document.getElementById("table").removeAttribute("summary");
+          content.document
+            .getElementById("sampleTable")
+            .removeAttribute("summary");
         });
       },
       true

diff --git a/accessible/tests/mochitest/events/test_stylechange.html b/accessible/tests/mochitest/events/test_stylechange.html
@@ -14,7 +14,7 @@
   <script type="application/javascript">
     async function testGotStyleChange(elem, name, value) {
       let waitFor = waitForEvent(
-        EVENT_TABLE_STYLING_CHANGED, "table"
+        EVENT_TABLE_STYLING_CHANGED, elem
       );
       if (value) {
         document.getElementById(elem).style.setProperty(name, value);
@@ -45,7 +45,7 @@
   </script>
 </head>
 <body>
-  <table id="table">
+  <table>
       <tr id="rowOne" style="background-color: red;">
         <td id="cellOne">cell1</td>
         <td>cell2</td>