Bug 1249253 - "After bug 1133213, in Google Docs, screen readers started to say "blank" before characters, lines, and when navigating". r=yzenevich a=ritu
authoralexander <surkov.alexander>
Mon, 14 Mar 2016 11:16:00 +0100
changeset 323470 2fd77fb72e36e32844715012c6bae54b0e9d34aa
parent 323469 112cb0b69503ef77205124328c00fcc44a2297f3
child 323471 cd661d54ee73de897099fae295e871be31271bfe
push id5913
push userjlund@mozilla.com
push dateMon, 25 Apr 2016 16:57:49 +0000
treeherdermozilla-beta@dcaf0a6fa115 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersyzenevich, ritu
bugs1249253, 1133213
milestone47.0a2
Bug 1249253 - "After bug 1133213, in Google Docs, screen readers started to say "blank" before characters, lines, and when navigating". r=yzenevich a=ritu
accessible/base/TreeWalker.cpp
accessible/base/TreeWalker.h
accessible/generic/DocAccessible.cpp
accessible/tests/mochitest/treeupdate/test_ariaowns.html
--- a/accessible/base/TreeWalker.cpp
+++ b/accessible/base/TreeWalker.cpp
@@ -40,19 +40,18 @@ TreeWalker::
 }
 
 TreeWalker::
   TreeWalker(Accessible* aContext, nsIContent* aAnchorNode, uint32_t aFlags) :
   mDoc(aContext->Document()), mContext(aContext), mAnchorNode(aAnchorNode),
   mARIAOwnsIdx(0),
   mChildFilter(nsIContent::eSkipPlaceholderContent), mFlags(aFlags)
 {
+  MOZ_ASSERT(mFlags & eWalkCache, "This constructor cannot be used for tree creation");
   MOZ_ASSERT(aAnchorNode, "No anchor node for the accessible tree walker");
-  MOZ_ASSERT(mDoc->GetAccessibleOrContainer(aAnchorNode) == mContext,
-             "Unexpected anchor node was given");
 
   mChildFilter |= mContext->NoXBLKids() ?
     nsIContent::eAllButXBL : nsIContent::eAllChildren;
 
   PushState(aAnchorNode);
 
   MOZ_COUNT_CTOR(TreeWalker);
 }
@@ -98,19 +97,25 @@ TreeWalker::Next()
       if (!skipSubtree && childNode->IsElement()) {
         top = PushState(childNode);
       }
     }
     top = PopState();
   }
 
   // If we traversed the whole subtree of the anchor node. Move to next node
-  // relative anchor node within the context subtree if possible.
-  if (mFlags != eWalkContextTree)
+  // relative anchor node within the context subtree if asked.
+  if (mFlags != eWalkContextTree) {
+    // eWalkCache flag presence indicates that the search is scoped to the
+    // anchor (no ARIA owns stuff).
+    if (mFlags & eWalkCache) {
+      return nullptr;
+    }
     return Next();
+  }
 
   nsINode* contextNode = mContext->GetNode();
   while (mAnchorNode != contextNode) {
     nsINode* parentNode = mAnchorNode->GetFlattenedTreeParent();
     if (!parentNode || !parentNode->IsElement())
       return nullptr;
 
     nsIContent* parent = parentNode->AsElement();
--- a/accessible/base/TreeWalker.h
+++ b/accessible/base/TreeWalker.h
@@ -40,17 +40,17 @@ public:
   /**
    * Used to navigate the accessible children relative to the anchor.
    *
    * @param aContext [in] container accessible for the given node, used to
    *                   define accessible context
    * @param aAnchorNode [in] the node the search will be prepared relative to
    * @param aFlags   [in] flags (see enum above)
    */
-  TreeWalker(Accessible* aContext, nsIContent* aAnchorNode, uint32_t aFlags = 0);
+  TreeWalker(Accessible* aContext, nsIContent* aAnchorNode, uint32_t aFlags = eWalkCache);
 
   ~TreeWalker();
 
   /**
    * Return the next accessible.
    *
    * @note Returned accessible is bound to the document, if the accessible is
    *       rejected during tree creation then the caller should be unbind it
--- a/accessible/generic/DocAccessible.cpp
+++ b/accessible/generic/DocAccessible.cpp
@@ -1790,44 +1790,19 @@ DocAccessible::UpdateTreeOnRemoval(Acces
 
   uint32_t updateFlags = eNoAccessible;
   RefPtr<AccReorderEvent> reorderEvent = new AccReorderEvent(aContainer);
   AutoTreeMutation mut(aContainer);
 
   if (child) {
     updateFlags |= UpdateTreeInternal(child, false, reorderEvent);
   } else {
-    // aChildNode may not coorespond to a particular accessible, to handle
-    // this we go through all the children of aContainer.  Then if a child
-    // has aChildNode as an ancestor, or does not have the node for
-    // aContainer as an ancestor remove that child of aContainer.  Note that
-    // when we are called aChildNode may already have been removed from the DOM
-    // so we can't expect it to have a parent or what was it's parent to have
-    // it as a child.
-    nsINode* containerNode = aContainer->GetNode();
-    for (uint32_t idx = 0; idx < aContainer->ContentChildCount();) {
-      Accessible* child = aContainer->ContentChildAt(idx);
-
-      // If accessible doesn't have its own content then we assume parent
-      // will handle its update.  If child is DocAccessible then we don't
-      // handle updating it here either.
-      if (!child->HasOwnContent() || child->IsDoc()) {
-        idx++;
-        continue;
-      }
-
-      nsINode* childNode = child->GetContent();
-      while (childNode != aChildNode && childNode != containerNode &&
-             (childNode = childNode->GetParentNode()));
-
-      if (childNode != containerNode) {
-        updateFlags |= UpdateTreeInternal(child, false, reorderEvent);
-      } else {
-        idx++;
-      }
+    TreeWalker walker(aContainer, aChildNode, TreeWalker::eWalkCache);
+    while (Accessible* child = walker.Next()) {
+      updateFlags |= UpdateTreeInternal(child, false, reorderEvent);
     }
   }
 
   // Content insertion/removal is not cause of accessible tree change.
   if (updateFlags == eNoAccessible)
     return;
 
   MaybeNotifyOfValueChange(aContainer);
--- a/accessible/tests/mochitest/treeupdate/test_ariaowns.html
+++ b/accessible/tests/mochitest/treeupdate/test_ariaowns.html
@@ -467,16 +467,53 @@
       }
 
       this.getID = function rearrangeARIAOwns_getID()
       {
         return `Rearrange @aria-owns attribute to '${aAttr}'`;
       }
     }
 
+    function removeNotARIAOwnedEl(aContainer, aChild)
+    {
+      this.eventSeq = [
+        new invokerChecker(EVENT_REORDER, aContainer)
+      ];
+
+      this.invoke = function removeNotARIAOwnedEl_invoke()
+      {
+        dumpTree(aContainer, "before");
+        var tree = {
+          SECTION: [
+            { TEXT_LEAF: [ ] },
+            { GROUPING: [ ] }
+          ]
+        };
+        testAccessibleTree(aContainer, tree);
+
+        getNode(aContainer).removeChild(getNode(aChild));
+      }
+
+      this.finalCheck = function removeNotARIAOwnedEl_finalCheck()
+      {
+        dumpTree(aContainer, "after");
+        var tree = {
+          SECTION: [
+            { GROUPING: [ ] }
+          ]
+        };
+        testAccessibleTree(aContainer, tree);
+      }
+
+      this.getID = function removeNotARIAOwnedEl_getID()
+      {
+        return `remove not ARIA owned child`;
+      }
+    }
+
 
     ////////////////////////////////////////////////////////////////////////////
     // Test
     ////////////////////////////////////////////////////////////////////////////
 
     //gA11yEventDumpToConsole = true;
     //enableLogging("tree,verbose"); // debug stuff
 
@@ -511,16 +548,18 @@
         "t5_container", "t5_checkbox t5_radio t5_button",
         [ "t5_checkbox", "t5_radio", "t5_button" ],
         [ "CHECKBUTTON", "RADIOBUTTON", "PUSHBUTTON" ]));
       gQueue.push(new rearrangeARIAOwns(
         "t5_container", "t5_radio t5_button t5_checkbox",
         [ "t5_radio", "t5_button" ],
         [ "RADIOBUTTON", "PUSHBUTTON", "CHECKBUTTON" ]));
 
+      gQueue.push(new removeNotARIAOwnedEl("t6_container", "t6_span"));
+
       gQueue.invoke(); // SimpleTest.finish() will be called in the end
     }
 
     SimpleTest.waitForExplicitFinish();
     addA11yLoadEvent(doTest);
 
   </script>
 </head>
@@ -558,11 +597,16 @@
     <div id="t4_child2" role="radio"></div>
   </div>
 
   <div id="t5_container">
     <div role="button" id="t5_button"></div>
     <div role="checkbox" id="t5_checkbox"></div>
     <div role="radio" id="t5_radio"></div>
   </div>
+
+  <div id="t6_container" aria-owns="t6_fake">
+    <span id="t6_span">hey</span>
+  </div>
+  <div id="t6_fake" role="group"></div>
 </body>
 
 </html>