Bug 524545, popups with large margins appearing offscreen, r=roc

layout/xul/base/src/nsMenuPopupFrame.cpp

@@ -1066,6 +1066,7 @@ nsMenuPopupFrame::FlipOrResize(nscoord& aScreenPoint, nscoord aSize,
           aScreenPoint = aScreenEnd - aSize;
         }
         else {
+          aScreenPoint = endpos + aMarginBegin;
           popupSize = aScreenEnd - aScreenPoint;
         }
       }
@@ -1090,7 +1091,22 @@ nsMenuPopupFrame::FlipOrResize(nscoord& aScreenPoint, nscoord aSize,
     }
   }
 
-  return popupSize;
+  // Make sure that the point is within the screen boundaries and that the
+  // size isn't off the edge of the screen. This can happen when a large
+  // positive or negative margin is used.
+  if (aScreenPoint < aScreenBegin) {
+    aScreenPoint = aScreenBegin;
+  }
+  if (aScreenPoint > aScreenEnd) {
+    aScreenPoint = aScreenEnd - aSize;
+  }
+
+  // If popupSize ended up being negative, or the original size was actually
+  // smaller than the calculated popup size, just use the original size instead.
+  if (popupSize <= 0 || aSize < popupSize) {
+    popupSize = aSize;
+  }
+  return NS_MIN(popupSize, aScreenEnd - aScreenPoint);
 }
 
 nsresult

toolkit/content/tests/chrome/popup_trigger.js

@@ -64,6 +64,11 @@ var popupTests = [
     if (window.opener)
       is(window.opener.document.popupNode, null, testname + " opener.document.popupNode");
 
+    // this will be used in some tests to ensure the size doesn't change
+    var popuprect = gMenuPopup.getBoundingClientRect();
+    gPopupWidth = Math.round(popuprect.width);
+    gPopupHeight = Math.round(popuprect.height);
+
     checkActive(gMenuPopup, "", testname);
     checkOpen("trigger", testname);
     // if a menu, the popup should be opened underneath the menu in the
@@ -245,6 +250,67 @@ var popupTests = [
     compareEdge(gTrigger, gMenuPopup, step, rightmod ? 8 : -8, bottommod ? 8 : -8, testname);
     gMenuPopup.removeAttribute("style");
   }
+},
+ {
+  testname: "open popup with large positive margin",
+  events: [ "popupshowing thepopup", "popupshown thepopup" ],
+  autohide: "thepopup",
+  steps: ["before_start", "before_end", "after_start", "after_end",
+          "start_before", "start_after", "end_before", "end_after", "after_pointer", "overlap"],
+  test: function(testname, step) {
+    gMenuPopup.setAttribute("style", "margin: 1000px;");
+    gMenuPopup.openPopup(gTrigger, step, 0, 0, false, false);
+  },
+  result: function(testname, step) {
+    var popuprect = gMenuPopup.getBoundingClientRect();
+    // as there is more room on the 'end' or 'after' side, popups will always
+    // appear on the right or bottom corners, depending on which side they are
+    // allowed to be flipped by.
+    var expectedleft = step == "before_end" || step == "after_end" ?
+                       0 : Math.round(window.innerWidth - gPopupWidth);
+    var expectedtop = step == "start_after" || step == "end_after" ?
+                      0 : Math.round(window.innerHeight - gPopupHeight);
+    is(Math.round(popuprect.left), expectedleft, testname + " x position " + step);
+    is(Math.round(popuprect.top), expectedtop, testname + " y position " + step);
+    gMenuPopup.removeAttribute("style");
+  }
+},
+{
+  testname: "open popup with large negative margin",
+  events: [ "popupshowing thepopup", "popupshown thepopup" ],
+  autohide: "thepopup",
+  steps: ["before_start", "before_end", "after_start", "after_end",
+          "start_before", "start_after", "end_before", "end_after", "after_pointer", "overlap"],
+  test: function(testname, step) {
+    gMenuPopup.setAttribute("style", "margin: -1000px;");
+    gMenuPopup.openPopup(gTrigger, step, 0, 0, false, false);
+  },
+  result: function(testname, step) {
+    var popuprect = gMenuPopup.getBoundingClientRect();
+    // using negative margins causes the reverse of positive margins, and
+    // popups will appear on the left or top corners.
+    var expectedleft = step == "before_end" || step == "after_end" ?
+                       Math.round(window.innerWidth - gPopupWidth) : 0;
+    var expectedtop = step == "start_after" || step == "end_after" ?
+                      Math.round(window.innerHeight - gPopupHeight) : 0;
+    is(Math.round(popuprect.left), expectedleft, testname + " x position " + step);
+    is(Math.round(popuprect.top), expectedtop, testname + " y position " + step);
+    gMenuPopup.removeAttribute("style");
+  }
+},
+{
+  testname: "popup with unknown step",
+  events: [ "popupshowing thepopup", "popupshown thepopup" ],
+  autohide: "thepopup",
+  test: function() {
+    gMenuPopup.openPopup(gTrigger, "other", 0, 0, false, false);
+  },
+  result: function (testname) {
+    var triggerrect = gMenuPopup.getBoundingClientRect();
+    var popuprect = gMenuPopup.getBoundingClientRect();
+    is(Math.round(popuprect.left), triggerrect.left, testname + " x position ");
+    is(Math.round(popuprect.top), triggerrect.top, testname + " y position ");
+  }
 },
 {
   // these tests check to ensure that the position attribute can be used

toolkit/content/tests/chrome/window_popup_attribute.xul

@@ -13,8 +13,8 @@
 window.opener.SimpleTest.waitForFocus(runTests, window);
 </script>
 
-<hbox style="margin-left: 325px; margin-top: 325px;">
-  <label id="trigger" popup="thepopup" value="Popup"/>
+<hbox style="margin-left: 200px; margin-top: 270px;">
+  <label id="trigger" popup="thepopup" value="Popup" height="60"/>
 </hbox>
 <!-- this frame is used to check that document.popupNode
      is inaccessible from different sources -->

toolkit/content/tests/chrome/window_popup_button.xul

@@ -13,8 +13,8 @@
 window.opener.SimpleTest.waitForFocus(runTests, window);
 </script>
 
-<hbox style="margin-left: 325px; margin-top: 325px;">
-  <button id="trigger" type="menu" label="Popup">
+<hbox style="margin-left: 200px; margin-top: 270px;">
+  <button id="trigger" type="menu" label="Popup" width="100" height="50">
     <menupopup id="thepopup">
       <menuitem id="item1" label="First"/>
       <menuitem id="item2" label="Main Item"/>

toolkit/content/tests/widgets/popup_shared.js

@@ -36,6 +36,7 @@ var gAutoHide = false;
 var gExpectedEventDetails = null;
 var gExpectedTriggerNode = null;
 var gWindowUtils;
+var gPopupWidth = -1, gPopupHeight = -1;
 
 function startPopupTests(tests)
 {
@@ -345,9 +346,15 @@ function compareEdge(anchor, popup, edge, offsetX, offsetY, testname)
   var popuprect = popup.getBoundingClientRect();
   var check1 = false, check2 = false;
 
-  ok((Math.round(popuprect.right) - Math.round(popuprect.left)) &&
-     (Math.round(popuprect.bottom) - Math.round(popuprect.top)),
-     testname + " size");
+  if (gPopupWidth == -1) {
+    ok((Math.round(popuprect.right) - Math.round(popuprect.left)) &&
+       (Math.round(popuprect.bottom) - Math.round(popuprect.top)),
+       testname + " size");
+  }
+  else {
+    is(Math.round(popuprect.width), gPopupWidth, testname + " width");
+    is(Math.round(popuprect.height), gPopupHeight, testname + " height");
+  }
 
   var spaceIdx = edge.indexOf(" ");
   if (spaceIdx > 0) {

toolkit/content/tests/widgets/test_arrowpanel.xul

@@ -10,10 +10,10 @@
   <script type="application/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>      
 
 <stack flex="1">
-  <label id="topleft" value="Top Left" left="0" top="0"/>
-  <label id="topright" value="Top Right" right="0" top="0"/>
-  <label id="bottomleft" value="Bottom Left" left="0" bottom="0"/>
-  <label id="bottomright" value="Bottom Right" right="0" bottom="0"/>
+  <label id="topleft" value="Top Left" left="15" top="15"/>
+  <label id="topright" value="Top Right" right="15" top="15"/>
+  <label id="bottomleft" value="Bottom Left" left="15" bottom="15"/>
+  <label id="bottomright" value="Bottom Right" right="15" bottom="15"/>
   <!-- Our SimpleTest/TestRunner.js runs tests inside an iframe which sizes are W=500 H=300.
        'left' and 'top' values need to be set so that the panel (popup) has enough room to display on its 4 sides. -->
   <label id="middle" value="+/- Centered" left="225" top="135"/>