Bug 1520611 - Consolidate ProxyAccessible::MustPruneChildren into nsAccUtils. r=Jamie
authorEitan Isaacson <eitan@monotonous.org>
Thu, 24 Jan 2019 00:23:30 +0000
changeset 515212 842a7d9f54bbf01448a887271a1488cc06311aee
parent 515211 fdb28a194129f4c934681746ded72d8a221fd957
child 515213 8507cbf5c07accbc6bfc4fa0f7f610a4407bc92b
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
bugs1520611
milestone66.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 1520611 - Consolidate ProxyAccessible::MustPruneChildren into nsAccUtils. r=Jamie Differential Revision: https://phabricator.services.mozilla.com/D16997
accessible/atk/AccessibleWrap.cpp
accessible/base/nsAccUtils.cpp
accessible/base/nsAccUtils.h
accessible/ipc/ProxyAccessibleBase.cpp
accessible/ipc/ProxyAccessibleBase.h
accessible/windows/msaa/AccessibleWrap.cpp
--- a/accessible/atk/AccessibleWrap.cpp
+++ b/accessible/atk/AccessibleWrap.cpp
@@ -721,17 +721,17 @@ gint getChildCountCB(AtkObject* aAtkObj)
 
     OuterDocAccessible* outerDoc = accWrap->AsOuterDoc();
     if (outerDoc && outerDoc->RemoteChildDoc()) {
       return 1;
     }
   }
 
   ProxyAccessible* proxy = GetProxy(aAtkObj);
-  if (proxy && !proxy->MustPruneChildren()) {
+  if (proxy && !nsAccUtils::MustPrune(proxy)) {
     return proxy->EmbeddedChildCount();
   }
 
   return 0;
 }
 
 AtkObject* refChildCB(AtkObject* aAtkObj, gint aChildIndex) {
   // aChildIndex should not be less than zero
@@ -752,17 +752,19 @@ AtkObject* refChildCB(AtkObject* aAtkObj
     } else {
       OuterDocAccessible* docOwner = accWrap->AsOuterDoc();
       if (docOwner) {
         ProxyAccessible* proxyDoc = docOwner->RemoteChildDoc();
         if (proxyDoc) childAtkObj = GetWrapperFor(proxyDoc);
       }
     }
   } else if (ProxyAccessible* proxy = GetProxy(aAtkObj)) {
-    if (proxy->MustPruneChildren()) return nullptr;
+    if (nsAccUtils::MustPrune(proxy)) {
+      return nullptr;
+    }
 
     ProxyAccessible* child = proxy->EmbeddedChildAt(aChildIndex);
     if (child) childAtkObj = GetWrapperFor(child);
   } else {
     return nullptr;
   }
 
   NS_ASSERTION(childAtkObj, "Fail to get AtkObj");
--- a/accessible/base/nsAccUtils.cpp
+++ b/accessible/base/nsAccUtils.cpp
@@ -384,33 +384,44 @@ uint32_t nsAccUtils::TextLength(Accessib
   // text. They don't have their own frame.
   // XXX In the future, list bullets may have frame and anon content, so
   // we should be able to remove this at that point
   nsAutoString text;
   aAccessible->AppendTextTo(text);  // Get all the text
   return text.Length();
 }
 
-bool nsAccUtils::MustPrune(Accessible* aAccessible) {
-  roles::Role role = aAccessible->Role();
+bool nsAccUtils::MustPrune(AccessibleOrProxy aAccessible) {
+  MOZ_ASSERT(!aAccessible.IsNull());
+  roles::Role role = aAccessible.Role();
+
+  if (role == roles::SLIDER) {
+    // Always prune the tree for sliders, as it doesn't make sense for a
+    // slider to have descendants and this confuses NVDA.
+    return true;
+  }
 
-  return
-      // Always prune the tree for sliders, as it doesn't make sense for a
-      // slider to have descendants and this confuses NVDA.
-      role == roles::SLIDER ||
-      // Don't prune the tree for certain roles if the tree is more complex than
-      // a single text leaf.
-      ((role == roles::MENUITEM || role == roles::COMBOBOX_OPTION ||
-        role == roles::OPTION || role == roles::ENTRY ||
-        role == roles::FLAT_EQUATION || role == roles::PASSWORD_TEXT ||
-        role == roles::PUSHBUTTON || role == roles::TOGGLE_BUTTON ||
-        role == roles::GRAPHIC || role == roles::PROGRESSBAR ||
-        role == roles::SEPARATOR) &&
-       aAccessible->ContentChildCount() == 1 &&
-       aAccessible->ContentChildAt(0)->IsTextLeaf());
+  if (role != roles::MENUITEM && role != roles::COMBOBOX_OPTION &&
+      role != roles::OPTION && role != roles::ENTRY &&
+      role != roles::FLAT_EQUATION && role != roles::PASSWORD_TEXT &&
+      role != roles::PUSHBUTTON && role != roles::TOGGLE_BUTTON &&
+      role != roles::GRAPHIC && role != roles::PROGRESSBAR &&
+      role != roles::SEPARATOR) {
+    // If it doesn't match any of these roles, don't prune its children.
+    return false;
+  }
+
+  if (aAccessible.ChildCount() != 1) {
+    // If the accessible has more than one child, don't prune it.
+    return false;
+  }
+
+  roles::Role childRole = aAccessible.FirstChild().Role();
+  // If the accessible's child is a text leaf, prune the accessible.
+  return childRole == roles::TEXT_LEAF || childRole == roles::STATICTEXT;
 }
 
 bool nsAccUtils::PersistentPropertiesToArray(nsIPersistentProperties* aProps,
                                              nsTArray<Attribute>* aAttributes) {
   if (!aProps) {
     return true;
   }
   nsCOMPtr<nsISimpleEnumerator> propEnum;
--- a/accessible/base/nsAccUtils.h
+++ b/accessible/base/nsAccUtils.h
@@ -4,16 +4,17 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #ifndef nsAccUtils_h_
 #define nsAccUtils_h_
 
 #include "mozilla/a11y/Accessible.h"
 #include "mozilla/a11y/DocManager.h"
 
+#include "AccessibleOrProxy.h"
 #include "nsAccessibilityService.h"
 #include "nsCoreUtils.h"
 
 #include "nsIDocShell.h"
 #include "nsPoint.h"
 
 namespace mozilla {
 
@@ -246,17 +247,17 @@ class nsAccUtils {
     *aIsExtra = !!extraState;
     return aState | extraState;
   }
 
   /**
    * Return true if the given accessible can't have children. Used when exposing
    * to platform accessibility APIs, should the children be pruned off?
    */
-  static bool MustPrune(Accessible* aAccessible);
+  static bool MustPrune(AccessibleOrProxy aAccessible);
 
   static bool PersistentPropertiesToArray(nsIPersistentProperties* aProps,
                                           nsTArray<Attribute>* aAttributes);
 
   /**
    * Return true if the given accessible is within an ARIA live region; i.e.
    * the container-live attribute would be something other than "off" or empty.
    */
--- a/accessible/ipc/ProxyAccessibleBase.cpp
+++ b/accessible/ipc/ProxyAccessibleBase.cpp
@@ -63,34 +63,16 @@ void ProxyAccessibleBase<Derived>::Clear
   // has not had a chance to remove itself, but was already replaced by Doc 2
   // in SetChildDoc(). This could result in two subsequent calls to
   // ClearChildDoc() even though mChildren.Length() == 1.
   MOZ_ASSERT(mChildren.Length() <= 1);
   mChildren.RemoveElement(aChildDoc);
 }
 
 template <class Derived>
-bool ProxyAccessibleBase<Derived>::MustPruneChildren() const {
-  // this is the equivalent to nsAccUtils::MustPrune for proxies and should be
-  // kept in sync with that.
-  if (mRole != roles::MENUITEM && mRole != roles::COMBOBOX_OPTION &&
-      mRole != roles::OPTION && mRole != roles::ENTRY &&
-      mRole != roles::FLAT_EQUATION && mRole != roles::PASSWORD_TEXT &&
-      mRole != roles::PUSHBUTTON && mRole != roles::TOGGLE_BUTTON &&
-      mRole != roles::GRAPHIC && mRole != roles::SLIDER &&
-      mRole != roles::PROGRESSBAR && mRole != roles::SEPARATOR)
-    return false;
-
-  if (mChildren.Length() != 1) return false;
-
-  return mChildren[0]->Role() == roles::TEXT_LEAF ||
-         mChildren[0]->Role() == roles::STATICTEXT;
-}
-
-template <class Derived>
 uint32_t ProxyAccessibleBase<Derived>::EmbeddedChildCount() const {
   size_t count = 0, kids = mChildren.Length();
   for (size_t i = 0; i < kids; i++) {
     if (mChildren[i]->IsEmbeddedObject()) {
       count++;
     }
   }
 
--- a/accessible/ipc/ProxyAccessibleBase.h
+++ b/accessible/ipc/ProxyAccessibleBase.h
@@ -66,17 +66,16 @@ class ProxyAccessibleBase {
 
   // XXX evaluate if this is fast enough.
   size_t IndexInParent() const {
     return Parent()->mChildren.IndexOf(static_cast<const Derived*>(this));
   }
   uint32_t EmbeddedChildCount() const;
   int32_t IndexOfEmbeddedChild(const Derived* aChild);
   Derived* EmbeddedChildAt(size_t aChildIdx);
-  bool MustPruneChildren() const;
 
   void Shutdown();
 
   void SetChildDoc(DocAccessibleParent* aChildDoc);
   void ClearChildDoc(DocAccessibleParent* aChildDoc);
 
   /**
    * Remove The given child.
--- a/accessible/windows/msaa/AccessibleWrap.cpp
+++ b/accessible/windows/msaa/AccessibleWrap.cpp
@@ -898,26 +898,26 @@ AccessibleWrap::accNavigate(
 #define RELATIONTYPE(geckoType, stringType, atkType, msaaType, ia2Type) \
   case msaaType:                                                        \
     xpRelation.emplace(RelationType::geckoType);                        \
     break;
 
   switch (navDir) {
     case NAVDIR_FIRSTCHILD:
       if (IsProxy()) {
-        if (!Proxy()->MustPruneChildren()) {
+        if (!nsAccUtils::MustPrune(Proxy())) {
           navAccessible = WrapperFor(Proxy()->FirstChild());
         }
       } else {
         if (!nsAccUtils::MustPrune(this)) navAccessible = FirstChild();
       }
       break;
     case NAVDIR_LASTCHILD:
       if (IsProxy()) {
-        if (!Proxy()->MustPruneChildren()) {
+        if (!nsAccUtils::MustPrune(Proxy())) {
           navAccessible = WrapperFor(Proxy()->LastChild());
         }
       } else {
         if (!nsAccUtils::MustPrune(this)) navAccessible = LastChild();
       }
       break;
     case NAVDIR_NEXT:
       navAccessible =
@@ -1382,17 +1382,17 @@ already_AddRefed<IAccessible> Accessible
     }
     // Otherwise, since we're a proxy and we have a null native interface, this
     // indicates that we need to obtain a COM proxy. To do this, we'll replace
     // CHILDID_SELF with our real MSAA ID and continue the search from there.
     varChild.lVal = GetExistingID();
   }
 
   if (varChild.ulVal != GetExistingID() &&
-      (IsProxy() ? Proxy()->MustPruneChildren()
+      (IsProxy() ? nsAccUtils::MustPrune(Proxy())
                  : nsAccUtils::MustPrune(this))) {
     // This accessible should have no subtree in platform, return null for its
     // children.
     return nullptr;
   }
 
   // If the MSAA ID is not a chrome id then we already know that we won't
   // find it here and should look remotely instead. This handles the case when