Bug 1189108 - Walk up tree to get LinkableAccessible actions instead of caching r=tbsaunde
authorLorien Hu <lorien@lorienhu.com>
Tue, 04 Aug 2015 23:33:54 -0400
changeset 287919 ef291c49717330a7e9132b075a68baccbd2e8748
parent 287918 b8a39e65c778d7c6a6c213e46e5655207f51bded
child 287920 89002bda1f558d5df20a36e0db4b1a5c10d517f8
push id5067
push userraliiev@mozilla.com
push dateMon, 21 Sep 2015 14:04:52 +0000
treeherdermozilla-beta@14221ffe5b2f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerstbsaunde
bugs1189108
milestone42.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 1189108 - Walk up tree to get LinkableAccessible actions instead of caching r=tbsaunde
accessible/generic/Accessible.h
accessible/generic/BaseAccessibles.cpp
accessible/generic/BaseAccessibles.h
--- a/accessible/generic/Accessible.h
+++ b/accessible/generic/Accessible.h
@@ -940,18 +940,18 @@ protected:
   /**
    * Cache accessible children.
    */
   virtual void CacheChildren();
 
   /**
    * Set accessible parent and index in parent.
    */
-  virtual void BindToParent(Accessible* aParent, uint32_t aIndexInParent);
-  virtual void UnbindFromParent();
+  void BindToParent(Accessible* aParent, uint32_t aIndexInParent);
+  void UnbindFromParent();
 
   /**
    * Return sibling accessible at the given offset.
    */
   virtual Accessible* GetSiblingAtOffset(int32_t aOffset,
                                          nsresult *aError = nullptr) const;
 
   /**
--- a/accessible/generic/BaseAccessibles.cpp
+++ b/accessible/generic/BaseAccessibles.cpp
@@ -62,174 +62,169 @@ LeafAccessible::CacheChildren()
   // No children for leaf accessible.
 }
 
 
 ////////////////////////////////////////////////////////////////////////////////
 // LinkableAccessible
 ////////////////////////////////////////////////////////////////////////////////
 
-LinkableAccessible::
-  LinkableAccessible(nsIContent* aContent, DocAccessible* aDoc) :
-  AccessibleWrap(aContent, aDoc),
-  mActionAcc(nullptr),
-  mIsLink(false),
-  mIsOnclick(false)
-{
-}
-
 NS_IMPL_ISUPPORTS_INHERITED0(LinkableAccessible, AccessibleWrap)
 
 ////////////////////////////////////////////////////////////////////////////////
 // LinkableAccessible. nsIAccessible
 
 void
 LinkableAccessible::TakeFocus()
 {
-  if (mActionAcc)
-    mActionAcc->TakeFocus();
-  else
+  if (Accessible* actionAcc = ActionWalk()) {
+    actionAcc->TakeFocus();
+  } else {
     AccessibleWrap::TakeFocus();
+  }
 }
 
 uint64_t
 LinkableAccessible::NativeLinkState() const
 {
-  if (mIsLink)
-    return states::LINKED | (mActionAcc->LinkState() & states::TRAVERSED);
+  bool isLink;
+  Accessible* actionAcc =
+    const_cast<LinkableAccessible*>(this)->ActionWalk(&isLink);
+  if (isLink) {
+    return states::LINKED | (actionAcc->LinkState() & states::TRAVERSED);
+  }
 
   return 0;
 }
 
 void
 LinkableAccessible::Value(nsString& aValue)
 {
   aValue.Truncate();
 
   Accessible::Value(aValue);
-  if (!aValue.IsEmpty())
+  if (!aValue.IsEmpty()) {
     return;
+  }
 
-  if (aValue.IsEmpty() && mIsLink)
-    mActionAcc->Value(aValue);
+  bool isLink;
+  Accessible* actionAcc = ActionWalk(&isLink);
+  if (isLink) {
+    actionAcc->Value(aValue);
+  }
 }
 
-
 uint8_t
 LinkableAccessible::ActionCount()
 {
-  return (mIsOnclick || mIsLink) ? 1 : 0;
+  bool isLink, isOnclick;
+  ActionWalk(&isLink, &isOnclick);
+  return (isLink || isOnclick) ? 1 : 0;
+}
+
+Accessible*
+LinkableAccessible::ActionWalk(bool* aIsLink, bool* aIsOnclick)
+{
+  if (aIsOnclick) {
+    *aIsOnclick = false;
+  }
+  if (aIsLink) {
+    *aIsLink = false;
+  }
+
+  if (nsCoreUtils::HasClickListener(mContent)) {
+    if (aIsOnclick) {
+      *aIsOnclick = true;
+    }
+    return nullptr;
+  }
+
+  // XXX: The logic looks broken since the click listener may be registered
+  // on non accessible node in parent chain but this node is skipped when tree
+  // is traversed.
+  Accessible* walkUpAcc = this;
+  while ((walkUpAcc = walkUpAcc->Parent()) && !walkUpAcc->IsDoc()) {
+    if (walkUpAcc->LinkState() & states::LINKED) {
+      if (aIsLink) {
+        *aIsLink = true;
+      }
+      return walkUpAcc;
+    }
+
+    if (nsCoreUtils::HasClickListener(walkUpAcc->GetContent())) {
+      if (aIsOnclick) {
+        *aIsOnclick = true;
+      }
+      return walkUpAcc;
+    }
+  }
+  return nullptr;
 }
 
 void
 LinkableAccessible::ActionNameAt(uint8_t aIndex, nsAString& aName)
 {
   aName.Truncate();
 
   // Action 0 (default action): Jump to link
   if (aIndex == eAction_Jump) {
-    if (mIsLink)
+    bool isOnclick, isLink;
+    ActionWalk(&isLink, &isOnclick);
+    if (isLink) {
       aName.AssignLiteral("jump");
-    else if (mIsOnclick)
+    } else if (isOnclick) {
       aName.AssignLiteral("click");
+    }
   }
 }
 
 bool
 LinkableAccessible::DoAction(uint8_t aIndex)
 {
-  if (aIndex != eAction_Jump)
+  if (aIndex != eAction_Jump) {
     return false;
+  }
 
-  return mActionAcc ? mActionAcc->DoAction(aIndex) :
-    AccessibleWrap::DoAction(aIndex);
+  if (Accessible* actionAcc = ActionWalk()) {
+    return actionAcc->DoAction(aIndex);
+  }
+
+  return AccessibleWrap::DoAction(aIndex);
 }
 
 KeyBinding
 LinkableAccessible::AccessKey() const
 {
-  return mActionAcc ?
-    mActionAcc->AccessKey() : Accessible::AccessKey();
-}
-
-////////////////////////////////////////////////////////////////////////////////
-// LinkableAccessible. Accessible
+  if (const Accessible* actionAcc =
+    const_cast<LinkableAccessible*>(this)->ActionWalk()) {
+    return actionAcc->AccessKey();
+  }
 
-void
-LinkableAccessible::Shutdown()
-{
-  mIsLink = false;
-  mIsOnclick = false;
-  mActionAcc = nullptr;
-  AccessibleWrap::Shutdown();
+  return Accessible::AccessKey();
 }
 
 ////////////////////////////////////////////////////////////////////////////////
 // LinkableAccessible: HyperLinkAccessible
 
 already_AddRefed<nsIURI>
 LinkableAccessible::AnchorURIAt(uint32_t aAnchorIndex)
 {
-  if (mIsLink) {
-    NS_ASSERTION(mActionAcc->IsLink(), "HyperLink isn't implemented.");
+  bool isLink;
+  Accessible* actionAcc = ActionWalk(&isLink);
+  if (isLink) {
+    NS_ASSERTION(actionAcc->IsLink(), "HyperLink isn't implemented.");
 
-    if (mActionAcc->IsLink())
-      return mActionAcc->AnchorURIAt(aAnchorIndex);
+    if (actionAcc->IsLink()) {
+      return actionAcc->AnchorURIAt(aAnchorIndex);
+    }
   }
 
   return nullptr;
 }
 
-////////////////////////////////////////////////////////////////////////////////
-// LinkableAccessible: Accessible protected
-
-void
-LinkableAccessible::BindToParent(Accessible* aParent,
-                                 uint32_t aIndexInParent)
-{
-  AccessibleWrap::BindToParent(aParent, aIndexInParent);
-
-  // Cache action content.
-  mActionAcc = nullptr;
-  mIsLink = false;
-  mIsOnclick = false;
-
-  if (nsCoreUtils::HasClickListener(mContent)) {
-    mIsOnclick = true;
-    return;
-  }
-
-  // XXX: The logic looks broken since the click listener may be registered
-  // on non accessible node in parent chain but this node is skipped when tree
-  // is traversed.
-  Accessible* walkUpAcc = this;
-  while ((walkUpAcc = walkUpAcc->Parent()) && !walkUpAcc->IsDoc()) {
-    if (walkUpAcc->LinkState() & states::LINKED) {
-      mIsLink = true;
-      mActionAcc = walkUpAcc;
-      return;
-    }
-
-    if (nsCoreUtils::HasClickListener(walkUpAcc->GetContent())) {
-      mActionAcc = walkUpAcc;
-      mIsOnclick = true;
-      return;
-    }
-  }
-}
-
-void
-LinkableAccessible::UnbindFromParent()
-{
-  mActionAcc = nullptr;
-  mIsLink = false;
-  mIsOnclick = false;
-
-  AccessibleWrap::UnbindFromParent();
-}
 
 ////////////////////////////////////////////////////////////////////////////////
 // DummyAccessible
 ////////////////////////////////////////////////////////////////////////////////
 
 uint64_t
 DummyAccessible::NativeState()
 {
--- a/accessible/generic/BaseAccessibles.h
+++ b/accessible/generic/BaseAccessibles.h
@@ -51,48 +51,43 @@ protected:
  * report the state of the host link (traveled or not) and can activate (click)
  * the host accessible programmatically.
  */
 class LinkableAccessible : public AccessibleWrap
 {
 public:
   enum { eAction_Jump = 0 };
 
-  LinkableAccessible(nsIContent* aContent, DocAccessible* aDoc);
+  LinkableAccessible(nsIContent* aContent, DocAccessible* aDoc) :
+    AccessibleWrap(aContent, aDoc)
+  {
+  }
 
   NS_DECL_ISUPPORTS_INHERITED
 
   // Accessible
-  virtual void Shutdown() override;
   virtual void Value(nsString& aValue) override;
   virtual uint64_t NativeLinkState() const override;
   virtual void TakeFocus() override;
 
   // ActionAccessible
   virtual uint8_t ActionCount() override;
   virtual void ActionNameAt(uint8_t aIndex, nsAString& aName) override;
   virtual bool DoAction(uint8_t index) override;
   virtual KeyBinding AccessKey() const override;
 
+  // ActionAccessible helpers
+  Accessible* ActionWalk(bool* aIsLink = nullptr,
+                          bool* aIsOnclick = nullptr);
   // HyperLinkAccessible
   virtual already_AddRefed<nsIURI> AnchorURIAt(uint32_t aAnchorIndex) override;
 
 protected:
   virtual ~LinkableAccessible() {}
 
-  // Accessible
-  virtual void BindToParent(Accessible* aParent, uint32_t aIndexInParent) override;
-  virtual void UnbindFromParent() override;
-
-  /**
-   * Parent accessible that provides an action for this linkable accessible.
-   */
-  Accessible* mActionAcc;
-  bool mIsLink;
-  bool mIsOnclick;
 };
 
 /**
  * A simple accessible that gets its enumerated role.
  */
 template<a11y::role R>
 class EnumRoleAccessible : public AccessibleWrap
 {