Commit b1f8ba42 authored by Emilio Cobos Álvarez's avatar Emilio Cobos Álvarez
Browse files

Bug 1768278 - Prevent negative margins from causing us to flip / resize /... 

Bug 1768278 - Prevent negative margins from causing us to flip / resize / slide popups. r=NeilDeakin

Differential Revision: https://phabricator.services.mozilla.com/D145879
parent b0915ba3

browser/base/content/test/forms/browser_selectpopup.js

+5 −3
Original line number Diff line number Diff line
@@ -814,12 +814,14 @@ async function performLargePopupTests(win) { 
    await openSelectPopup(selectPopup, "key", "select", win);



    let rect = selectPopup.getBoundingClientRect();

    let marginBottom = parseFloat(getComputedStyle(selectPopup).marginBottom);

    let marginTop = parseFloat(getComputedStyle(selectPopup).marginTop);

    ok(

      rect.top >= browserRect.top,

      rect.top - marginTop >= browserRect.top,

      "Popup top position in within browser area"

    );

    ok(

      rect.bottom <= browserRect.bottom,

      rect.bottom + marginBottom <= browserRect.bottom,

      "Popup bottom position in within browser area"

    );
@@ -846,7 +848,7 @@ async function performLargePopupTests(win) { 
      // fuzz-factor of 1.

      SimpleTest.isfuzzy(

        selectPopup.children[selectedOption].getBoundingClientRect().bottom,

        selectPopup.getBoundingClientRect().bottom - bpBottom,

        selectPopup.getBoundingClientRect().bottom - bpBottom + marginBottom,

        1,

        "Popup scroll at correct position " + bpBottom

      );

browser/components/extensions/test/browser/browser_ext_popup_select.js

+4 −2
Original line number Diff line number Diff line
@@ -71,15 +71,17 @@ add_task(async function testPopupSelectPopup() { 
    });



    let popupRect = selectPopup.getOuterScreenRect();

    let marginTop = parseFloat(getComputedStyle(selectPopup).marginTop);

    let marginLeft = parseFloat(getComputedStyle(selectPopup).marginLeft);



    is(

      Math.floor(browser.screenX + elemRect.left),

      Math.floor(browser.screenX + elemRect.left + marginLeft),

      popupRect.left,

      "Select popup has the correct x origin"

    );



    is(

      Math.floor(browser.screenY + elemRect.bottom),

      Math.floor(browser.screenY + elemRect.bottom + marginTop),

      popupRect.top,

      "Select popup has the correct y origin"

    );

layout/xul/nsMenuPopupFrame.cpp

+44 −31
Original line number Diff line number Diff line
@@ -1495,7 +1495,7 @@ nsresult nsMenuPopupFrame::SetPopupPosition(nsIFrame* aAnchorFrame, 
  // indicators of whether the popup should be flipped or resized.

  FlipStyle hFlip = FlipStyle_None, vFlip = FlipStyle_None;



  nsMargin margin = GetMargin();

  const nsMargin margin = GetMargin();



  // the screen rectangle of the root frame, in dev pixels.

  nsRect rootScreenRect = rootFrame->GetScreenRectInAppUnits();
@@ -1594,23 +1594,38 @@ nsresult nsMenuPopupFrame::SetPopupPosition(nsIFrame* aAnchorFrame, 
      (mInContentShell ||

       (mFlip != FlipType_None &&

        (!aIsMove || mIsOffset || mPopupType != ePopupTypePanel)))) {

    const nsRect screenRect = [&] {

      int32_t appPerDev = presContext->AppUnitsPerDevPixel();

    LayoutDeviceIntRect anchorRectDevPix =

      auto anchorRectDevPix =

          LayoutDeviceIntRect::FromAppUnitsToNearest(anchorRect, appPerDev);

    LayoutDeviceIntRect rootScreenRectDevPix =

      auto rootScreenRectDevPix =

          LayoutDeviceIntRect::FromAppUnitsToNearest(rootScreenRect, appPerDev);

    LayoutDeviceIntRect screenRectDevPix =

      auto screenRectDevPix =

          GetConstraintRect(anchorRectDevPix, rootScreenRectDevPix, popupLevel);

    nsRect screenRect =

        LayoutDeviceIntRect::ToAppUnits(screenRectDevPix, appPerDev);

      nsRect sr = LayoutDeviceIntRect::ToAppUnits(screenRectDevPix, appPerDev);



      // Expand the allowable screen rect by the negative margins. Note that we

      // intentionally don't include the context menu offset in the margin here.

      nsMargin rawMargin;

      if (StyleMargin()->GetMargin(rawMargin)) {

        rawMargin.EnsureAtMost(nsMargin());

        sr.Deflate(rawMargin);

      }

      return sr;

    }();



    // Ensure that anchorRect is on screen.

    anchorRect = anchorRect.Intersect(screenRect);



    // shrink the the popup down if it is larger than the screen size

    if (mRect.width > screenRect.width) mRect.width = screenRect.width;

    if (mRect.height > screenRect.height) mRect.height = screenRect.height;

    // Shrink the the popup down if it is larger than the screen size

    if (mRect.width > screenRect.width) {

      mRect.width = screenRect.width;

    }

    if (mRect.height > screenRect.height) {

      mRect.height = screenRect.height;

    }



    // at this point the anchor (anchorRect) is within the available screen

    // At this point the anchor (anchorRect) is within the available screen

    // area (screenRect) and the popup is known to be no larger than the

    // screen.
@@ -1629,42 +1644,40 @@ nsresult nsMenuPopupFrame::SetPopupPosition(nsIFrame* aAnchorFrame, 
    // Next, check if there is enough space to show the popup at full size

    // when positioned at screenPoint. If not, flip the popups to the opposite

    // side of their anchor point, or resize them as necessary.

    const nsPoint preOffsetScreenPoint = screenPoint;

    if (slideHorizontal) {

      mRect.width = SlideOrResize(screenPoint.x, mRect.width, screenRect.x,

                                  screenRect.XMost(), &mAlignmentOffset);

    } else {

      bool endAligned = IsDirectionRTL()

                            ? mPopupAlignment == POPUPALIGNMENT_TOPLEFT ||

                                  mPopupAlignment == POPUPALIGNMENT_BOTTOMLEFT

                            : mPopupAlignment == POPUPALIGNMENT_TOPRIGHT ||

                                  mPopupAlignment == POPUPALIGNMENT_BOTTOMRIGHT;

    nscoord preOffsetScreenPoint = screenPoint.x;

    if (slideHorizontal) {

      mRect.width = SlideOrResize(screenPoint.x, mRect.width, screenRect.x,

                                  screenRect.XMost(), &mAlignmentOffset);

    } else {

      mRect.width =

          FlipOrResize(screenPoint.x, mRect.width, screenRect.x,

                       screenRect.XMost(), anchorRect.x, anchorRect.XMost(),

                       margin.left, margin.right, hFlip, endAligned, &mHFlip);

    }

    mIsOffset = preOffsetScreenPoint != screenPoint.x;



    endAligned = mPopupAlignment == POPUPALIGNMENT_BOTTOMLEFT ||

                 mPopupAlignment == POPUPALIGNMENT_BOTTOMRIGHT;

    preOffsetScreenPoint = screenPoint.y;

    if (slideVertical) {

      mRect.height = SlideOrResize(screenPoint.y, mRect.height, screenRect.y,

                                   screenRect.YMost(), &mAlignmentOffset);

    } else {

      bool endAligned = mPopupAlignment == POPUPALIGNMENT_BOTTOMLEFT ||

                        mPopupAlignment == POPUPALIGNMENT_BOTTOMRIGHT;

      mRect.height =

          FlipOrResize(screenPoint.y, mRect.height, screenRect.y,

                       screenRect.YMost(), anchorRect.y, anchorRect.YMost(),

                       margin.top, margin.bottom, vFlip, endAligned, &mVFlip);

    }

    mIsOffset = mIsOffset || (preOffsetScreenPoint != screenPoint.y);

    mIsOffset = preOffsetScreenPoint != screenPoint;



    NS_ASSERTION(screenPoint.x >= screenRect.x &&

                     screenPoint.y >= screenRect.y &&

                     screenPoint.x + mRect.width <= screenRect.XMost() &&

                     screenPoint.y + mRect.height <= screenRect.YMost(),

                 "Popup is offscreen");

    NS_ASSERTION(screenPoint.x >= screenRect.x, "Popup is offscreen (x start)");

    NS_ASSERTION(screenPoint.y >= screenRect.y, "Popup is offscreen (y start)");

    NS_ASSERTION(screenPoint.x + mRect.width <= screenRect.XMost(),

                 "Popup is offscreen (x end)");

    NS_ASSERTION(screenPoint.y + mRect.height <= screenRect.YMost(),

                 "Popup is offscreen (y end)");

  }



  // snap the popup's position in screen coordinates to device pixels,

toolkit/content/tests/chrome/popup_trigger.js

+22 −20
Original line number Diff line number Diff line
@@ -453,26 +453,28 @@ var popupTests = [ 
      gMenuPopup.openPopup(gTrigger, step, 0, 0, false, false);

    },

    result(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

      var rightmod =

        step == "before_end" ||

        step == "after_end" ||

        step == "start_before" ||

        step == "start_after" ||

        step.match(/topright$/) ||

        step.match(/bottomright$/);

      var bottommod =

        step == "before_start" ||

        step == "before_end" ||

        step == "start_after" ||

        step == "end_after" ||

        step.match(/bottomleft$/) ||

        step.match(/bottomright$/);

      // Negative margins expand available screen position.

      compareEdge(

        gTrigger,

        gMenuPopup,

        step,

        rightmod ? 1000 : -1000,

        bottommod ? 1000 : -1000,

        testname

      );

      gMenuPopup.removeAttribute("style");

    },

toolkit/content/tests/chrome/test_arrowpanel.xhtml

+44 −41
Original line number Diff line number Diff line
@@ -45,6 +45,11 @@ 
     * using em unit padding, which is the default as of bug 1701920.

     */

    --arrowpanel-padding: 16px;

    /**

     * Linux and windows have some negative margin-inline that can change the

     * overflow calculations

     */

    margin-inline: 0 !important;

  }

</html:style>
@@ -53,8 +58,6 @@ 


SimpleTest.waitForExplicitFinish();



const isOSXYosemite = navigator.userAgent.includes("Mac OS X 10.10");



var expectedAnchor = null;

var expectedSide = "", expectedAnchorEdge = "";

var zoomFactor = 1;
@@ -72,7 +75,7 @@ function* nextTest() 
{

  var panel = $("panel");



  function openPopup(position, anchor, expected, anchorEdge, alignment)

  function openPopup(position, anchor, expected, anchorEdge)

  {

    expectedAnchor = anchor instanceof Node ? anchor : $(anchor);

    expectedSide = expected;
@@ -85,9 +88,9 @@ function* nextTest() 
  for (var iter = 0; iter < 2; iter++) {

    openPopup("after_start", "topleft", "top", "left");

    yield;

    openPopup("after_start", "bottomleft", "bottom", "left", "before_start");

    openPopup("after_start", "bottomleft", "bottom", "left");

    yield;

    openPopup("before_start", "topleft", "top", "left", "after_start");

    openPopup("before_start", "topleft", "top", "left");

    yield;

    openPopup("before_start", "bottomleft", "bottom", "left");

    yield;
@@ -96,13 +99,13 @@ function* nextTest() 
    openPopup("before_start", "middle", "bottom", "left");

    yield;



    openPopup("after_start", "topright", "top", "right", "after_end");

    openPopup("after_start", "topright", "top", "right");

    yield;

    openPopup("after_start", "bottomright", "bottom", "right", "before_end");

    openPopup("after_start", "bottomright", "bottom", "right");

    yield;

    openPopup("before_start", "topright", "top", "right", "after_end");

    openPopup("before_start", "topright", "top", "right");

    yield;

    openPopup("before_start", "bottomright", "bottom", "right", "before_end");

    openPopup("before_start", "bottomright", "bottom", "right");

    yield;



    openPopup("after_end", "middle", "top", "right");
@@ -110,26 +113,26 @@ function* nextTest() 
    openPopup("before_end", "middle", "bottom", "right");

    yield;



    openPopup("start_before", "topleft", "left", "top", "end_before");

    openPopup("start_before", "topleft", "left", "top");

    yield;

    openPopup("start_before", "topright", "right", "top");

    yield;

    openPopup("end_before", "topleft", "left", "top");

    yield;

    openPopup("end_before", "topright", "right", "top", "start_before");

    openPopup("end_before", "topright", "right", "top");

    yield;

    openPopup("start_before", "middle", "right", "top");

    yield;

    openPopup("end_before", "middle", "left", "top");

    yield;



    openPopup("start_before", "bottomleft", "left", "bottom", "end_after");

    openPopup("start_before", "bottomleft", "left", "bottom");

    yield;

    openPopup("start_before", "bottomright", "right", "bottom", "start_after");

    openPopup("start_before", "bottomright", "right", "bottom");

    yield;

    openPopup("end_before", "bottomleft", "left", "bottom", "end_after");

    openPopup("end_before", "bottomleft", "left", "bottom");

    yield;

    openPopup("end_before", "bottomright", "right", "bottom", "start_after");

    openPopup("end_before", "bottomright", "right", "bottom");

    yield;



    openPopup("start_after", "middle", "right", "bottom");
@@ -137,22 +140,22 @@ function* nextTest() 
    openPopup("end_after", "middle", "left", "bottom");

    yield;



    openPopup("topcenter bottomleft", "bottomleft", "bottom", "center left", "before_start");

    openPopup("topcenter bottomleft", "bottomleft", "bottom", "center left");

    yield;

    openPopup("bottomcenter topleft", "topleft", "top", "center left", "after_start");

    openPopup("bottomcenter topleft", "topleft", "top", "center left");

    yield;

    openPopup("topcenter bottomright", "bottomright", "bottom", "center right", "before_end");

    openPopup("topcenter bottomright", "bottomright", "bottom", "center right");

    yield;

    openPopup("bottomcenter topright", "topright", "top", "center right", "after_end");

    openPopup("bottomcenter topright", "topright", "top", "center right");

    yield;

    openPopup("topcenter bottomleft", "middle", "bottom", "center left", "before_start");

    openPopup("topcenter bottomleft", "middle", "bottom", "center left");

    yield;

    openPopup("bottomcenter topleft", "middle", "top", "center left", "after_start");

    openPopup("bottomcenter topleft", "middle", "top", "center left");

    yield;



    openPopup("leftcenter topright", "middle", "right", "center top", "start_before");

    openPopup("leftcenter topright", "middle", "right", "center top");

    yield;

    openPopup("rightcenter bottomleft", "middle", "left", "center bottom", "end_after");

    openPopup("rightcenter bottomleft", "middle", "left", "center bottom");

    yield;



/*
@@ -190,10 +193,10 @@ function* nextTest() 
    $("bottomright").style.marginLeft = "15px";

    $("bottomright").style.justifySelf = "left";



    $("topleft").style.removeProperty("marginLeft");

    $("topright").style.removeProperty("marginRight");

    $("bottomleft").style.removeProperty("marginLeft");

    $("bottomright").style.removeProperty("marginRight");

    $("topleft").style.removeProperty("margin-left");

    $("topright").style.removeProperty("margin-right");

    $("bottomleft").style.removeProperty("margin-left");

    $("bottomright").style.removeProperty("margin-right");

  }



  // Test that a transition occurs when opening or closing the popup.
@@ -267,6 +270,7 @@ function checkPanelPosition(panel) 
      break;

  }



  let desc = panel.id + ": anchored on " + expectedAnchorEdge + " to " + anchor.id + " | " + (isRTL ? "rtl" : "ltr") + " | " + anchor.getAttribute("style");

  let iscentered = false;

  if (expectedAnchorEdge.indexOf("center ") == 0) {

    expectedAnchorEdge = expectedAnchorEdge.substring(7);
@@ -275,29 +279,28 @@ function checkPanelPosition(panel) 


  switch (expectedAnchorEdge) {

    case "top":

      adj = vwinpos + parseInt(getComputedStyle(panel, "").marginTop);

      adj = vwinpos + parseInt(getComputedStyle(panel).marginTop);

      if (iscentered)

        adj += Math.round(anchorRect.height) / 2;

      isWithinHalfPixel(panelRect.top, anchorRect.top * zoomFactor + adj, "anchored on top");

        adj += anchorRect.height / 2;

      isWithinHalfPixel(panelRect.top, anchorRect.top * zoomFactor + adj, desc);

      break;

    case "bottom":

      adj = vwinpos + parseInt(getComputedStyle(panel, "").marginBottom);

      adj = vwinpos + parseInt(getComputedStyle(panel).marginBottom);

      if (iscentered)

        adj += Math.round(anchorRect.height) / 2;

      isWithinHalfPixel(panelRect.bottom, anchorRect.bottom * zoomFactor - adj, "anchored on bottom");

        adj += anchorRect.height / 2;

      isWithinHalfPixel(panelRect.bottom, anchorRect.bottom * zoomFactor - adj, desc);

      break;

    case "left":

      adj = hwinpos + parseInt(getComputedStyle(panel, "").marginLeft);

      adj = hwinpos + parseInt(getComputedStyle(panel).marginLeft);

      if (iscentered)

        adj += Math.round(anchorRect.width) / 2;

      isWithinHalfPixel(panelRect.left, anchorRect.left * zoomFactor + adj, "anchored on left ");

        adj += anchorRect.width / 2;

      isWithinHalfPixel(panelRect.left, anchorRect.left * zoomFactor + adj, desc);

      break;

    case "right":

      adj = hwinpos + parseInt(getComputedStyle(panel, "").marginRight);

      adj = hwinpos + parseInt(getComputedStyle(panel).marginRight);

      if (iscentered)

        adj += Math.round(anchorRect.width) / 2;

      if (!isOSXYosemite)

        isWithinHalfPixel(panelRect.right, anchorRect.right * zoomFactor - adj, "anchored on right");

        adj += anchorRect.width / 2;

      isWithinHalfPixel(panelRect.right, anchorRect.right * zoomFactor - adj, desc);

      break;

  }
@@ -310,7 +313,7 @@ function checkPanelPosition(panel) 


function isWithinHalfPixel(a, b, desc)

{

  ok(Math.abs(a - b) <= 0.5, desc);

  ok(Math.abs(a - b) <= 0.5, `${desc}: ${a} vs. ${b}`);

}



function checkBigPanel(panel)