Backing out changeset c0424e568b5e from bug 457369 because it breaks the market-leading screen reader
authorMarco Zehe <marco.zehe@googlemail.com>
Fri, 10 Oct 2008 16:27:44 +0200
changeset 20256 5c4d829c1a57a2d9ef2694cef08c242cd146a248
parent 20247 c0424e568b5eac407f2e5782313c578edaa3bf6c
child 20257 55ab8cb47c1f796a35add6a287f474ca7c262862
push id1
push userroot
push dateTue, 26 Apr 2011 22:38:44 +0000
treeherdermozilla-beta@bfdb6e623a36 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs457369
milestone1.9.1b2pre
Backing out changeset c0424e568b5e from bug 457369 because it breaks the market-leading screen reader
accessible/src/base/nsAccessNode.cpp
accessible/src/msaa/nsAccessNodeWrap.cpp
accessible/src/msaa/nsAccessNodeWrap.h
accessible/tests/mochitest/Makefile.in
accessible/tests/mochitest/test_bug434464.html
accessible/tests/mochitest/test_nsIAccessNode_invalidation.html
--- a/accessible/src/base/nsAccessNode.cpp
+++ b/accessible/src/base/nsAccessNode.cpp
@@ -490,42 +490,46 @@ nsAccessNode::ScrollToPoint(PRUint32 aCo
     nsAccUtils::ScrollFrameToPoint(parentFrame, frame, coords);
 
   return NS_OK;
 }
 
 nsresult
 nsAccessNode::MakeAccessNode(nsIDOMNode *aNode, nsIAccessNode **aAccessNode)
 {
+  *aAccessNode = nsnull;
+  
   nsIAccessibilityService *accService = GetAccService();
-  NS_ENSURE_STATE(accService);
+  NS_ENSURE_TRUE(accService, NS_ERROR_FAILURE);
+
+  nsCOMPtr<nsIAccessNode> accessNode;
+  accService->GetCachedAccessNode(aNode, mWeakShell, getter_AddRefs(accessNode));
 
-  // XXX: always try to get accessible, see bug 434464. This should be fixed
-  // in bug 453832.
-  nsCOMPtr<nsIAccessible> accessible;
-  accService->GetAccessibleInWeakShell(aNode, mWeakShell,
-                                       getter_AddRefs(accessible));
+  if (!accessNode) {
+    nsCOMPtr<nsIAccessible> accessible;
+    accService->GetAccessibleInWeakShell(aNode, mWeakShell,
+                                         getter_AddRefs(accessible));
 
-  if (accessible) {
-    CallQueryInterface(accessible, aAccessNode);
+    accessNode = do_QueryInterface(accessible);
+  }
+
+  if (accessNode) {
+    NS_ADDREF(*aAccessNode = accessNode);
     return NS_OK;
   }
 
-  // Try to get access node from cache. XXX: see bug 453832 for invalidation
-  // problems.
-  accService->GetCachedAccessNode(aNode, mWeakShell, aAccessNode);
-  if (*aAccessNode)
-    return NS_OK;
-
-  nsRefPtr<nsAccessNode> newAccessNode =
-    new nsAccessNodeWrap(aNode, mWeakShell);
-  NS_ENSURE_TRUE(newAccessNode, NS_ERROR_OUT_OF_MEMORY);
+  nsAccessNode *newAccessNode = new nsAccessNode(aNode, mWeakShell);
+  if (!newAccessNode) {
+    return NS_ERROR_OUT_OF_MEMORY;
+  }
 
   NS_ADDREF(*aAccessNode = newAccessNode);
-  return newAccessNode->Init();
+  newAccessNode->Init();
+
+  return NS_OK;
 }
 
 NS_IMETHODIMP
 nsAccessNode::GetFirstChildNode(nsIAccessNode **aAccessNode)
 {
   NS_ENSURE_ARG_POINTER(aAccessNode);
   *aAccessNode = nsnull;
   NS_ENSURE_TRUE(mDOMNode, NS_ERROR_NULL_POINTER);
--- a/accessible/src/msaa/nsAccessNodeWrap.cpp
+++ b/accessible/src/msaa/nsAccessNodeWrap.cpp
@@ -376,114 +376,154 @@ STDMETHODIMP nsAccessNodeWrap::scrollTo(
   nsresult rv = ScrollTo(scrollType);
   if (NS_SUCCEEDED(rv))
     return S_OK;
 } __except(FilterA11yExceptions(::GetExceptionCode(), GetExceptionInformation())) { }
 
   return E_FAIL;
 }
 
-STDMETHODIMP
-nsAccessNodeWrap::get_parentNode(ISimpleDOMNode __RPC_FAR *__RPC_FAR *aNode)
+ISimpleDOMNode* nsAccessNodeWrap::MakeAccessNode(nsIDOMNode *node)
+{
+  if (!node) 
+    return NULL;
+
+  nsAccessNodeWrap *newNode = NULL;
+  
+  nsCOMPtr<nsIContent> content(do_QueryInterface(node));
+  nsCOMPtr<nsIDocument> doc;
+
+  if (content) 
+    doc = content->GetDocument();
+  else {
+    // Get the document via QueryInterface, since there is no content node
+    doc = do_QueryInterface(node);
+    content = do_QueryInterface(node);
+  }
+
+  if (!doc)
+    return NULL;
+
+  nsCOMPtr<nsIAccessibilityService> accService(do_GetService("@mozilla.org/accessibilityService;1"));
+  if (!accService)
+    return NULL;
+
+  ISimpleDOMNode *iNode = NULL;
+  nsCOMPtr<nsIAccessible> nsAcc;
+  accService->GetAccessibleInWeakShell(node, mWeakShell, getter_AddRefs(nsAcc));
+  if (nsAcc) {
+    nsCOMPtr<nsIAccessNode> accessNode(do_QueryInterface(nsAcc));
+    NS_ASSERTION(accessNode, "nsIAccessible impl does not inherit from nsIAccessNode");
+    IAccessible *msaaAccessible;
+    nsAcc->GetNativeInterface((void**)&msaaAccessible); // addrefs
+    msaaAccessible->QueryInterface(IID_ISimpleDOMNode, (void**)&iNode); // addrefs
+    msaaAccessible->Release(); // Release IAccessible
+  }
+  else {
+    newNode = new nsAccessNodeWrap(node, mWeakShell);
+    if (!newNode)
+      return NULL;
+
+    newNode->Init();
+    iNode = static_cast<ISimpleDOMNode*>(newNode);
+    iNode->AddRef();
+  }
+
+  return iNode;
+}
+
+
+STDMETHODIMP nsAccessNodeWrap::get_parentNode(ISimpleDOMNode __RPC_FAR *__RPC_FAR *aNode)
 {
 __try {
-  *aNode = NULL;
-
-  nsCOMPtr<nsIAccessNode> accessNode;
-  nsresult rv = GetParentNode(getter_AddRefs(accessNode));
-  if (NS_FAILED(rv))
-    return GetHRESULT(rv);
-
-  return QueryISimpleDOMNode(accessNode, aNode);
-} __except(FilterA11yExceptions(::GetExceptionCode(), GetExceptionInformation())) { }
-
-  return S_OK;
-}
-
-STDMETHODIMP
-nsAccessNodeWrap::get_firstChild(ISimpleDOMNode __RPC_FAR *__RPC_FAR *aNode)
-{
-__try {
-  *aNode = NULL;
-
-  nsCOMPtr<nsIAccessNode> accessNode;
-  nsresult rv = GetFirstChildNode(getter_AddRefs(accessNode));
-  if (NS_FAILED(rv))
-    return GetHRESULT(rv);
-  
-  return QueryISimpleDOMNode(accessNode, aNode);
+  if (!mDOMNode)
+    return E_FAIL;
+ 
+  nsCOMPtr<nsIDOMNode> node;
+  mDOMNode->GetParentNode(getter_AddRefs(node));
+  *aNode = MakeAccessNode(node);
 } __except(FilterA11yExceptions(::GetExceptionCode(), GetExceptionInformation())) { }
 
   return S_OK;
 }
 
-STDMETHODIMP
-nsAccessNodeWrap::get_lastChild(ISimpleDOMNode __RPC_FAR *__RPC_FAR *aNode)
+STDMETHODIMP nsAccessNodeWrap::get_firstChild(ISimpleDOMNode __RPC_FAR *__RPC_FAR *aNode)
 {
 __try {
-  *aNode = NULL;
-
-  nsCOMPtr<nsIAccessNode> accessNode;
-  nsresult rv = GetLastChildNode(getter_AddRefs(accessNode));
-  if (NS_FAILED(rv))
-    return GetHRESULT(rv);
-  
-  return QueryISimpleDOMNode(accessNode, aNode);
+  if (!mDOMNode)
+    return E_FAIL;
+ 
+  nsCOMPtr<nsIDOMNode> node;
+  mDOMNode->GetFirstChild(getter_AddRefs(node));
+  *aNode = MakeAccessNode(node);
 } __except(FilterA11yExceptions(::GetExceptionCode(), GetExceptionInformation())) { }
 
   return S_OK;
 }
 
-STDMETHODIMP
-nsAccessNodeWrap::get_previousSibling(ISimpleDOMNode __RPC_FAR *__RPC_FAR *aNode)
+STDMETHODIMP nsAccessNodeWrap::get_lastChild(ISimpleDOMNode __RPC_FAR *__RPC_FAR *aNode)
 {
-__try {
-  *aNode = NULL;
+  __try {
+  if (!mDOMNode)
+    return E_FAIL;
 
-  nsCOMPtr<nsIAccessNode> accessNode;
-  nsresult rv = GetPreviousSiblingNode(getter_AddRefs(accessNode));
-  if (NS_FAILED(rv))
-    return GetHRESULT(rv);
-  
-  return QueryISimpleDOMNode(accessNode, aNode);
+  nsCOMPtr<nsIDOMNode> node;
+  mDOMNode->GetLastChild(getter_AddRefs(node));
+  *aNode = MakeAccessNode(node);
 } __except(FilterA11yExceptions(::GetExceptionCode(), GetExceptionInformation())) { }
 
   return S_OK;
 }
 
-STDMETHODIMP
-nsAccessNodeWrap::get_nextSibling(ISimpleDOMNode __RPC_FAR *__RPC_FAR *aNode)
+STDMETHODIMP nsAccessNodeWrap::get_previousSibling(ISimpleDOMNode __RPC_FAR *__RPC_FAR *aNode)
 {
 __try {
-  *aNode = NULL;
+  if (!mDOMNode)
+    return E_FAIL;
+
+  nsCOMPtr<nsIDOMNode> node;
+  mDOMNode->GetPreviousSibling(getter_AddRefs(node));
+  *aNode = MakeAccessNode(node);
+} __except(FilterA11yExceptions(::GetExceptionCode(), GetExceptionInformation())) { }
 
-  nsCOMPtr<nsIAccessNode> accessNode;
-  nsresult rv = GetNextSiblingNode(getter_AddRefs(accessNode));
-  if (NS_FAILED(rv))
-    return GetHRESULT(rv);
-  
-  return QueryISimpleDOMNode(accessNode, aNode);
+  return S_OK;
+}
+
+STDMETHODIMP nsAccessNodeWrap::get_nextSibling(ISimpleDOMNode __RPC_FAR *__RPC_FAR *aNode)
+{
+__try {
+  if (!mDOMNode)
+    return E_FAIL;
+
+  nsCOMPtr<nsIDOMNode> node;
+  mDOMNode->GetNextSibling(getter_AddRefs(node));
+  *aNode = MakeAccessNode(node);
 } __except(FilterA11yExceptions(::GetExceptionCode(), GetExceptionInformation())) { }
 
   return S_OK;
 }
 
 STDMETHODIMP 
 nsAccessNodeWrap::get_childAt(unsigned aChildIndex,
                               ISimpleDOMNode __RPC_FAR *__RPC_FAR *aNode)
 {
 __try {
-  *aNode = NULL;
+  *aNode = nsnull;
+
+  nsCOMPtr<nsIContent> content(do_QueryInterface(mDOMNode));
+  if (!content)
+    return E_FAIL;  // Node already shut down
 
-  nsCOMPtr<nsIAccessNode> accessNode;
-  nsresult rv = GetChildNodeAt(aChildIndex, getter_AddRefs(accessNode));
-  if (NS_FAILED(rv))
-    return GetHRESULT(rv);
-  
-  return QueryISimpleDOMNode(accessNode, aNode);
+  nsCOMPtr<nsIDOMNode> node =
+    do_QueryInterface(content->GetChildAt(aChildIndex));
+
+  if (!node)
+    return E_FAIL; // No such child
+
+  *aNode = MakeAccessNode(node);
 } __except(FilterA11yExceptions(::GetExceptionCode(), GetExceptionInformation())) { }
 
   return S_OK;
 }
 
 STDMETHODIMP 
 nsAccessNodeWrap::get_innerHTML(BSTR __RPC_FAR *aInnerHTML)
 {
@@ -597,37 +637,16 @@ int nsAccessNodeWrap::FilterA11yExceptio
   }
   else {
     NS_NOTREACHED("We should only be catching crash exceptions");
   }
   return EXCEPTION_CONTINUE_SEARCH;
 }
 
 HRESULT
-nsAccessNodeWrap::QueryISimpleDOMNode(nsIAccessNode *aAccessNode,
-                                      ISimpleDOMNode __RPC_FAR *__RPC_FAR *aNode)
-{
-  if (!aAccessNode)
-    return S_OK;
-
-  nsCOMPtr<nsIWinAccessNode> winAccessNode(do_QueryInterface(aAccessNode));
-  if (!winAccessNode)
-    return E_FAIL;
-
-  void *instancePtr = NULL;
-  nsresult rv =  winAccessNode->QueryNativeInterface(IID_ISimpleDOMNode,
-                                                     &instancePtr);
-  if (NS_FAILED(rv))
-    return GetHRESULT(rv);
-
-  *aNode = static_cast<ISimpleDOMNode*>(instancePtr);
-  return S_OK;
-}
-
-HRESULT
 GetHRESULT(nsresult aResult)
 {
   switch (aResult) {
     case NS_OK:
       return S_OK;
 
     case NS_ERROR_INVALID_ARG: case NS_ERROR_INVALID_POINTER:
       return E_INVALIDARG;
--- a/accessible/src/msaa/nsAccessNodeWrap.h
+++ b/accessible/src/msaa/nsAccessNodeWrap.h
@@ -161,22 +161,16 @@ class nsAccessNodeWrap :  public nsAcces
 
     static void TurnOffNewTabSwitchingForJawsAndWE();
 
     static void DoATSpecificProcessing();
   protected:
     void GetAccessibleFor(nsIDOMNode *node, nsIAccessible **newAcc);
     ISimpleDOMNode* MakeAccessNode(nsIDOMNode *node);
 
-    /**
-     * Query ISimpleDOMNode interface from nsIAccessNode object.
-     */
-    static HRESULT QueryISimpleDOMNode(nsIAccessNode *aAccessNode,
-                                       ISimpleDOMNode __RPC_FAR *__RPC_FAR *aNode);
-
     static PRBool gIsEnumVariantSupportDisabled;
 
     /**
      * Used to determine whether an IAccessible2 compatible screen reader is
      * loaded. Currently used for JAWS versions older than 8.0.2173.
      */
      static PRBool gIsIA2Disabled;
 
--- a/accessible/tests/mochitest/Makefile.in
+++ b/accessible/tests/mochitest/Makefile.in
@@ -80,21 +80,21 @@ include $(topsrcdir)/config/rules.mk
 		test_nsIAccessibleHyperLink.xul \
 		test_nsIAccessibleHyperText.html \
 		test_nsIAccessibleImage.html \
 		test_nsIAccessibleTable_1.html \
 		test_nsIAccessibleTable_2.html \
 		test_nsIAccessibleTable_3.html \
 		test_nsIAccessibleTable_4.html \
 		test_nsIAccessibleTable_listboxes.xul \
-		test_nsIAccessNode_invalidation.html \
 		test_nsIAccessNode_utils.html \
 		test_nsOuterDocAccessible.html \
 		test_textattrs.html \
 		test_textboxes.html \
 		test_textboxes.xul \
 		testTextboxes.js \
 		test_bug428479.html \
 		test_bug429285.html \
+		test_bug434464.html \
 		$(NULL)
 
 libs:: $(_TEST_FILES)
 	$(INSTALL) $(foreach f,$^,"$f") $(DEPTH)/_tests/testing/mochitest/a11y/$(relativesrcdir)
new file mode 100644
--- /dev/null
+++ b/accessible/tests/mochitest/test_bug434464.html
@@ -0,0 +1,88 @@
+<!DOCTYPE html>
+<html>
+<!--
+https://bugzilla.mozilla.org/show_bug.cgi?id=434464
+-->
+<head>
+  <title>Test NSIAccessNode cache invalidation</title>
+  <link rel="stylesheet" type="text/css" href="chrome://mochikit/content/tests/SimpleTest/test.css" />
+
+  <script type="application/javascript" src="chrome://mochikit/content/MochiKit/packed.js"></script>
+  <script type="application/javascript" src="chrome://mochikit/content/tests/SimpleTest/SimpleTest.js"></script>
+
+  <script type="application/javascript">
+    function doTest()
+    {
+      var accRetrieval = Components.classes["@mozilla.org/accessibleRetrieval;1"].
+                       getService(Components.interfaces.nsIAccessibleRetrieval);
+
+      var parentElm = document.getElementById("parent");
+      if (!parentElm) {
+        ok(false, "no parent element for paragraph!");
+        SimpleTest.finish();
+      }
+
+      var elm = document.getElementById("para");
+      if (!elm) {
+        ok(false, "no element for paragraph!");
+        SimpleTest.finish();
+      }
+
+      // It's currently hidden. Ask for its parent's nsIAccessNode.
+      var parentElmAccNode = null;
+      try {
+        parentElmAccNode = accRetrieval.getAccessibleFor(parentElm).
+                     QueryInterface(Components.interfaces.nsIAccessNode);
+      } catch(e) {}
+
+      if (!parentElmAccNode) {
+        ok(false, "No accessNode for parent of hidden paragraph!");
+        SimpleTest.finish();
+      }
+
+      // Get the paragraph's accessNode.
+      var elmAccNode = null;
+      try {
+        elmAccNode = parentElmAccNode.firstChildNode;
+      } catch(e) {}
+
+      if (!elmAccNode) {
+        ok(false, "No accessNode for hidden paragraph!");
+        SimpleTest.finish();
+      }
+
+      // Now make the paragraph visible. This invalidates the just-retrieved
+      // AccessNode. An nsIAccessible should then be retrievable.
+      elm.style.display = "block";
+
+      // Now, get an accessible for it. Use a timeout so the layout engine can
+      // catch up.
+      window.setTimeout(
+        function()
+        {
+          var elmAcc = null;
+          try {
+            elmAcc = accRetrieval.getAccessibleFor(elm);
+          } catch(e) {}
+    
+          ok(elmAcc, "No accessible for paragraph after it became visible!");
+
+          SimpleTest.finish();
+        },
+      200);
+    }
+
+    SimpleTest.waitForExplicitFinish();
+    addLoadEvent(doTest);
+  </script>
+</head>
+<body>
+
+  <a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=434464">Mozilla Bug 434464</a>
+  <p id="display"></p>
+  <div id="content" style="display: none"></div>
+  <pre id="test">
+  </pre>
+  <div id="parent"><p style="display: none;" id="para">I'm hidden initially, but then made visible.</p></div>
+</body>
+</html>
deleted file mode 100644
--- a/accessible/tests/mochitest/test_nsIAccessNode_invalidation.html
+++ /dev/null
@@ -1,85 +0,0 @@
-<!DOCTYPE html>
-<html>
-<!--
-https://bugzilla.mozilla.org/show_bug.cgi?id=434464
--->
-<head>
-  <title>Test NSIAccessNode cache invalidation</title>
-  <link rel="stylesheet" type="text/css" href="chrome://mochikit/content/tests/SimpleTest/test.css" />
-
-  <script type="application/javascript"
-          src="chrome://mochikit/content/MochiKit/packed.js"></script>
-  <script type="application/javascript"
-          src="chrome://mochikit/content/tests/SimpleTest/SimpleTest.js"></script>
-
-  <script type="application/javascript"
-          src="chrome://mochikit/content/a11y/accessible/common.js"></script>
-
-  <script type="application/javascript">
-    function doTest()
-    {
-      var parentElmAccNode = getAccessible("parent", nsIAccessNode);
-
-      // Get the paragraph's accessNode.
-      var elmAccNode = null;
-      try {
-        elmAccNode = parentElmAccNode.firstChildNode;
-      } catch(e) {}
-
-      if (!elmAccNode) {
-        ok(false, "No accessNode for hidden paragraph!");
-        SimpleTest.finish();
-      }
-
-      var elmAccNode2 = null;
-      try {
-        elmAccNode2 = parentElmAccNode.firstChildNode;
-      } catch(e) {}
-
-      if (!elmAccNode2) {
-        ok(false, "No accessNode for hidden paragraph!");
-        SimpleTest.finish();
-      }
-
-      // bug 457369
-      is(elmAccNode, elmAccNode2,
-         "Different access nodes for the same DOM node!");
-
-      // Now make the paragraph visible. This invalidates the just-retrieved
-      // AccessNode. An nsIAccessible should then be retrievable.
-      var elm = document.getElementById("p");
-      if (!elm) {
-        ok(false, "no element for paragraph!");
-        SimpleTest.finish();
-      }
-
-      elm.style.display = "block";
-
-      // Now, get an accessible for it. Use a timeout so the layout engine can
-      // catch up.
-      window.setTimeout(
-        function()
-        {
-          var elmAcc = getAccessible(elm);    
-          ok(elmAcc, "No accessible for paragraph after it became visible!");
-
-          SimpleTest.finish();
-        },
-      200);
-    }
-
-    SimpleTest.waitForExplicitFinish();
-    addLoadEvent(doTest);
-  </script>
-</head>
-<body>
-
-  <a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=434464">Mozilla Bug 434464</a>
-  <p id="display"></p>
-  <div id="content" style="display: none"></div>
-  <pre id="test">
-  </pre>
-
-  <div id="parent"><p style="display: none;" id="p">I'm hidden initially, but then made visible.</p></div>
-</body>
-</html>