bug 405756 - be careful with coordinateType, r=aaronlev, evan.yan, a=dsicore
authorsurkov.alexander@gmail.com
Sat, 01 Dec 2007 09:30:09 -0800
changeset 8525 1e81ef184676665560b2b6fafb5c4a639448e15d
parent 8524 87b732f2ff122237b34fabec03e6e5ed24537772
child 8526 2ecd6f59355b4a757fc738339d6c4a55e95431d9
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)
reviewersaaronlev, evan.yan, dsicore
bugs405756
milestone1.9b2pre
bug 405756 - be careful with coordinateType, r=aaronlev, evan.yan, a=dsicore
accessible/public/nsIAccessibleImage.idl
accessible/src/atk/nsMaiInterfaceImage.cpp
accessible/src/base/nsAccessibilityUtils.cpp
accessible/src/base/nsAccessibilityUtils.h
accessible/src/html/nsHTMLImageAccessible.cpp
accessible/src/html/nsHTMLImageAccessible.h
accessible/src/html/nsHyperTextAccessible.cpp
accessible/src/msaa/CAccessibleImage.cpp
--- a/accessible/public/nsIAccessibleImage.idl
+++ b/accessible/public/nsIAccessibleImage.idl
@@ -35,16 +35,32 @@
  * ***** END LICENSE BLOCK ***** */
 
 #include "nsISupports.idl"
 
 /**
  *
  * @status UNDER_REVIEW
  */
-[scriptable, uuid(6e80cec3-ff7f-48f2-9823-0b962a0ed508)]
-interface nsIAccessibleImage: nsISupports
+[scriptable, uuid(09086623-0f09-4310-ac56-c2cda7c29648)]
+interface nsIAccessibleImage : nsISupports
 {
-  void getImageBounds(out long x,
-                      out long y,
-                      out long width,
-                      out long height);
+  /**
+   * Returns the coordinates of the image.
+   *
+   * @param coordType  specifies coordinates origin (for available constants
+   *                   refer to nsIAccessibleCoordinateType)
+   * @param x          the x coordinate
+   * @param y          the y coordinate
+   */
+  void getImagePosition(in unsigned long coordType,
+                        out long x,
+                        out long y);
+
+  /**
+   * Returns the size of the image.
+   *
+   * @param width      the heigth
+   * @param height     the width
+   */
+  void getImageSize(out long width, out long height);
 };
+
--- a/accessible/src/atk/nsMaiInterfaceImage.cpp
+++ b/accessible/src/atk/nsMaiInterfaceImage.cpp
@@ -63,29 +63,22 @@ getImagePositionCB(AtkImage *aImage, gin
       return;
 
     nsCOMPtr<nsIAccessibleImage> image;
     accWrap->QueryInterface(NS_GET_IID(nsIAccessibleImage),
                             getter_AddRefs(image));
     if (!image)
       return;
 
-    PRInt32 width, height; // dummy
+    PRUint32 geckoCoordType = (aCoordType == ATK_XY_WINDOW) ?
+      nsIAccessibleCoordinateType::COORDTYPE_WINDOW_RELATIVE :
+      nsIAccessibleCoordinateType::COORDTYPE_SCREEN_RELATIVE;
+
     // Returned in screen coordinates
-    nsresult rv = image->GetImageBounds(aAccX, aAccY, &width, &height);
-    if (NS_FAILED(rv))
-      return;
-    
-    if (aCoordType == ATK_XY_WINDOW) {
-        nsCOMPtr<nsIDOMNode> domNode;
-        accWrap->GetDOMNode(getter_AddRefs(domNode));
-        nsIntPoint winCoords = nsAccUtils::GetScreenCoordsForWindow(domNode);
-        *aAccX -= winCoords.x;
-        *aAccY -= winCoords.y;
-    }
+    image->GetImagePosition(geckoCoordType, aAccX, aAccY);
 }
 
 const gchar *
 getImageDescriptionCB(AtkImage *aImage)
 {
    return getDescriptionCB(ATK_OBJECT(aImage));
 }
 
@@ -97,11 +90,10 @@ getImageSizeCB(AtkImage *aImage, gint *a
       return;
 
     nsCOMPtr<nsIAccessibleImage> image;
     accWrap->QueryInterface(NS_GET_IID(nsIAccessibleImage),
                             getter_AddRefs(image));
     if (!image)
       return;
 
-    PRInt32 x,y; // dummy
-    image->GetImageBounds(&x, &y, aAccWidth, aAccHeight);
+    image->GetImageSize(aAccWidth, aAccHeight);
 }
--- a/accessible/src/base/nsAccessibilityUtils.cpp
+++ b/accessible/src/base/nsAccessibilityUtils.cpp
@@ -431,50 +431,58 @@ nsAccUtils::ConvertToScreenCoords(PRInt3
 
   switch (aCoordinateType) {
     case nsIAccessibleCoordinateType::COORDTYPE_SCREEN_RELATIVE:
       break;
 
     case nsIAccessibleCoordinateType::COORDTYPE_WINDOW_RELATIVE:
     {
       NS_ENSURE_ARG(aAccessNode);
-
-      nsCOMPtr<nsIDOMNode> DOMNode;
-      aAccessNode->GetDOMNode(getter_AddRefs(DOMNode));
-      NS_ENSURE_STATE(DOMNode);
-
-      nsIntPoint wndCoords = nsAccUtils::GetScreenCoordsForWindow(DOMNode);
-      *aCoords += wndCoords;
+      *aCoords += GetScreenCoordsForWindow(aAccessNode);
       break;
     }
 
     case nsIAccessibleCoordinateType::COORDTYPE_PARENT_RELATIVE:
     {
       NS_ENSURE_ARG(aAccessNode);
+      *aCoords += GetScreenCoordsForParent(aAccessNode);
+      break;
+    }
 
-      nsCOMPtr<nsPIAccessNode> parent;
-      nsCOMPtr<nsIAccessible> accessible(do_QueryInterface(aAccessNode));
-      if (accessible) {
-        nsCOMPtr<nsIAccessible> parentAccessible;
-        accessible->GetParent(getter_AddRefs(parentAccessible));
-        parent = do_QueryInterface(parentAccessible);
-      } else {
-        nsCOMPtr<nsIAccessNode> parentAccessNode;
-        aAccessNode->GetParentNode(getter_AddRefs(parentAccessNode));
-        parent = do_QueryInterface(parentAccessNode);
-      }
+    default:
+      return NS_ERROR_INVALID_ARG;
+  }
+
+  return NS_OK;
+}
+
+nsresult
+nsAccUtils::ConvertScreenCoordsTo(PRInt32 *aX, PRInt32 *aY,
+                                  PRUint32 aCoordinateType,
+                                  nsIAccessNode *aAccessNode)
+{
+  switch (aCoordinateType) {
+    case nsIAccessibleCoordinateType::COORDTYPE_SCREEN_RELATIVE:
+      break;
 
-      NS_ENSURE_STATE(parent);
+    case nsIAccessibleCoordinateType::COORDTYPE_WINDOW_RELATIVE:
+    {
+      NS_ENSURE_ARG(aAccessNode);
+      nsIntPoint coords = GetScreenCoordsForWindow(aAccessNode);
+      *aX -= coords.x;
+      *aY -= coords.y;
+      break;
+    }
 
-      nsIFrame *parentFrame = parent->GetFrame();
-      NS_ENSURE_STATE(parentFrame);
-
-      nsIntRect parentRect = parentFrame->GetScreenRectExternal();
-      aCoords->x += parentRect.x;
-      aCoords->y += parentRect.y;
+    case nsIAccessibleCoordinateType::COORDTYPE_PARENT_RELATIVE:
+    {
+      NS_ENSURE_ARG(aAccessNode);
+      nsIntPoint coords = GetScreenCoordsForParent(aAccessNode);
+      *aX -= coords.x;
+      *aY -= coords.y;
       break;
     }
 
     default:
       return NS_ERROR_INVALID_ARG;
   }
 
   return NS_OK;
@@ -501,16 +509,53 @@ nsAccUtils::GetScreenCoordsForWindow(nsI
   if (!windowInter)
     return coords;
 
   windowInter->GetScreenX(&coords.x);
   windowInter->GetScreenY(&coords.y);
   return coords;
 }
 
+nsIntPoint
+nsAccUtils::GetScreenCoordsForWindow(nsIAccessNode *aAccessNode)
+{
+  nsCOMPtr<nsIDOMNode> DOMNode;
+  aAccessNode->GetDOMNode(getter_AddRefs(DOMNode));
+  if (DOMNode)
+    return GetScreenCoordsForWindow(DOMNode);
+
+  return nsIntPoint(0, 0);
+}
+
+nsIntPoint
+nsAccUtils::GetScreenCoordsForParent(nsIAccessNode *aAccessNode)
+{
+  nsCOMPtr<nsPIAccessNode> parent;
+  nsCOMPtr<nsIAccessible> accessible(do_QueryInterface(aAccessNode));
+  if (accessible) {
+    nsCOMPtr<nsIAccessible> parentAccessible;
+    accessible->GetParent(getter_AddRefs(parentAccessible));
+    parent = do_QueryInterface(parentAccessible);
+  } else {
+    nsCOMPtr<nsIAccessNode> parentAccessNode;
+    aAccessNode->GetParentNode(getter_AddRefs(parentAccessNode));
+    parent = do_QueryInterface(parentAccessNode);
+  }
+
+  if (!parent)
+    return nsIntPoint(0, 0);
+
+  nsIFrame *parentFrame = parent->GetFrame();
+  if (!parentFrame)
+    return nsIntPoint(0, 0);
+
+  nsIntRect parentRect = parentFrame->GetScreenRectExternal();
+  return nsIntPoint(parentRect.x, parentRect.y);
+}
+
 already_AddRefed<nsIDocShellTreeItem>
 nsAccUtils::GetDocShellTreeItemFor(nsIDOMNode *aNode)
 {
   if (!aNode)
     return nsnull;
 
   nsCOMPtr<nsIDOMDocument> domDoc;
   aNode->GetOwnerDocument(getter_AddRefs(domDoc));
--- a/accessible/src/base/nsAccessibilityUtils.h
+++ b/accessible/src/base/nsAccessibilityUtils.h
@@ -210,23 +210,52 @@ public:
    * @param aCoords          [out] converted coordinates
    */
   static nsresult ConvertToScreenCoords(PRInt32 aX, PRInt32 aY,
                                         PRUint32 aCoordinateType,
                                         nsIAccessNode *aAccessNode,
                                         nsIntPoint *aCoords);
 
   /**
+   * Converts the given coordinates relative screen to another coordinate
+   * system.
+   *
+   * @param aX               [in, out] the given x coord
+   * @param aY               [in, out] the given y coord
+   * @param aCoordinateType  [in] specifies coordinates origin (refer to
+   *                         nsIAccessibleCoordinateType)
+   * @param aAccessNode      [in] the accessible if coordinates are given
+   *                         relative it
+   */
+  static nsresult ConvertScreenCoordsTo(PRInt32 *aX, PRInt32 *aY,
+                                        PRUint32 aCoordinateType,
+                                        nsIAccessNode *aAccessNode);
+
+  /**
    * Returns coordinates relative screen for the top level window.
    *
-   * @param - aNode - the DOM node hosted in the window.
+   * @param aNode  the DOM node hosted in the window.
    */
   static nsIntPoint GetScreenCoordsForWindow(nsIDOMNode *aNode);
 
   /**
+   * Returns coordinates relative screen for the top level window.
+   *
+   * @param aAccessNode  the accessible hosted in the window
+   */
+  static nsIntPoint GetScreenCoordsForWindow(nsIAccessNode *aAccessNode);
+
+  /**
+   * Returns coordinates relative screen for the parent of the given accessible.
+   *
+   * @param aAccessNode  the accessible
+   */
+  static nsIntPoint GetScreenCoordsForParent(nsIAccessNode *aAccessNode);
+
+  /**
    * Return document shell tree item for the given DOM node.
    */
   static already_AddRefed<nsIDocShellTreeItem>
     GetDocShellTreeItemFor(nsIDOMNode *aNode);
 
   /**
    * Get the ID for an element, in some types of XML this may not be the ID attribute
    * @param aContent  Node to get the ID for
--- a/accessible/src/html/nsHTMLImageAccessible.cpp
+++ b/accessible/src/html/nsHTMLImageAccessible.cpp
@@ -258,19 +258,33 @@ NS_IMETHODIMP nsHTMLImageAccessible::DoA
     NS_ENSURE_TRUE(win, NS_ERROR_FAILURE);
     nsCOMPtr<nsIDOMWindow> tmp;
     return win->Open(longDesc, NS_LITERAL_STRING(""), NS_LITERAL_STRING(""),
                      getter_AddRefs(tmp));
   }
   return nsLinkableAccessible::DoAction(index);
 }
 
-NS_IMETHODIMP nsHTMLImageAccessible::GetImageBounds(PRInt32 *x, PRInt32 *y, PRInt32 *width, PRInt32 *height)
+NS_IMETHODIMP
+nsHTMLImageAccessible::GetImagePosition(PRUint32 aCoordType,
+                                        PRInt32 *aX, PRInt32 *aY)
 {
-  return GetBounds(x, y, width, height);
+  PRInt32 width, height;
+  nsresult rv = GetBounds(aX, aY, &width, &height);
+  if (NS_FAILED(rv))
+    return rv;
+
+  return nsAccUtils::ConvertScreenCoordsTo(aX, aY, aCoordType, this);
+}
+
+NS_IMETHODIMP
+nsHTMLImageAccessible::GetImageSize(PRInt32 *aWidth, PRInt32 *aHeight)
+{
+  PRInt32 x, y;
+  return GetBounds(&x, &y, aWidth, aHeight);
 }
 
 NS_IMETHODIMP
 nsHTMLImageAccessible::Shutdown()
 {
   nsLinkableAccessible::Shutdown();
 
   if (mAccessNodeCache) {
--- a/accessible/src/html/nsHTMLImageAccessible.h
+++ b/accessible/src/html/nsHTMLImageAccessible.h
@@ -61,21 +61,22 @@ public:
   nsHTMLImageAccessible(nsIDOMNode* aDomNode, nsIWeakReference* aShell);
 
   // nsIAccessible
   NS_IMETHOD GetName(nsAString& _retval); 
   NS_IMETHOD GetState(PRUint32 *aState, PRUint32 *aExtraState);
   NS_IMETHOD GetRole(PRUint32 *_retval);
   NS_IMETHOD DoAction(PRUint8 index);
 
-  NS_IMETHOD GetImageBounds(PRInt32 *x, PRInt32 *y, PRInt32 *width, PRInt32 *height);
-
   // nsPIAccessNode
   NS_IMETHOD Shutdown();
 
+  // nsIAccessibleImage
+  NS_DECL_NSIACCESSIBLEIMAGE
+
 protected:
   virtual void CacheChildren();
   already_AddRefed<nsIAccessible> GetAreaAccessible(PRInt32 aAreaNum);
   nsCOMPtr<nsIDOMHTMLMapElement> mMapElement;
 
   // Cache of area accessibles. We do not use common cache because images can
   // share area elements but we need to have separate area accessibles for
   // each image accessible.
--- a/accessible/src/html/nsHyperTextAccessible.cpp
+++ b/accessible/src/html/nsHyperTextAccessible.cpp
@@ -1102,42 +1102,17 @@ NS_IMETHODIMP nsHyperTextAccessible::Get
     return NS_ERROR_FAILURE;
   }
 
   *aX = boundsRect.x;
   *aY = boundsRect.y;
   *aWidth = boundsRect.width;
   *aHeight = boundsRect.height;
 
-  if (aCoordType == nsIAccessibleCoordinateType::COORDTYPE_WINDOW_RELATIVE) {
-    //co-ord type = window
-    nsCOMPtr<nsIPresShell> shell = GetPresShell();
-    NS_ENSURE_TRUE(shell, NS_ERROR_FAILURE);
-    nsCOMPtr<nsIDocument> doc = shell->GetDocument();
-    nsCOMPtr<nsIDOMDocumentView> docView(do_QueryInterface(doc));
-    NS_ENSURE_TRUE(docView, NS_ERROR_FAILURE);
-
-    nsCOMPtr<nsIDOMAbstractView> abstractView;
-    docView->GetDefaultView(getter_AddRefs(abstractView));
-    NS_ENSURE_TRUE(abstractView, NS_ERROR_FAILURE);
-
-    nsCOMPtr<nsIDOMWindowInternal> windowInter(do_QueryInterface(abstractView));
-    NS_ENSURE_TRUE(windowInter, NS_ERROR_FAILURE);
-
-    PRInt32 screenX, screenY;
-    if (NS_FAILED(windowInter->GetScreenX(&screenX)) ||
-        NS_FAILED(windowInter->GetScreenY(&screenY))) {
-      return NS_ERROR_FAILURE;
-    }
-    *aX -= screenX;
-    *aY -= screenY;
-  }
-  // else default: co-ord type = screen
-
-  return NS_OK;
+  return nsAccUtils::ConvertScreenCoordsTo(aX, aY, aCoordType, this);
 }
 
 /*
  * Gets the offset of the character located at coordinates x and y. x and y are interpreted as being relative to
  * the screen or this widget's window depending on coords.
  */
 NS_IMETHODIMP
 nsHyperTextAccessible::GetOffsetAtPoint(PRInt32 aX, PRInt32 aY,
@@ -1149,42 +1124,28 @@ nsHyperTextAccessible::GetOffsetAtPoint(
     return NS_ERROR_FAILURE;
   }
   nsIFrame *hyperFrame = GetFrame();
   if (!hyperFrame) {
     return NS_ERROR_FAILURE;
   }
   nsIntRect frameScreenRect = hyperFrame->GetScreenRectExternal();
 
-  if (aCoordType == nsIAccessibleCoordinateType::COORDTYPE_WINDOW_RELATIVE) {
-    nsCOMPtr<nsIDocument> doc = shell->GetDocument();
-    nsCOMPtr<nsIDOMDocumentView> docView(do_QueryInterface(doc));
-    NS_ENSURE_TRUE(docView, NS_ERROR_FAILURE);
-
-    nsCOMPtr<nsIDOMAbstractView> abstractView;
-    docView->GetDefaultView(getter_AddRefs(abstractView));
-    NS_ENSURE_TRUE(abstractView, NS_ERROR_FAILURE);
-
-    nsCOMPtr<nsIDOMWindowInternal> windowInter(do_QueryInterface(abstractView));
-    NS_ENSURE_TRUE(windowInter, NS_ERROR_FAILURE);
+  nsIntPoint coords;
+  nsresult rv = nsAccUtils::ConvertToScreenCoords(aX, aY, aCoordType,
+                                                  this, &coords);
+  NS_ENSURE_SUCCESS(rv, rv);
 
-    PRInt32 windowX, windowY;
-    if (NS_FAILED(windowInter->GetScreenX(&windowX)) ||
-        NS_FAILED(windowInter->GetScreenY(&windowY))) {
-      return NS_ERROR_FAILURE;
-    }
-    aX += windowX;
-    aY += windowY;
-  }
-  // aX, aY are currently screen coordinates, and we need to turn them into
+  // coords are currently screen coordinates, and we need to turn them into
   // frame coordinates relative to the current accessible
-  if (!frameScreenRect.Contains(aX, aY)) {
+  if (!frameScreenRect.Contains(coords.x, coords.y)) {
     return NS_OK;   // Not found, will return -1
   }
-  nsPoint pointInHyperText(aX - frameScreenRect.x, aY - frameScreenRect.y);
+  nsPoint pointInHyperText(coords.x - frameScreenRect.x,
+                           coords.y - frameScreenRect.y);
   nsPresContext *context = GetPresContext();
   NS_ENSURE_TRUE(context, NS_ERROR_FAILURE);
   pointInHyperText.x = context->DevPixelsToAppUnits(pointInHyperText.x);
   pointInHyperText.y = context->DevPixelsToAppUnits(pointInHyperText.y);
 
   // Go through the frames to check if each one has the point.
   // When one does, add up the character offsets until we have a match
 
--- a/accessible/src/msaa/CAccessibleImage.cpp
+++ b/accessible/src/msaa/CAccessibleImage.cpp
@@ -39,16 +39,17 @@
  * ***** END LICENSE BLOCK ***** */
 
 #include "CAccessibleImage.h"
 
 #include "AccessibleImage_i.c"
 
 #include "nsIAccessible.h"
 #include "nsIAccessibleImage.h"
+#include "nsIAccessibleTypes.h"
 
 #include "nsCOMPtr.h"
 #include "nsString.h"
 
 // IUnknown
 
 STDMETHODIMP
 CAccessibleImage::QueryInterface(REFIID iid, void** ppv)
@@ -83,35 +84,33 @@ CAccessibleImage::get_description(BSTR *
     return E_FAIL;
 
   INT result = ::SysReAllocStringLen(aDescription, description.get(),
                                      description.Length());
   return result ? NS_OK : E_OUTOFMEMORY;
 }
 
 STDMETHODIMP
-CAccessibleImage::get_imagePosition(enum IA2CoordinateType aCoordinateType,
+CAccessibleImage::get_imagePosition(enum IA2CoordinateType aCoordType,
                                     long *aX,
                                     long *aY)
 {
   *aX = 0;
   *aY = 0;
 
-  // XXX: nsIAccessibleImage::getImageBounds() return coordinates relative
-  // to the screen. Make getImageBounds() to use nsIAccessibleCoordinateType to
-  // control it.
-  if (aCoordinateType != IA2_COORDTYPE_SCREEN_RELATIVE)
-    return E_NOTIMPL;
+  PRUint32 geckoCoordType = (aCoordType == IA2_COORDTYPE_SCREEN_RELATIVE) ?
+    nsIAccessibleCoordinateType::COORDTYPE_SCREEN_RELATIVE :
+    nsIAccessibleCoordinateType::COORDTYPE_PARENT_RELATIVE;
 
   nsCOMPtr<nsIAccessibleImage> imageAcc(do_QueryInterface(this));
   if (!imageAcc)
     return E_FAIL;
 
-  PRInt32 x = 0, y = 0, width = 0, height = 0;
-  nsresult rv = imageAcc->GetImageBounds(&x, &y, &width, &height);
+  PRInt32 x = 0, y = 0;
+  nsresult rv = imageAcc->GetImagePosition(geckoCoordType, &x, &y);
   if (NS_FAILED(rv))
     return E_FAIL;
 
   *aX = x;
   *aY = y;
 
   return S_OK;
 }
@@ -122,17 +121,17 @@ CAccessibleImage::get_imageSize(long *aH
   *aHeight = 0;
   *aWidth = 0;
 
   nsCOMPtr<nsIAccessibleImage> imageAcc(do_QueryInterface(this));
   if (!imageAcc)
     return E_FAIL;
 
   PRInt32 x = 0, y = 0, width = 0, height = 0;
-  nsresult rv = imageAcc->GetImageBounds(&x, &y, &width, &height);
+  nsresult rv = imageAcc->GetImageSize(&width, &height);
   if (NS_FAILED(rv))
     return E_FAIL;
 
   *aHeight = width;
   *aWidth = height;
 
   return S_OK;
 }