From 79233aaef438f8dc6780520930742208b279c4be Mon Sep 17 00:00:00 2001
From: Neil Deakin <neil@mozilla.com>
Date: Mon, 20 Jul 2015 08:59:50 -0400
Subject: [PATCH] Bug 1182607, use the right rectangle for a popup anchored at
 a rectangle when determining if the mouse click was over the anchor, r=tn

---
 layout/xul/nsMenuPopupFrame.h    |  6 +--
 layout/xul/nsXULPopupManager.cpp | 63 +++++++++++++++++++-------------
 2 files changed, 40 insertions(+), 29 deletions(-)

diff --git a/layout/xul/nsMenuPopupFrame.h b/layout/xul/nsMenuPopupFrame.h
index bf3a5fddec4..a8d7878c46b 100644
--- a/layout/xul/nsMenuPopupFrame.h
+++ b/layout/xul/nsMenuPopupFrame.h
@@ -390,9 +390,9 @@ public:
   // Return the anchor if there is one.
   nsIContent* GetAnchor() const { return mAnchorContent; }
 
-  // Return the screen coordinates of the popup, or (-1, -1) if anchored.
-  // This position is in CSS pixels.
-  nsIntPoint ScreenPosition() const { return mScreenRect.TopLeft(); }
+  // Return the screen coordinates in CSS pixels of the popup,
+  // or (-1, -1, 0, 0) if anchored.
+  nsIntRect GetScreenAnchorRect() const { return mScreenRect; }
 
   nsIntPoint GetLastClientOffset() const { return mLastClientOffset; }
 
diff --git a/layout/xul/nsXULPopupManager.cpp b/layout/xul/nsXULPopupManager.cpp
index 63d8fed4cb9..8e7aa89abf5 100644
--- a/layout/xul/nsXULPopupManager.cpp
+++ b/layout/xul/nsXULPopupManager.cpp
@@ -227,38 +227,49 @@ nsXULPopupManager::Rollup(uint32_t aCount, bool aFlush,
     // when the click was over the anchor. This way, clicking on a menu doesn't
     // reopen the menu.
     if ((consumeResult == ConsumeOutsideClicks_ParentOnly || noRollupOnAnchor) && pos) {
-      nsCOMPtr<nsIContent> anchor = item->Frame()->GetAnchor();
+      nsMenuPopupFrame* popupFrame = item->Frame();
+      nsIntRect anchorRect;
+      if (popupFrame->IsAnchored()) {
+        // Check if the popup has a screen anchor rectangle. If not, get the rectangle
+        // from the anchor element.
+        anchorRect = popupFrame->GetScreenAnchorRect();
+        if (anchorRect.x == -1 || anchorRect.y == -1) {
+          nsCOMPtr<nsIContent> anchor = popupFrame->GetAnchor();
 
-      // Check if the anchor has indicated another node to use for checking
-      // for roll-up. That way, we can anchor a popup on anonymous content or
-      // an individual icon, while clicking elsewhere within a button or other
-      // container doesn't result in us re-opening the popup.
-      if (anchor) {
-        nsAutoString consumeAnchor;
-        anchor->GetAttr(kNameSpaceID_None, nsGkAtoms::consumeanchor,
-                        consumeAnchor);
-        if (!consumeAnchor.IsEmpty()) {
-          nsIDocument* doc = anchor->GetOwnerDocument();
-          nsIContent* newAnchor = doc->GetElementById(consumeAnchor);
-          if (newAnchor) {
-            anchor = newAnchor;
+          // Check if the anchor has indicated another node to use for checking
+          // for roll-up. That way, we can anchor a popup on anonymous content or
+          // an individual icon, while clicking elsewhere within a button or other
+          // container doesn't result in us re-opening the popup.
+          if (anchor) {
+            nsAutoString consumeAnchor;
+            anchor->GetAttr(kNameSpaceID_None, nsGkAtoms::consumeanchor,
+                            consumeAnchor);
+            if (!consumeAnchor.IsEmpty()) {
+              nsIDocument* doc = anchor->GetOwnerDocument();
+              nsIContent* newAnchor = doc->GetElementById(consumeAnchor);
+              if (newAnchor) {
+                anchor = newAnchor;
+              }
+            }
+          }
+
+          if (anchor && anchor->GetPrimaryFrame()) {
+            anchorRect = anchor->GetPrimaryFrame()->GetScreenRect();
           }
         }
       }
 
-      if (anchor && anchor->GetPrimaryFrame()) {
-        // It's possible that some other element is above the anchor at the same
-        // position, but the only thing that would happen is that the mouse
-        // event will get consumed, so here only a quick coordinates check is
-        // done rather than a slower complete check of what is at that location.
-        if (anchor->GetPrimaryFrame()->GetScreenRect().Contains(*pos)) {
-          if (consumeResult == ConsumeOutsideClicks_ParentOnly) {
-            consume = true;
-          }
+      // It's possible that some other element is above the anchor at the same
+      // position, but the only thing that would happen is that the mouse
+      // event will get consumed, so here only a quick coordinates check is
+      // done rather than a slower complete check of what is at that location.
+      if (anchorRect.Contains(*pos)) {
+        if (consumeResult == ConsumeOutsideClicks_ParentOnly) {
+          consume = true;
+        }
 
-          if (noRollupOnAnchor) {
-            rollup = false;
-          }
+        if (noRollupOnAnchor) {
+          rollup = false;
         }
       }
     }