Bug 764686 - Always check if point are in match's bounds in Accessible::ChildAtPoint(). r=davidb
☠☠ backed out by 8cc7aa5c0c95 ☠ ☠
authorEitan Isaacson <eitan@monotonous.org>
Mon, 18 Jun 2012 18:40:47 -0700
changeset 101785 2168e72ab8d323e9ff8f6f6416fc57ba6fc9783a
parent 101784 e7e1c12cc88cd16341165ccafbc3486dc4a902b0
child 101786 8a7588128430bca0346f89481e3428b3bccd37b7
push id1316
push userakeybl@mozilla.com
push dateMon, 27 Aug 2012 22:37:00 +0000
treeherdermozilla-beta@db4b09302ee2 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdavidb
bugs764686
milestone16.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 764686 - Always check if point are in match's bounds in Accessible::ChildAtPoint(). r=davidb
accessible/src/generic/Accessible.cpp
accessible/tests/mochitest/hittest/test_general.html
--- a/accessible/src/generic/Accessible.cpp
+++ b/accessible/src/generic/Accessible.cpp
@@ -794,62 +794,59 @@ Accessible::ChildAtPoint(PRInt32 aX, PRI
   NS_ASSERTION(contentDocAcc, "could not get the document accessible");
   if (!contentDocAcc)
     return fallbackAnswer;
 
   Accessible* accessible = contentDocAcc->GetAccessibleOrContainer(content);
   if (!accessible)
     return fallbackAnswer;
 
-  if (accessible == this) {
-    // Manually walk through accessible children and see if the are within this
-    // point. Skip offscreen or invisible accessibles. This takes care of cases
-    // where layout won't walk into things for us, such as image map areas and
-    // sub documents (XXX: subdocuments should be handled by methods of
-    // OuterDocAccessibles).
-    PRUint32 childCount = ChildCount();
-    for (PRUint32 childIdx = 0; childIdx < childCount; childIdx++) {
-      Accessible* child = GetChildAt(childIdx);
-
-      PRInt32 childX, childY, childWidth, childHeight;
-      child->GetBounds(&childX, &childY, &childWidth, &childHeight);
-      if (aX >= childX && aX < childX + childWidth &&
-          aY >= childY && aY < childY + childHeight &&
-          (child->State() & states::INVISIBLE) == 0) {
-
-        if (aWhichChild == eDeepestChild)
-          return child->ChildAtPoint(aX, aY, eDeepestChild);
-
-        return child;
-      }
-    }
-
-    // The point is in this accessible but not in a child. We are allowed to
-    // return |this| as the answer.
-    return accessible;
-  }
-
+  // Hurray! We have an accessible for the frame that layout gave us.
   // Since DOM node of obtained accessible may be out of flow then we should
   // ensure obtained accessible is a child of this accessible.
   Accessible* child = accessible;
-  while (true) {
+  while (child != this) {
     Accessible* parent = child->Parent();
     if (!parent) {
       // Reached the top of the hierarchy. These bounds were inside an
       // accessible that is not a descendant of this one.
       return fallbackAnswer;
     }
 
-    if (parent == this)
-      return aWhichChild == eDeepestChild ? accessible : child;
+    // If we landed on a legitimate child of |this|, and we want the direct
+    // child, return it here.
+    if (parent == this && aWhichChild == eDirectChild)
+        return child;
 
     child = parent;
   }
 
-  return nsnull;
+  // Manually walk through accessible children and see if the are within this
+  // point. Skip offscreen or invisible accessibles. This takes care of cases
+  // where layout won't walk into things for us, such as image map areas and
+  // sub documents (XXX: subdocuments should be handled by methods of
+  // OuterDocAccessibles).
+  PRUint32 childCount = accessible->ChildCount();
+  for (PRUint32 childIdx = 0; childIdx < childCount; childIdx++) {
+    Accessible* child = accessible->GetChildAt(childIdx);
+
+    PRInt32 childX, childY, childWidth, childHeight;
+    child->GetBounds(&childX, &childY, &childWidth, &childHeight);
+    if (aX >= childX && aX < childX + childWidth &&
+        aY >= childY && aY < childY + childHeight &&
+        (child->State() & states::INVISIBLE) == 0) {
+
+      if (aWhichChild == eDeepestChild)
+        return child->ChildAtPoint(aX, aY, eDeepestChild);
+        
+      return child;
+    }
+  }
+
+  return accessible;
 }
 
 // nsIAccessible getChildAtPoint(in long x, in long y)
 NS_IMETHODIMP
 Accessible::GetChildAtPoint(PRInt32 aX, PRInt32 aY,
                             nsIAccessible** aAccessible)
 {
   NS_ENSURE_ARG_POINTER(aAccessible);
--- a/accessible/tests/mochitest/hittest/test_general.html
+++ b/accessible/tests/mochitest/hittest/test_general.html
@@ -1,23 +1,33 @@
 <!DOCTYPE html>
 <html>
 <head>
   <title>nsIAccessible::childAtPoint() tests</title>
   <link rel="stylesheet" type="text/css" href="chrome://mochikit/content/tests/SimpleTest/test.css" />
 
   <script type="application/javascript"
           src="chrome://mochikit/content/tests/SimpleTest/SimpleTest.js"></script>
+  <script type="application/javascript"
+          src="chrome://mochikit/content/tests/SimpleTest/EventUtils.js"></script>
 
   <script type="application/javascript"
           src="../common.js"></script>
   <script type="application/javascript"
           src="../layout.js"></script>
+  <script type="application/javascript"
+          src="../events.js"></script>
 
   <script type="application/javascript">
+    function doPreTest()
+    {
+      var imgMap = document.getElementById("imgmap");
+      waitForImageMap(imgMap, doTest);
+    }
+
     function doTest()
     {
       // Not specific case, child and deepchild testing.
       var list = getAccessible("list");
       var listitem = getAccessible("listitem");
       var image = getAccessible("image");
 if (!MAC) {
       testChildAtPoint(list, 1, 1, listitem, image.firstChild);
@@ -49,21 +59,33 @@ if (!MAC) {
       // because it's not a child of the accessible even visually it is.
       var rectArea = getNode("area").getBoundingClientRect();
       var outOfFlow = getNode("outofflow");
       outOfFlow.style.left = rectArea.left + "px";
       outOfFlow.style.top = rectArea.top + "px";
 
       testChildAtPoint("area", 1, 1, "area", "area");
 
+      var container = getAccessible("container");
+      var paragraph = getAccessible("paragraph");
+      var [tx, ty, tw, th] = getBounds(paragraph);
+      var [cx, cy, cw, ch] = getBounds(container);
+      // Test the point in the vertical center of the paragraph, between the two lines.
+      testChildAtPoint("container", tx - cx + 1, ty - cy + Math.round(tw/2), paragraph, paragraph.firstChild);
+
+      // Test image maps. Their children are not in the layout tree.
+      var theLetterA = getAccessible("imgmap").firstChild;
+      hitTest("imgmap", theLetterA, theLetterA);
+      hitTest("container", "imgmap", theLetterA);
+
       SimpleTest.finish();
     }
 
     SimpleTest.waitForExplicitFinish();
-    addA11yLoadEvent(doTest);
+    addA11yLoadEvent(doPreTest);
   </script>
 </head>
 <body>
 
   <a target="_blank"
      href="https://bugzilla.mozilla.org/show_bug.cgi?id=491657"
      title="nsIAccessible::childAtPoint() tests">Mozilla Bug 491657</a>
   <p id="display"></p>
@@ -77,10 +99,21 @@ if (!MAC) {
 
   <span role="button">button1</span><span role="button" id="btn">button2</span>
 
   <span role="textbox">textbox1</span><span role="textbox" id="txt">textbox2</span>
 
   <div id="outofflow" style="width: 10px; height: 10px; position: absolute; left: 0px; top: 0px; background-color: yellow;">
   </div>
   <div id="area" style="width: 100px; height: 100px; background-color: blue;"></div>
+
+  <map name="atoz_map">
+    <area href="http://www.bbc.co.uk/radio4/atoz/index.shtml#a"
+          coords="0,0,15,15" alt="thelettera" shape="rect"/>
+  </map>
+
+  <div id="container">
+    <p id="paragraph" style="width: 5em; line-height: 3em;">Hello World</p>
+    <img id="imgmap" width="447" height="15" usemap="#atoz_map" src="../letters.gif"/>
+  </div>
+
 </body>
 </html>