Bug 402548, 402551, popups positioned wrong when document is zoomed. Affects anchored popups anchored in a different document, and popups opened at a given screen position, r+sr=roc

2007-11-22 06:12:03 -08:00 · 2007-11-22 06:12:03 -08:00 · 2a85df3528
--- a/layout/xul/base/src/nsMenuPopupFrame.cpp
+++ b/layout/xul/base/src/nsMenuPopupFrame.cpp
@ -654,7 +654,7 @@ nsMenuPopupFrame::GetRootViewForPopup(nsIFrame* aStartFrame)
 // the left or right edge of the parent.
 // 
 void
-nsMenuPopupFrame::AdjustPositionForAnchorAlign(PRInt32* ioXPos, PRInt32* ioYPos, const nsRect & inParentRect,
+nsMenuPopupFrame::AdjustPositionForAnchorAlign(PRInt32* ioXPos, PRInt32* ioYPos, const nsSize & inParentSize,
                                               PRBool* outFlushWithTopBottom)
 {
  PRInt8 popupAnchor(mPopupAnchor);
@ -683,23 +683,23 @@ nsMenuPopupFrame::AdjustPositionForAnchorAlign(PRInt32* ioXPos, PRInt32* ioYPos,
  }
  
  if (popupAnchor == POPUPALIGNMENT_TOPRIGHT && popupAlign == POPUPALIGNMENT_TOPLEFT) {
-    *ioXPos += inParentRect.width;
+    *ioXPos += inParentSize.width;
  }
  else if (popupAnchor == POPUPALIGNMENT_TOPLEFT && popupAlign == POPUPALIGNMENT_TOPLEFT) {
    *outFlushWithTopBottom = PR_TRUE;
  }
  else if (popupAnchor == POPUPALIGNMENT_TOPRIGHT && popupAlign == POPUPALIGNMENT_BOTTOMRIGHT) {
-    *ioXPos -= (mRect.width - inParentRect.width);
+    *ioXPos -= (mRect.width - inParentSize.width);
    *ioYPos -= mRect.height;
    *outFlushWithTopBottom = PR_TRUE;
  }
  else if (popupAnchor == POPUPALIGNMENT_BOTTOMRIGHT && popupAlign == POPUPALIGNMENT_BOTTOMLEFT) {
-    *ioXPos += inParentRect.width;
-    *ioYPos -= (mRect.height - inParentRect.height);
+    *ioXPos += inParentSize.width;
+    *ioYPos -= (mRect.height - inParentSize.height);
  }
  else if (popupAnchor == POPUPALIGNMENT_BOTTOMRIGHT && popupAlign == POPUPALIGNMENT_TOPRIGHT) {
-    *ioXPos -= (mRect.width - inParentRect.width);
-    *ioYPos += inParentRect.height;
+    *ioXPos -= (mRect.width - inParentSize.width);
+    *ioYPos += inParentSize.height;
    *outFlushWithTopBottom = PR_TRUE;
  }
  else if (popupAnchor == POPUPALIGNMENT_TOPLEFT && popupAlign == POPUPALIGNMENT_TOPRIGHT) {
@ -711,10 +711,10 @@ nsMenuPopupFrame::AdjustPositionForAnchorAlign(PRInt32* ioXPos, PRInt32* ioYPos,
  }
  else if (popupAnchor == POPUPALIGNMENT_BOTTOMLEFT && popupAlign == POPUPALIGNMENT_BOTTOMRIGHT) {
    *ioXPos -= mRect.width;
-    *ioYPos -= (mRect.height - inParentRect.height);
+    *ioYPos -= (mRect.height - inParentSize.height);
  }
  else if (popupAnchor == POPUPALIGNMENT_BOTTOMLEFT && popupAlign == POPUPALIGNMENT_TOPLEFT) {
-    *ioYPos += inParentRect.height;
+    *ioYPos += inParentSize.height;
    *outFlushWithTopBottom = PR_TRUE;
  }
  else
@ -860,14 +860,24 @@ nsMenuPopupFrame::SetPopupPosition(nsIFrame* aAnchorFrame)
    sizedToPopup = nsMenuFrame::IsSizedToPopup(aAnchorFrame->GetContent(), PR_FALSE);
  }

-  // |parentRect|
-  //   The dimensions of the frame invoking the popup. 
-  nsRect parentRect = aAnchorFrame->GetRect();
+  // |ParentSize|
+  //   The dimensions of the anchor in its app units
+  nsSize parentSize = aAnchorFrame->GetSize();
+
+  // the anchor may be in a different document with a different scale,
+  // so adjust the size so that it is in the app units of the popup instead
+  // of the anchor. This is done by converting to device pixels by dividing
+  // by the anchor's app units per device pixel and then converting back to
+  // app units by multiplying by the popup's app units per device pixel.
+  float adj = float(presContext->AppUnitsPerDevPixel()) /
+              aAnchorFrame->PresContext()->AppUnitsPerDevPixel();
+  parentSize.width = NSToCoordCeil(parentSize.width * adj);
+  parentSize.height = NSToCoordCeil(parentSize.height * adj);

  // If we stick to our parent's width, set it here before we move the
  // window around, because moving is done with respect to the width...
  if (sizedToPopup) {
-    mRect.width = parentRect.width;
+    mRect.width = parentSize.width;
  }

  // |xpos| and |ypos| hold the x and y positions of where the popup will be moved to,
@ -883,6 +893,7 @@ nsMenuPopupFrame::SetPopupPosition(nsIFrame* aAnchorFrame)
  nsRect anchorScreenRect;
  nsRect rootScreenRect = rootFrame->GetScreenRect();

+  nsIDeviceContext* devContext = PresContext()->DeviceContext();
  if (mScreenXPos == -1 && mScreenYPos == -1) {
    // if we are anchored to our parent, there are certain things we don't want to do
    // when repositioning the view to fit on the screen, such as end up positioned over
@ -896,7 +907,7 @@ nsMenuPopupFrame::SetPopupPosition(nsIFrame* aAnchorFrame)

      // move the popup according to the anchor and alignment. This will also tell us
      // which axis the popup is flush against in case we have to move it around later.
-      AdjustPositionForAnchorAlign(&xpos, &ypos, parentRect, &readjustAboveBelow);
+      AdjustPositionForAnchorAlign(&xpos, &ypos, parentSize, &readjustAboveBelow);
    }
    else {
      // with no anchor, the popup is positioned relative to the root frame
@ -914,10 +925,19 @@ nsMenuPopupFrame::SetPopupPosition(nsIFrame* aAnchorFrame)
    screenViewLocY = presContext->DevPixelsToAppUnits(rootScreenRect.y) + ypos;
  }
  else {
-    // positioned on screen
+    // the popup is positioned at a screen coordinate.
+    // first convert the screen position in mScreenXPos and mScreenYPos from
+    // CSS pixels into device pixels, ignoring any scaling as mScreenXPos and
+    // mScreenYPos are unscaled screen coordinates.
+    PRInt32 factor = devContext->UnscaledAppUnitsPerDevPixel();
+    screenViewLocX = nsPresContext::CSSPixelsToAppUnits(mScreenXPos) / factor;
+    screenViewLocY = nsPresContext::CSSPixelsToAppUnits(mScreenYPos) / factor;
+
+    // next, convert back into app units accounting for the scaling,
+    // and add the margins on the popup
    GetStyleMargin()->GetMargin(margin);
-    screenViewLocX = nsPresContext::CSSPixelsToAppUnits(mScreenXPos) + margin.left;
-    screenViewLocY = nsPresContext::CSSPixelsToAppUnits(mScreenYPos) + margin.top;
+    screenViewLocX = presContext->DevPixelsToAppUnits(screenViewLocX) + margin.left;
+    screenViewLocY = presContext->DevPixelsToAppUnits(screenViewLocY) + margin.top;

    // determine the x and y position by subtracting the desired screen
    // position from the screen position of the root frame.
@ -928,7 +948,6 @@ nsMenuPopupFrame::SetPopupPosition(nsIFrame* aAnchorFrame)
  // Compute info about the screen dimensions. Because of multiple monitor systems,
  // the left or top sides of the screen may be in negative space (main monitor is on the
  // right, etc). We need to be sure to do the right thing.
-  nsIDeviceContext* devContext = PresContext()->DeviceContext();
  nsRect rect;
  if ( mMenuCanOverlapOSBar ) {
    devContext->GetRect(rect);
@ -972,7 +991,7 @@ nsMenuPopupFrame::SetPopupPosition(nsIFrame* aAnchorFrame)
    //              | |                        |
    //              | |                        | (screenViewLocX,screenViewLocY)
    //              - |========================|+--------------
-    //                | parentRect           > ||
+    //                | parentSize           > ||
    //                |========================||
    //                |                        || Submenu 
    //                +------------------------+|  ( = mRect )
@ -1152,7 +1171,7 @@ nsMenuPopupFrame::SetPopupPosition(nsIFrame* aAnchorFrame)

  if (sizedToPopup) {
    nsBoxLayoutState state(PresContext());
-    SetBounds(state, nsRect(mRect.x, mRect.y, parentRect.width, mRect.height));
+    SetBounds(state, nsRect(mRect.x, mRect.y, parentSize.width, mRect.height));
  }

  return NS_OK;
--- a/layout/xul/base/src/nsMenuPopupFrame.h
+++ b/layout/xul/base/src/nsMenuPopupFrame.h
@ -280,7 +280,7 @@ protected:
  void InitPositionFromAnchorAlign(const nsAString& aAnchor,
                                   const nsAString& aAlign);

-  void AdjustPositionForAnchorAlign ( PRInt32* ioXPos, PRInt32* ioYPos, const nsRect & inParentRect,
+  void AdjustPositionForAnchorAlign ( PRInt32* ioXPos, PRInt32* ioYPos, const nsSize & inParentRect,
                                      PRBool* outFlushWithTopBottom ) ;

  PRBool IsMoreRoomOnOtherSideOfParent ( PRBool inFlushAboveBelow, PRInt32 inScreenViewLocX, PRInt32 inScreenViewLocY,
--- a/toolkit/content/tests/widgets/Makefile.in
+++ b/toolkit/content/tests/widgets/Makefile.in
@ -87,6 +87,7 @@ _TEST_FILES = 	test_bug360220.xul \
 		test_popup_tree.xul \
 		test_popup_keys.xul \
 		test_popuphidden.xul \
+		test_popup_scaled.xul \
 		test_popupremoving.xul \
 		test_popupremoving_frame.xul \
 		frame_popupremoving_frame.xul \
--- a/toolkit/content/tests/widgets/test_popup_scaled.xul
+++ b/toolkit/content/tests/widgets/test_popup_scaled.xul
@ -0,0 +1,98 @@
+<?xml version="1.0"?>
+<?xml-stylesheet href="chrome://global/skin" type="text/css"?>
+<?xml-stylesheet href="/tests/SimpleTest/test.css" type="text/css"?>
+
+<window title="Popups in Scaled Content"
+        onload="setTimeout(runTests, 0);"
+        xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul">
+
+  <script type="application/javascript" src="/MochiKit/packed.js"></script>
+  <script type="application/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>      
+  <script type="application/javascript" src="/tests/SimpleTest/EventUtils.js"></script>      
+
+<!-- This test checks that the position is correct in two cases:
+      - a popup anchored at an element in a scaled document
+      - a popup opened at a screen coordinate in a scaled window
+  -->
+
+<iframe id="frame" width="60" height="140"
+        src="data:text/html,&lt;html&gt;&lt;body&gt;&lt;input id='one'&gt;&lt;input id='two'&gt;&lt;/body&gt;&lt;/html&gt;"/>
+
+<menupopup id="popup" onpopupshown="shown()" onpopuphidden="nextTest()">
+  <menuitem label="One"/>
+</menupopup>
+
+<script class="testbody" type="application/javascript">
+<![CDATA[
+
+var screenTest = false;
+var screenx = -1, screeny = -1;
+
+SimpleTest.waitForExplicitFinish();
+
+function runTests()
+{
+  netscape.security.PrivilegeManager.enablePrivilege("UniversalXPConnect");
+  setScale($("frame").contentWindow, 2);
+
+  var anchor = $("frame").contentDocument.getElementById("two");
+  $("popup").openPopup(anchor, "after_start");
+}
+
+function setScale(win, scale)
+{
+  netscape.security.PrivilegeManager.enablePrivilege("UniversalXPConnect");
+  var wn = win.QueryInterface(Components.interfaces.nsIInterfaceRequestor)
+              .getInterface(Components.interfaces.nsIWebNavigation);
+  var shell = wn.QueryInterface(Components.interfaces.nsIDocShell);
+  var docViewer = shell.contentViewer.QueryInterface(Components.interfaces.nsIMarkupDocumentViewer);
+  docViewer.fullZoom = scale;
+}
+
+function shown()
+{
+  if (screenTest) {
+    is($("popup").boxObject.screenX, screenx, "screen left position");
+    is($("popup").boxObject.screenY, screeny, "screen top position");
+  }
+  else {
+    netscape.security.PrivilegeManager.enablePrivilege("UniversalXPConnect");
+
+    var anchor = $("frame").contentDocument.getElementById("two");
+
+    is(Math.round(anchor.getBoundingClientRect().left * 2),
+       Math.round($("popup").getBoundingClientRect().left), "anchored left position");
+    is(Math.round(anchor.getBoundingClientRect().bottom * 2),
+       Math.round($("popup").getBoundingClientRect().top), "anchored top position");
+  }
+
+  $("popup").hidePopup();
+}
+
+function nextTest()
+{
+  if (screenTest) {
+    SimpleTest.finish();
+  }
+  else {
+    screenTest = true;
+    screenx = document.documentElement.boxObject.screenX + 120;
+    screeny = document.documentElement.boxObject.screenY + 50;
+    setScale(window, 2);
+    $("popup").openPopupAtScreen(screenx, screeny);
+  }
+}
+
+]]>
+</script>
+
+<body xmlns="http://www.w3.org/1999/xhtml">
+<p id="display">
+</p>
+<div id="content" style="display: none">
+</div>
+<pre id="test">
+</pre>
+</body>
+
+</window>