From 4db5a0f814ae60735182143e13cb873071e88af7 Mon Sep 17 00:00:00 2001 From: "roc+%cs.cmu.edu" Date: Wed, 30 Apr 2008 01:41:56 +0000 Subject: [PATCH] Bug 409427. Don't slide popups up if they're offscreen, flip them up above the anchor point instead. r=enndeakin,sr=neil,a=damon --- layout/xul/base/src/nsMenuPopupFrame.cpp | 37 ++++++++----------- .../tests/widgets/test_popupincontent.xul | 15 +++++++- 2 files changed, 29 insertions(+), 23 deletions(-) diff --git a/layout/xul/base/src/nsMenuPopupFrame.cpp b/layout/xul/base/src/nsMenuPopupFrame.cpp index a99cb77dfb0..ca53238c275 100644 --- a/layout/xul/base/src/nsMenuPopupFrame.cpp +++ b/layout/xul/base/src/nsMenuPopupFrame.cpp @@ -1194,7 +1194,8 @@ nsMenuPopupFrame::SetPopupPosition(nsIFrame* aAnchorFrame) xpos -= (screenViewLocX + mRect.width) - screenRightTwips; // Now the Y position. If the popup is up too high, slide it down so it's - // on screen. + // on screen. This can't make the popup overlap screenViewLocX/Y since + // we're moving it down away from screenViewLocY. if ( screenViewLocY < screenTopTwips ) { PRInt32 moveDistY = screenTopTwips - screenViewLocY; ypos += moveDistY; @@ -1203,24 +1204,12 @@ nsMenuPopupFrame::SetPopupPosition(nsIFrame* aAnchorFrame) // Now if the popup extends down too far, either resize it or flip it to be // above the anchor point and resize it to fit above, depending on where we - // have more room. + // have more room. But ensure it doesn't overlap screenViewLocX/Y. if ( (screenViewLocY + mRect.height) > screenBottomTwips ) { // XXXbz it'd be good to make use of IsMoreRoomOnOtherSideOfParent and // such here, but that's really focused on having a nonempty parent // rect... - if (screenViewLocY > screenBottomTwips) { - // if the popup is positioned off the edge, move it up. This is important - // when the popup is constrained to the content area so that the popup - // doesn't extend past the edge. This is a rare situation so include this - // check within the other. - - // we already constrained the height to the screen size above, so this - // calculation should always result in a y position below the top. - NS_ASSERTION(mRect.height <= screenBottomTwips - screenTopTwips, "height too large"); - ypos += screenBottomTwips - screenViewLocY - mRect.height; - } - else if (screenBottomTwips - screenViewLocY > - screenViewLocY - screenTopTwips) { + if (screenBottomTwips - screenViewLocY > screenViewLocY - screenTopTwips) { // More space below our desired point. Resize to fit in this space. // Note that this is making mRect smaller; othewise we would not have // reached this code. @@ -1228,17 +1217,21 @@ nsMenuPopupFrame::SetPopupPosition(nsIFrame* aAnchorFrame) } else { // More space above our desired point. Flip and resize to fit in this // space. - // First move screenViewLocY and ypos up because the offset needs to flip - screenViewLocY -= 2*offsetForContextMenu; - ypos -= 2*offsetForContextMenu; - if (mRect.height > screenViewLocY - screenTopTwips) { + // First figure out where the bottom of the popup is going to be. + nscoord newBottomY = + screenViewLocY - 2*offsetForContextMenu - margin.TopBottom(); + // Make sure the bottom is on the screen + newBottomY = PR_MIN(newBottomY, screenBottomTwips); + newBottomY = PR_MAX(newBottomY, screenTopTwips); + if (mRect.height > newBottomY - screenTopTwips) { // We wouldn't fit. Shorten before flipping. - mRect.height = screenViewLocY - screenTopTwips; + mRect.height = newBottomY - screenTopTwips; } - ypos -= (mRect.height + margin.top + margin.bottom); + // Adjust ypos to match + ypos += newBottomY - screenViewLocY - mRect.height; } } - } + } presContext->GetViewManager()->MoveViewTo(GetView(), xpos, ypos); diff --git a/toolkit/content/tests/widgets/test_popupincontent.xul b/toolkit/content/tests/widgets/test_popupincontent.xul index 00a054c4259..2379aeb8f06 100644 --- a/toolkit/content/tests/widgets/test_popupincontent.xul +++ b/toolkit/content/tests/widgets/test_popupincontent.xul @@ -17,7 +17,7 @@ - + @@ -59,6 +59,14 @@ function nextTest() synthesizeMouse($("menu"), 2, 2, { }); break; case "left and top": + step = "open near bottom"; + // request that the menu be opened with a target point near the bottom of the window, + // so that the menu's top margin will push it completely outside the window. + var bo = document.documentElement.boxObject; + popup.setAttribute("top", bo.screenY + window.innerHeight - 5); + synthesizeMouse($("menu"), 2, 2, { }); + break; + case "open near bottom": step = "large menu"; popup.removeAttribute("left"); popup.removeAttribute("top"); @@ -90,6 +98,11 @@ function popupShown() if (step == "left and top") originalHeight = popuprect.bottom - popuprect.top; + if (step == "open near bottom") { + // check that the menu flipped up so it's above our requested point + ok(popuprect.bottom - 1 <= windowrect.bottom - 5, step + " bottom"); + } + if (step == "largemenu") { ok(popuprect.top == windowrect.top, step + " top"); ok(popuprect.bottom - 1 == windowrect.bottom, step + " bottom");