Bug 1378257 - Don't move/reclaim aria-owned children to their current position. r=surkov, r=yzen
authorEitan Isaacson <eitan@monotonous.org>
Tue, 25 Jul 2017 16:31:00 -0400
changeset 422122 1d14aaceb2f6e3afb490ef4f4af54fd32265b1e8
parent 422121 151b602f0649a6fd2048c4c95a8bea2523ae4b99
child 422123 dd757326f72bac40ecc8b387a4ea8ed1dd53b873
push id1517
push userjlorenzo@mozilla.com
push dateThu, 14 Sep 2017 16:50:54 +0000
treeherdermozilla-release@3b41fd564418 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssurkov, yzen
bugs1378257
milestone56.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 1378257 - Don't move/reclaim aria-owned children to their current position. r=surkov, r=yzen
accessible/generic/DocAccessible.cpp
accessible/moz.build
accessible/tests/browser/events.js
accessible/tests/browser/shared-head.js
accessible/tests/browser/tree/browser.ini
accessible/tests/browser/tree/browser_test_aria_owns.js
accessible/tests/browser/tree/head.js
--- a/accessible/generic/DocAccessible.cpp
+++ b/accessible/generic/DocAccessible.cpp
@@ -2102,21 +2102,41 @@ DocAccessible::DoARIAOwnsRelocation(Acce
     }
 
 #ifdef A11Y_LOG
   logging::TreeInfo("aria owns traversal", logging::eVerbose,
                     "candidate", child, nullptr);
 #endif
 
     // Same child on same position, no change.
-    if (child->Parent() == aOwner &&
-        child->IndexInParent() == static_cast<int32_t>(insertIdx)) {
-      MOZ_ASSERT(owned->ElementAt(idx) == child, "Not in sync!");
-      idx++;
-      continue;
+    if (child->Parent() == aOwner) {
+      int32_t indexInParent = child->IndexInParent();
+
+      // The child is being placed in its current index,
+      // eg. aria-owns='id1 id2 id3' is changed to aria-owns='id3 id2 id1'.
+      if (indexInParent == static_cast<int32_t>(insertIdx)) {
+        MOZ_ASSERT(child->IsRelocated(),
+                   "A child, having an index in parent from aria ownded indices range, has to be aria owned");
+        MOZ_ASSERT(owned->ElementAt(idx) == child,
+                   "Unexpected child in ARIA owned array");
+        idx++;
+        continue;
+      }
+
+      // The child is being inserted directly after its current index,
+      // resulting in a no-move case. This will happen when a parent aria-owns
+      // its last ordinal child:
+      // <ul aria-owns='id2'><li id='id1'></li><li id='id2'></li></ul>
+      if (indexInParent == static_cast<int32_t>(insertIdx) - 1) {
+        MOZ_ASSERT(!child->IsRelocated(), "Child should be in its ordinal position");
+        child->SetRelocated(true);
+        owned->InsertElementAt(idx, child);
+        idx++;
+        continue;
+      }
     }
 
     MOZ_ASSERT(owned->SafeElementAt(idx) != child, "Already in place!");
 
     if (owned->IndexOf(child) < idx) {
       continue; // ignore second entry of same ID
     }
 
@@ -2185,17 +2205,33 @@ DocAccessible::PutChildrenBack(nsTArray<
           MOZ_DIAGNOSTIC_ASSERT(origContainer == prevChild->Parent(), "Broken tree");
           origContainer = prevChild->Parent();
         }
         else {
           idxInParent = 0;
         }
       }
     }
-    MoveChild(child, origContainer, idxInParent);
+
+    // The child may have already be in its ordinal place for 2 reasons:
+    // 1. It was the last ordinal child, and the first aria-owned child.
+    //    given:      <ul id="list" aria-owns="b"><li id="a"></li><li id="b"></li></ul>
+    //    after load: $("list").setAttribute("aria-owns", "");
+    // 2. The preceding adopted children were just reclaimed, eg:
+    //    given:      <ul id="list"><li id="b"></li></ul>
+    //    after load: $("list").setAttribute("aria-owns", "a b");
+    //    later:      $("list").setAttribute("aria-owns", "");
+    if (origContainer != owner || child->IndexInParent() != idxInParent) {
+      MoveChild(child, origContainer, idxInParent);
+    } else {
+      MOZ_ASSERT(!child->PrevSibling() || !child->PrevSibling()->IsRelocated(),
+                 "No relocated child should appear before this one");
+      MOZ_ASSERT(!child->NextSibling() || child->NextSibling()->IsRelocated(),
+                 "No ordinal child should appear after this one");
+    }
   }
 
   aChildren->RemoveElementsAt(aStartIdx, aChildren->Length() - aStartIdx);
 }
 
 bool
 DocAccessible::MoveChild(Accessible* aChild, Accessible* aNewParent,
                          int32_t aIdxInParent)
--- a/accessible/moz.build
+++ b/accessible/moz.build
@@ -31,13 +31,14 @@ if CONFIG['MOZ_XUL']:
 TEST_DIRS += ['tests/mochitest']
 
 BROWSER_CHROME_MANIFESTS += [
   'tests/browser/bounds/browser.ini',
   'tests/browser/browser.ini',
   'tests/browser/e10s/browser.ini',
   'tests/browser/events/browser.ini',
   'tests/browser/scroll/browser.ini',
-  'tests/browser/states/browser.ini'
+  'tests/browser/states/browser.ini',
+  'tests/browser/tree/browser.ini'
 ]
 
 with Files("**"):
     BUG_COMPONENT = ("Core", "Disability Access APIs")
--- a/accessible/tests/browser/events.js
+++ b/accessible/tests/browser/events.js
@@ -155,32 +155,34 @@ class UnexpectedEvents {
 }
 
 /**
  * A helper function that waits for a sequence of accessible events in
  * specified order.
  * @param {Array} events        a list of events to wait (same format as
  *                              waitForEvent arguments)
  */
-function waitForEvents(events, unexpected = [], ordered = false) {
+function waitForEvents(events, ordered = false) {
+  let expected = events.expected || events;
+  let unexpected = events.unexpected || [];
   // Next expected event index.
   let currentIdx = 0;
 
   let unexpectedListener = new UnexpectedEvents(unexpected);
 
-  return Promise.all(events.map((evt, idx) => {
+  return Promise.all(expected.map((evt, idx) => {
     let promise = evt instanceof Array ? waitForEvent(...evt) : evt;
     return promise.then(result => {
       if (ordered) {
         is(idx, currentIdx++,
           `Unexpected event order: ${result}`);
       }
       return result;
     });
   })).then(results => {
     unexpectedListener.stop();
     return results;
   });
 }
 
-function waitForOrderedEvents(events, unexpected = []) {
-  return waitForEvents(events, unexpected, true);
+function waitForOrderedEvents(events) {
+  return waitForEvents(events, true);
 }
--- a/accessible/tests/browser/shared-head.js
+++ b/accessible/tests/browser/shared-head.js
@@ -6,17 +6,17 @@
 
 /* import-globals-from ../mochitest/common.js */
 /* import-globals-from events.js */
 
 /* exported Logger, MOCHITESTS_DIR, invokeSetAttribute, invokeFocus,
             invokeSetStyle, getAccessibleDOMNodeID, getAccessibleTagName,
             addAccessibleTask, findAccessibleChildByID, isDefunct,
             CURRENT_CONTENT_DIR, loadScripts, loadFrameScripts, snippetToURL,
-            Cc, Cu */
+            Cc, Cu, arrayFromChildren */
 
 const { interfaces: Ci, utils: Cu, classes: Cc } = Components;
 
 /**
  * Current browser test directory path used to load subscripts.
  */
 const CURRENT_DIR =
   'chrome://mochitests/content/browser/accessible/tests/browser/';
@@ -353,8 +353,13 @@ function queryInterfaces(accessible, int
       accessible.QueryInterface(iface);
     } catch (e) {
       ok(false, "Can't query " + iface);
     }
   }
 
   return accessible;
 }
+
+function arrayFromChildren(accessible) {
+  return Array.from({ length: accessible.childCount }, (c, i) =>
+    accessible.getChildAt(i));
+}
new file mode 100644
--- /dev/null
+++ b/accessible/tests/browser/tree/browser.ini
@@ -0,0 +1,9 @@
+[DEFAULT]
+skip-if = e10s && os == 'win' && release_or_beta
+support-files =
+  head.js
+  !/accessible/tests/browser/events.js
+  !/accessible/tests/browser/shared-head.js
+  !/accessible/tests/mochitest/*.js
+
+[browser_test_aria_owns.js]
new file mode 100644
--- /dev/null
+++ b/accessible/tests/browser/tree/browser_test_aria_owns.js
@@ -0,0 +1,96 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+"use strict";
+
+function testChildrenIds(acc, expectedIds) {
+  let ids = arrayFromChildren(acc).map(child => getAccessibleDOMNodeID(child));
+  Assert.deepEqual(ids, expectedIds,
+    `Children for ${getAccessibleDOMNodeID(acc)} are wrong.`);
+}
+
+async function runTests(browser, accDoc) {
+  let one = findAccessibleChildByID(accDoc, "one");
+  let two = findAccessibleChildByID(accDoc, "two");
+  let three = findAccessibleChildByID(accDoc, "three");
+  let four = findAccessibleChildByID(accDoc, "four");
+
+  testChildrenIds(one, ["a"]);
+  testChildrenIds(two, ["b", "c", "d"]);
+  testChildrenIds(three, []);
+
+  let onReorders = waitForEvents({
+    expected: [
+      [EVENT_REORDER, "two"]], // children will be reordered via aria-owns
+    unexpected: [
+      [EVENT_REORDER, "one"],  // child will remain in place
+      [EVENT_REORDER, "three"], // none of its children will be reclaimed
+      [EVENT_REORDER, "four"]] // child will remain in place
+  });
+
+  await ContentTask.spawn(browser, null, async function() {
+    // aria-own ordinal child in place, should be a no-op.
+    document.getElementById("one").setAttribute("aria-owns", "a");
+    // remove aria-owned child that is already ordinal, should be no-op.
+    document.getElementById("four").removeAttribute("aria-owns");
+    // shuffle aria-owns with markup child.
+    document.getElementById("two").setAttribute("aria-owns", "d c");
+  });
+
+  await onReorders;
+
+  testChildrenIds(one, ["a"]);
+  testChildrenIds(two, ["b", "d", "c"]);
+  testChildrenIds(three, []);
+  testChildrenIds(four, ["e"]);
+
+  onReorders = waitForEvent(EVENT_REORDER, "one");
+
+  await ContentTask.spawn(browser, null, async function() {
+    let aa = document.createElement("li");
+    aa.id = "aa";
+    document.getElementById("one").appendChild(aa);
+  });
+
+  await onReorders;
+
+  testChildrenIds(one, ["aa", "a"]);
+
+  onReorders = waitForEvents([
+      [EVENT_REORDER, "two"],    // "b" will go to "three"
+      [EVENT_REORDER, "three"], // some children will be reclaimed and acquired
+      [EVENT_REORDER, "one"]]); // removing aria-owns will reorder native children
+
+  await ContentTask.spawn(browser, null, async function() {
+    // removing aria-owns should reorder the children
+    document.getElementById("one").removeAttribute("aria-owns");
+    // child order will be overridden by aria-owns
+    document.getElementById("three").setAttribute("aria-owns", "b d");
+  });
+
+  await onReorders;
+
+  testChildrenIds(one, ["a", "aa"]);
+  testChildrenIds(two, ["c"]);
+  testChildrenIds(three, ["b", "d"]);
+}
+
+/**
+ * Test caching of accessible object states
+ */
+addAccessibleTask(`
+    <ul id="one">
+      <li id="a">Test</li>
+    </ul>
+    <ul id="two" aria-owns="d">
+      <li id="b">Test 2</li>
+      <li id="c">Test 3</li>
+    </ul>
+    <ul id="three">
+      <li id="d">Test 4</li>
+    </ul>
+    <ul id="four" aria-owns="e">
+      <li id="e">Test 5</li>
+    </ul>
+    `, runTests);
new file mode 100644
--- /dev/null
+++ b/accessible/tests/browser/tree/head.js
@@ -0,0 +1,15 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+'use strict';
+
+// Load the shared-head file first.
+/* import-globals-from ../shared-head.js */
+Services.scriptloader.loadSubScript(
+  'chrome://mochitests/content/browser/accessible/tests/browser/shared-head.js',
+  this);
+
+// Loading and common.js from accessible/tests/mochitest/ for all tests, as
+// well as events.js.
+loadScripts({ name: 'common.js', dir: MOCHITESTS_DIR }, 'events.js', 'layout.js');