Bug 1424657: Make IAccessible::accChild handle remote ids for popup windows. r?eeejay draft
authorJames Teh <jteh@mozilla.com>
Mon, 11 Dec 2017 14:35:00 +1000
changeset 710802 f845e91d3821cd01b44252edb02bde975fff62e4
parent 710801 3359c0737b92a93e532a56ad7792b4a8171760b0
child 743652 d19a0dc9122ebb47c427d55a35f25226fb211d3d
push id92900
push userbmo:jteh@mozilla.com
push dateTue, 12 Dec 2017 03:41:47 +0000
reviewerseeejay
bugs1424657, 1422201
milestone59.0a1
Bug 1424657: Make IAccessible::accChild handle remote ids for popup windows. r?eeejay Bug 1422201 changed GetIAccessibleFor so it only handles remote ids when called on the root accessible. However, this breaks webextension popup documents. These popups have their own HWND, so the root accessible of that HWND needs to handle accChild for ids in remote documents within that HWND. Therefore, expand the restriction to cover the root accessible of any HWND, not just the main HWND. MozReview-Commit-ID: 69v4XSeQLcS
accessible/windows/msaa/AccessibleWrap.cpp
accessible/windows/msaa/AccessibleWrap.h
--- a/accessible/windows/msaa/AccessibleWrap.cpp
+++ b/accessible/windows/msaa/AccessibleWrap.cpp
@@ -1431,16 +1431,29 @@ GetProxiedAccessibleInSubtree(const DocA
   RefPtr<IDispatch> disp;
   if (FAILED(comProxy->get_accChild(aVarChild, getter_AddRefs(disp)))) {
     return nullptr;
   }
 
   return disp.forget();
 }
 
+bool
+AccessibleWrap::IsRootForHWND()
+{
+  if (IsRoot()) {
+    return true;
+  }
+  HWND thisHwnd = GetHWNDFor(this);
+  AccessibleWrap* parent = static_cast<AccessibleWrap*>(Parent());
+  MOZ_ASSERT(parent);
+  HWND parentHwnd = GetHWNDFor(parent);
+  return thisHwnd != parentHwnd;
+}
+
 already_AddRefed<IAccessible>
 AccessibleWrap::GetIAccessibleFor(const VARIANT& aVarChild, bool* aIsDefunct)
 {
   if (aVarChild.vt != VT_I4)
     return nullptr;
 
   VARIANT varChild = aVarChild;
 
@@ -1479,18 +1492,19 @@ AccessibleWrap::GetIAccessibleFor(const 
   // find it here and should look remotely instead. This handles the case when
   // accessible is part of the chrome process and is part of the xul browser
   // window and the child id points in the content documents. Thus we need to
   // make sure that it is never called on proxies.
   // Bug 1422674: We must only handle remote ids here (< 0), not child indices.
   // Child indices (> 0) are handled below for both local and remote children.
   if (XRE_IsParentProcess() && !IsProxy() &&
       varChild.lVal < 0 && !sIDGen.IsChromeID(varChild.lVal)) {
-    if (!IsRoot()) {
-      // Bug 1422201: accChild with a remote id is only valid on the root accessible.
+    if (!IsRootForHWND()) {
+      // Bug 1422201, 1424657: accChild with a remote id is only valid on the
+      // root accessible for an HWND.
       // Otherwise, we might return remote accessibles which aren't descendants
       // of this accessible. This would confuse clients which use accChild to
       // check whether something is a descendant of a document.
       return nullptr;
     }
     return GetRemoteIAccessibleFor(varChild);
   }
 
--- a/accessible/windows/msaa/AccessibleWrap.h
+++ b/accessible/windows/msaa/AccessibleWrap.h
@@ -176,16 +176,22 @@ public: // construction, destruction
                                    const LayoutDeviceIntRect& aCaretRect);
 
 private:
   static void UpdateSystemCaretFor(HWND aCaretWnd,
                                    const LayoutDeviceIntRect& aCaretRect);
 
 public:
   /**
+   * Determine whether this is the root accessible for its HWND.
+   */
+  bool
+  IsRootForHWND();
+
+  /**
    * Find an accessible by the given child ID in cached documents.
    */
   MOZ_MUST_USE already_AddRefed<IAccessible>
   GetIAccessibleFor(const VARIANT& aVarChild, bool* aIsDefunct);
 
   virtual void GetNativeInterface(void **aOutAccessible) override;
 
   static IDispatch* NativeAccessible(Accessible* aAccessible);