Bug 1515186 - Always calculate group position for all children of an accessible after a tree mutation, r=Jamie
authorMarco Zehe <mzehe@mozilla.com>
Thu, 10 Jan 2019 05:59:01 +0000
changeset 510310 74bb778f7879
parent 510309 4e654e9222bd
child 510311 6d3e6c915370
child 510343 a853501137bf
push id10547
push userffxbld-merge
push dateMon, 21 Jan 2019 13:03:58 +0000
treeherdermozilla-beta@24ec1916bffe [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersJamie
bugs1515186
milestone66.0a1
first release with
nightly linux32
74bb778f7879 / 66.0a1 / 20190110093854 / files
nightly linux64
74bb778f7879 / 66.0a1 / 20190110093854 / files
nightly mac
74bb778f7879 / 66.0a1 / 20190110093854 / files
nightly win32
74bb778f7879 / 66.0a1 / 20190110093854 / files
nightly win64
74bb778f7879 / 66.0a1 / 20190110093854 / files
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
releases
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1515186 - Always calculate group position for all children of an accessible after a tree mutation, r=Jamie Previously, if we had children a, b, c, and d, then removed b, the group position for c and d would potentially be marked as dirty, but a would not. This caused the check for the availability of previous group info to return outdated information. This patch now always forces the update of all children's group position when a children move has occurred, since it potentially affects all the children, not just the ones after it. In addition, accGroupInfo::Update() now checks if the previous and next siblings that are being used as shortcuts have dirty group info, and are being used only if they do not. Differential Revision: https://phabricator.services.mozilla.com/D16059
accessible/base/AccGroupInfo.cpp
accessible/base/EventTree.cpp
accessible/generic/Accessible.cpp
accessible/tests/mochitest/attributes/a11y.ini
accessible/tests/mochitest/attributes/test_listbox.html
--- a/accessible/base/AccGroupInfo.cpp
+++ b/accessible/base/AccGroupInfo.cpp
@@ -54,17 +54,17 @@ void AccGroupInfo::Update() {
       break;
     }
 
     // Skip subset.
     if (siblingLevel > level) continue;
 
     // If the previous item in the group has calculated group information then
     // build group information for this item based on found one.
-    if (sibling->mBits.groupInfo) {
+    if (sibling->mBits.groupInfo && !sibling->HasDirtyGroupInfo()) {
       mPosInSet += sibling->mBits.groupInfo->mPosInSet;
       mParent = sibling->mBits.groupInfo->mParent;
       mSetSize = sibling->mBits.groupInfo->mSetSize;
       return;
     }
 
     mPosInSet++;
   }
@@ -88,17 +88,17 @@ void AccGroupInfo::Update() {
     int32_t siblingLevel = nsAccUtils::GetARIAOrDefaultLevel(sibling);
     if (siblingLevel < level) break;
 
     // Skip subset.
     if (siblingLevel > level) continue;
 
     // If the next item in the group has calculated group information then
     // build group information for this item based on found one.
-    if (sibling->mBits.groupInfo) {
+    if (sibling->mBits.groupInfo && !sibling->HasDirtyGroupInfo()) {
       mParent = sibling->mBits.groupInfo->mParent;
       mSetSize = sibling->mBits.groupInfo->mSetSize;
       return;
     }
 
     mSetSize++;
   }
 
--- a/accessible/base/EventTree.cpp
+++ b/accessible/base/EventTree.cpp
@@ -98,16 +98,19 @@ void TreeMutation::Done() {
     MOZ_ASSERT(
         mParent->mChildren[idx]->mIndexInParent == static_cast<int32_t>(idx),
         "Wrong index detected");
   }
 #endif
 
   for (uint32_t idx = mStartIdx; idx < length; idx++) {
     mParent->mChildren[idx]->mInt.mIndexOfEmbeddedChild = -1;
+  }
+
+  for (uint32_t idx = 0; idx < length; idx++) {
     mParent->mChildren[idx]->mStateFlags |= Accessible::eGroupInfoDirty;
   }
 
   mParent->mEmbeddedObjCollector = nullptr;
   mParent->mStateFlags |= mStateFlagsCopy & Accessible::eKidsMutating;
 
 #ifdef DEBUG
   mIsDone = true;
--- a/accessible/generic/Accessible.cpp
+++ b/accessible/generic/Accessible.cpp
@@ -2145,18 +2145,21 @@ void Accessible::MoveChild(uint32_t aNew
     }
   } else {
     // The child is moved prior its current position.
     mChildren.InsertElementAt(aNewIndex, aChild);
   }
 
   for (uint32_t idx = startIdx; idx <= endIdx; idx++) {
     mChildren[idx]->mIndexInParent = idx;
+    mChildren[idx]->mInt.mIndexOfEmbeddedChild = -1;
+  }
+
+  for (uint32_t idx = 0; idx < mChildren.Length(); idx++) {
     mChildren[idx]->mStateFlags |= eGroupInfoDirty;
-    mChildren[idx]->mInt.mIndexOfEmbeddedChild = -1;
   }
 
   RefPtr<AccShowEvent> showEvent = new AccShowEvent(aChild);
   DebugOnly<bool> added = mDoc->Controller()->QueueMutationEvent(showEvent);
   MOZ_ASSERT(added);
   aChild->SetShowEventTarget(true);
 }
 
--- a/accessible/tests/mochitest/attributes/a11y.ini
+++ b/accessible/tests/mochitest/attributes/a11y.ini
@@ -1,14 +1,15 @@
 [DEFAULT]
 support-files =
   !/accessible/tests/mochitest/*.js
 
 [test_dpub_aria_xml-roles.html]
 [test_graphics_aria_xml-roles.html]
+[test_listbox.html]
 [test_obj.html]
 [test_obj_css.html]
 [test_obj_css.xul]
 [test_obj_group.html]
 [test_obj_group.xul]
 [test_obj_group_tree.xul]
 [test_tag.html]
 [test_xml-roles.html]
new file mode 100644
--- /dev/null
+++ b/accessible/tests/mochitest/attributes/test_listbox.html
@@ -0,0 +1,83 @@
+<html>
+
+<head>
+  <title>Listbox group attribute tests</title>
+  <meta charset="utf-8" />
+  <link rel="stylesheet" type="text/css" href="chrome://mochikit/content/tests/SimpleTest/test.css" />
+
+  <script type="application/javascript"
+          src="chrome://mochikit/content/tests/SimpleTest/SimpleTest.js"></script>
+
+  <script type="application/javascript"
+          src="../common.js"></script>
+  <script type="application/javascript"
+          src="../attributes.js"></script>
+  <script type="application/javascript"
+          src="../events.js"></script>
+
+  <script type="application/javascript">
+    async function doTest() {
+      // First test the whole lot.
+      testGroupAttrs("a", 1, 6);
+      testGroupAttrs("b", 2, 6);
+      testGroupAttrs("c", 3, 6);
+      testGroupAttrs("d", 4, 6);
+      testGroupAttrs("e", 5, 6);
+      testGroupAttrs("f", 6, 6);
+      // Remove c, reducing the set to 5.
+      let listbox = getAccessible("listbox");
+      let updated = waitForEventPromise(EVENT_REORDER, listbox);
+      c.remove();
+      await updated;
+      testGroupAttrs("a", 1, 5);
+      testGroupAttrs("b", 2, 5);
+      testGroupAttrs("d", 3, 5);
+      testGroupAttrs("e", 4, 5);
+      testGroupAttrs("f", 5, 5);
+      // Now, remove the first element.
+      updated = waitForEventPromise(EVENT_REORDER, listbox);
+      a.remove();
+      await updated;
+      testGroupAttrs("b", 1, 4);
+      testGroupAttrs("d", 2, 4);
+      testGroupAttrs("e", 3, 4);
+      testGroupAttrs("f", 4, 4);
+      // Remove the last item.
+      updated = waitForEventPromise(EVENT_REORDER, listbox);
+      f.remove();
+      await updated;
+      testGroupAttrs("b", 1, 3);
+      testGroupAttrs("d", 2, 3);
+      testGroupAttrs("e", 3, 3);
+      // Finally, remove the middle item.
+      updated = waitForEventPromise(EVENT_REORDER, listbox);
+      d.remove();
+      await updated;
+      testGroupAttrs("b", 1, 2);
+      testGroupAttrs("e", 2, 2);
+
+      SimpleTest.finish();
+    }
+
+    SimpleTest.waitForExplicitFinish();
+    addA11yLoadEvent(doTest);
+  </script>
+</head>
+<body>
+  <p id="display"></p>
+  <div id="content" style="display: none"></div>
+  <pre id="test">
+  </pre>
+
+  <!-- Group information updated after removal of list items, bug 1515186 -->
+  <div id="listbox" role="listbox">
+    <div id="a" role="option">Option a</div>
+    <div id="b" role="option">Option b</div>
+    <div id="c" role="option">Option c</div>
+    <div id="d" role="option">Option d</div>
+    <div id="e" role="option">Option e</div>
+    <div id="f" role="option">Option f</div>
+  </div>  
+  
+</body>
+</html>