Bug 1075089 - Move popup menu frame offset to LookAndFeel and fix default offset for OS X. r=Enn

This commit is contained in:
Nick Robson 2015-08-04 16:41:00 -04:00
parent e8a2dde4aa
commit 97eeb3e821
16 changed files with 144 additions and 53 deletions

View File

@ -60,6 +60,11 @@ using namespace mozilla;
using mozilla::dom::PopupBoxObject;
int8_t nsMenuPopupFrame::sDefaultLevelIsTop = -1;
// XXX, kyle.yuan@sun.com, there are 4 definitions for the same purpose:
// nsMenuPopupFrame.h, nsListControlFrame.cpp, listbox.xml, tree.xml
// need to find a good place to put them together.
// if someone changes one, please also change the other.
uint32_t nsMenuPopupFrame::sTimeoutOfIncrementalSearch = 1000;
const char* kPrefIncrementalSearchTimeout =
@ -1170,7 +1175,7 @@ nsMenuPopupFrame::FlipOrResize(nscoord& aScreenPoint, nscoord aSize,
// if the newly calculated position is different than the existing
// position, we flip such that the popup is to the left or top of the
// anchor point instead.
nscoord newScreenPoint = startpos - aSize - aMarginBegin - aOffsetForContextMenu;
nscoord newScreenPoint = startpos - aSize - aMarginBegin - std::max(aOffsetForContextMenu, 0);
if (newScreenPoint != aScreenPoint) {
*aFlipSide = true;
aScreenPoint = newScreenPoint;
@ -1316,7 +1321,7 @@ nsMenuPopupFrame::SetPopupPosition(nsIFrame* aAnchorFrame, bool aIsMove, bool aS
nsRect rootScreenRect = rootFrame->GetScreenRectInAppUnits();
nsDeviceContext* devContext = presContext->DeviceContext();
nscoord offsetForContextMenu = 0;
nsPoint offsetForContextMenu;
bool isNoAutoHide = IsNoAutoHide();
nsPopupLevel popupLevel = PopupLevel(isNoAutoHide);
@ -1377,13 +1382,17 @@ nsMenuPopupFrame::SetPopupPosition(nsIFrame* aAnchorFrame, bool aIsMove, bool aS
// coordinates.
int32_t factor = devContext->AppUnitsPerDevPixelAtUnitFullZoom();
// context menus should be offset by two pixels so that they don't appear
// directly where the cursor is. Otherwise, it is too easy to have the
// context menu close up again.
// Depending on the platform, context menus should be offset by varying amounts
// to ensure that they don't appear directly where the cursor is. Otherwise,
// it is too easy to have the context menu close up again.
if (mAdjustOffsetForContextMenu) {
int32_t offsetForContextMenuDev =
nsPresContext::CSSPixelsToAppUnits(CONTEXT_MENU_OFFSET_PIXELS) / factor;
offsetForContextMenu = presContext->DevPixelsToAppUnits(offsetForContextMenuDev);
nsPoint offsetForContextMenuDev;
offsetForContextMenuDev.x = nsPresContext::CSSPixelsToAppUnits(LookAndFeel::GetInt(
LookAndFeel::eIntID_ContextMenuOffsetHorizontal)) / factor;
offsetForContextMenuDev.y = nsPresContext::CSSPixelsToAppUnits(LookAndFeel::GetInt(
LookAndFeel::eIntID_ContextMenuOffsetVertical)) / factor;
offsetForContextMenu.x = presContext->DevPixelsToAppUnits(offsetForContextMenuDev.x);
offsetForContextMenu.y = presContext->DevPixelsToAppUnits(offsetForContextMenuDev.y);
}
// next, convert into app units accounting for the zoom
@ -1394,8 +1403,8 @@ nsMenuPopupFrame::SetPopupPosition(nsIFrame* aAnchorFrame, bool aIsMove, bool aS
anchorRect = nsRect(screenPoint, nsSize(0, 0));
// add the margins on the popup
screenPoint.MoveBy(margin.left + offsetForContextMenu,
margin.top + offsetForContextMenu);
screenPoint.MoveBy(margin.left + offsetForContextMenu.x,
margin.top + offsetForContextMenu.y);
// screen positioned popups can be flipped vertically but never horizontally
vFlip = FlipStyle_Outside;
@ -1439,7 +1448,7 @@ nsMenuPopupFrame::SetPopupPosition(nsIFrame* aAnchorFrame, bool aIsMove, bool aS
} else {
mRect.width = FlipOrResize(screenPoint.x, mRect.width, screenRect.x,
screenRect.XMost(), anchorRect.x, anchorRect.XMost(),
margin.left, margin.right, offsetForContextMenu, hFlip,
margin.left, margin.right, offsetForContextMenu.x, hFlip,
&mHFlip);
}
if (slideVertical) {
@ -1448,7 +1457,7 @@ nsMenuPopupFrame::SetPopupPosition(nsIFrame* aAnchorFrame, bool aIsMove, bool aS
} else {
mRect.height = FlipOrResize(screenPoint.y, mRect.height, screenRect.y,
screenRect.YMost(), anchorRect.y, anchorRect.YMost(),
margin.top, margin.bottom, offsetForContextMenu, vFlip,
margin.top, margin.bottom, offsetForContextMenu.y, vFlip,
&mVFlip);
}
@ -2158,10 +2167,10 @@ nsMenuPopupFrame::MoveTo(int32_t aLeft, int32_t aTop, bool aUpdateAttrs)
// Workaround for bug 788189. See also bug 708278 comment #25 and following.
if (mAdjustOffsetForContextMenu) {
nscoord offsetForContextMenu =
nsPresContext::CSSPixelsToAppUnits(CONTEXT_MENU_OFFSET_PIXELS);
margin.left += offsetForContextMenu;
margin.top += offsetForContextMenu;
margin.left += nsPresContext::CSSPixelsToAppUnits(LookAndFeel::GetInt(
LookAndFeel::eIntID_ContextMenuOffsetHorizontal));
margin.top += nsPresContext::CSSPixelsToAppUnits(LookAndFeel::GetInt(
LookAndFeel::eIntID_ContextMenuOffsetVertical));
}
nsPresContext* presContext = PresContext();

View File

@ -122,13 +122,6 @@ enum MenuPopupAnchorType {
#define POPUPPOSITION_HFLIP(v) (v ^ 1)
#define POPUPPOSITION_VFLIP(v) (v ^ 2)
// XXX, kyle.yuan@sun.com, there are 4 definitions for the same purpose:
// nsMenuPopupFrame.h, nsListControlFrame.cpp, listbox.xml, tree.xml
// need to find a good place to put them together.
// if someone changes one, please also change the other.
#define CONTEXT_MENU_OFFSET_PIXELS 2
nsIFrame* NS_NewMenuPopupFrame(nsIPresShell* aPresShell, nsStyleContext* aContext);
class nsView;

View File

@ -75,6 +75,8 @@ prefs.setIntPref("ui.click_hold_context_menus.delay", arguments[0]);
def test_wait_with_value(self):
wait_with_value(self.marionette, self.wait_for_condition, "button1-mousemove-mousedown-mouseup-click")
"""
// Skipping due to Bug 1191066
def test_context_menu(self):
context_menu(self.marionette, self.wait_for_condition, "button1-mousemove-mousedown-contextmenu", "button1-mousemove-mousedown-contextmenu-mouseup-click")
@ -83,7 +85,8 @@ prefs.setIntPref("ui.click_hold_context_menus.delay", arguments[0]);
def test_long_press_on_xy_action(self):
long_press_on_xy_action(self.marionette, self.wait_for_condition, "button1-mousemove-mousedown-contextmenu-mouseup-click")
"""
"""
//Skipping due to Bug 865334
def test_long_press_fail(self):

View File

@ -507,9 +507,11 @@ var popupTests = [
}
}
var openX = 8;
var openY = 16;
var rect = gMenuPopup.getBoundingClientRect();
is(rect.left, 10, testname + " left");
is(rect.top, 18, testname + " top");
is(rect.left, openX + (platformIsMac() ? 1 : 2), testname + " left");
is(rect.top, openY + (platformIsMac() ? -6 : 2), testname + " top");
ok(rect.right, testname + " right is " + rect.right);
ok(rect.bottom, testname + " bottom is " + rect.bottom);
}
@ -749,7 +751,7 @@ var popupTests = [
autohide: "thepopup",
test: function(testname, step) {
gTrigger.focus();
synthesizeKey("VK_DOWN", { altKey: (navigator.platform.indexOf("Mac") == -1) });
synthesizeKey("VK_DOWN", { altKey: !platformIsMac() });
},
result: function(testname, step) {
checkOpen("trigger", testname);
@ -762,7 +764,7 @@ var popupTests = [
events: [ "popupshowing thepopup", "popupshown thepopup" ],
test: function(testname, step) {
gTrigger.focus();
synthesizeKey("VK_UP", { altKey: (navigator.platform.indexOf("Mac") == -1) });
synthesizeKey("VK_UP", { altKey: !platformIsMac() });
},
result: function(testname, step) {
checkOpen("trigger", testname);
@ -789,7 +791,7 @@ var popupTests = [
autohide: "thepopup",
test: function(testname, step) {
gTrigger.focus();
synthesizeKey((navigator.platform.indexOf("Mac") == -1) ? "VK_F4" : " ", { });
synthesizeKey(platformIsMac() ? " " : "VK_F4", { });
},
result: function(testname, step) {
checkOpen("trigger", testname);
@ -802,10 +804,10 @@ var popupTests = [
condition: function() { return gIsMenu; },
test: function(testname, step) {
gTrigger.focus();
if (navigator.platform.indexOf("Mac") == -1)
synthesizeKey("", { metaKey: true });
else
if (platformIsMac())
synthesizeKey("VK_F4", { altKey: true });
else
synthesizeKey("", { metaKey: true });
},
result: function(testname, step) {
checkClosed("trigger", testname);
@ -850,3 +852,8 @@ var popupTests = [
}
];
function platformIsMac()
{
return navigator.platform.indexOf("Mac") > -1;
}

View File

@ -103,8 +103,23 @@ function openContextMenu() {
var x = menubox.screenX - winbox.screenX;
var y = menubox.screenY - winbox.screenY;
ok(y >= mouseY,
"menu top " + y + " should be below click point " + mouseY);
if (navigator.userAgent.indexOf("Mac") > -1)
{
// This check is alterered slightly for OSX which adds padding to the top
// and bottom of its context menus. The menu position calculation must
// be changed to allow for the pointer to be outside this padding
// when the menu opens.
// (Bug 1075089)
ok(y + 6 >= mouseY,
"menu top " + (y + 6) + " should be below click point " + mouseY);
}
else
{
ok(y >= mouseY,
"menu top " + y + " should be below click point " + mouseY);
}
ok(y <= mouseY + 20,
"menu top " + y + " should not be too far below click point " + mouseY);

View File

@ -189,7 +189,7 @@ function checkContextMenu(event)
// context menu from mouse click
switch (gTestId) {
case -1:
var expected = gSelectionStep == 2 ? 1 : (navigator.platform.indexOf("Mac") >= 0 ? 3 : 0);
var expected = gSelectionStep == 2 ? 1 : (platformIsMac() ? 3 : 0);
is($(gTestElement).selectedIndex, expected, "index after click " + gSelectionStep);
break;
case 0:
@ -222,47 +222,57 @@ function checkContextMenuForMenu(event)
function checkPopup()
{
var menurect = $("themenu").getBoundingClientRect();
// Context menus are offset by a number of pixels from the mouse click
// which activates them. This is so that they don't appear exactly
// under the mouse which can cause them to be mistakenly dismissed.
// The number of pixels depends on the platform and is defined in
// each platform's nsLookAndFeel
var contextMenuOffsetX = platformIsMac() ? 1 : 2;
var contextMenuOffsetY = platformIsMac() ? -6 : 2;
if (gTestId == 0) {
if (gTestElement == "list") {
var itemrect = $("item3").getBoundingClientRect();
isRoundedX(menurect.left, itemrect.left + 2,
isRoundedX(menurect.left, itemrect.left + contextMenuOffsetX,
"list selection keyboard left");
isRoundedY(menurect.top, itemrect.bottom + 2,
isRoundedY(menurect.top, itemrect.bottom + contextMenuOffsetY,
"list selection keyboard top");
}
else {
var tree = $("tree");
var bodyrect = $("treechildren").getBoundingClientRect();
isRoundedX(menurect.left, bodyrect.left + 2,
isRoundedX(menurect.left, bodyrect.left + contextMenuOffsetX,
"tree selection keyboard left");
isRoundedY(menurect.top, bodyrect.top +
tree.treeBoxObject.rowHeight * 3 + 2,
tree.treeBoxObject.rowHeight * 3 + contextMenuOffsetY,
"tree selection keyboard top");
}
}
else if (gTestId == 1) {
// activating a context menu with the mouse from position (7, 1).
// Add 2 pixels to these values as context menus are offset by 2 pixels
// so that they don't appear exactly only the menu making them easier to
// dismiss. See nsXULPopupListener.
// activating a context menu with the mouse from position (7, 4).
var elementrect = $(gTestElement).getBoundingClientRect();
isRoundedX(menurect.left, elementrect.left + 9,
isRoundedX(menurect.left, elementrect.left + 7 + contextMenuOffsetX,
gTestElement + " mouse left");
isRoundedY(menurect.top, elementrect.top + 6,
isRoundedY(menurect.top, elementrect.top + 4 + contextMenuOffsetY,
gTestElement + " mouse top");
}
else {
var elementrect = $(gTestElement).getBoundingClientRect();
isRoundedX(menurect.left, elementrect.left + 2,
isRoundedX(menurect.left, elementrect.left + contextMenuOffsetX,
gTestElement + " no selection keyboard left");
isRoundedY(menurect.top, elementrect.bottom + 2,
isRoundedY(menurect.top, elementrect.bottom + contextMenuOffsetY,
gTestElement + " no selection keyboard top");
}
$("themenu").hidePopup();
}
function platformIsMac()
{
return navigator.platform.indexOf("Mac") > -1;
}
]]>
</script>

View File

@ -214,14 +214,17 @@ function contextMenuPopupShown()
var popup = document.getElementById("popup");
var rect = popup.getBoundingClientRect();
var labelrect = document.getElementById("label").getBoundingClientRect();
is(rect.left, labelrect.left + 6, gTests[gTestIndex] + " left");
// Click to open popup in popupHidden() occurs at (4,4) in label's coordinate space
var clickX = clickY = 4;
is(rect.left, labelrect.left + clickX + (platformIsMac() ? 1 : 2), gTests[gTestIndex] + " left");
switch (gTests[gTestIndex]) {
case "context menu enough space below":
is(rect.top, labelrect.top + 6, gTests[gTestIndex] + " top");
is(rect.top, labelrect.top + clickY + (platformIsMac() ? -6 : 2), gTests[gTestIndex] + " top");
break;
case "context menu more space above":
is(rect.top, labelrect.top - rect.height + 2, gTests[gTestIndex] + " top");
is(rect.top, labelrect.top + clickY - rect.height - (platformIsMac() ? 0 : 2), gTests[gTestIndex] + " top");
break;
case "context menu too big either side":
[, gScreenY] = getScreenXY(document.documentElement);
@ -281,7 +284,7 @@ function testPopupMovement()
var screenX, screenY, buttonScreenX, buttonScreenY;
var rect = popup.getBoundingClientRect();
var overlapOSChrome = (navigator.platform.indexOf("Mac") == -1);
var overlapOSChrome = !platformIsMac();
popup.moveTo(1, 1);
[screenX, screenY] = getScreenXY(popup);
@ -345,6 +348,11 @@ function testPopupMovement()
popup.hidePopup();
}
function platformIsMac()
{
return navigator.platform.indexOf("Mac") > -1;
}
window.opener.wrappedJSObject.SimpleTest.waitForFocus(runTests, window);
]]>

View File

@ -404,7 +404,14 @@ public:
* Overlay scrollbar animation constants.
*/
eIntID_ScrollbarFadeBeginDelay,
eIntID_ScrollbarFadeDuration
eIntID_ScrollbarFadeDuration,
/**
* Distance in pixels to offset the context menu from the cursor
* on open.
*/
eIntID_ContextMenuOffsetVertical,
eIntID_ContextMenuOffsetHorizontal
};
/**

View File

@ -415,6 +415,11 @@ nsLookAndFeel::GetIntImpl(IntID aID, int32_t &aResult)
case eIntID_ScrollbarButtonAutoRepeatBehavior:
aResult = 0;
break;
case eIntID_ContextMenuOffsetVertical:
case eIntID_ContextMenuOffsetHorizontal:
aResult = 2;
break;
default:
aResult = 0;

View File

@ -473,6 +473,12 @@ nsLookAndFeel::GetIntImpl(IntID aID, int32_t &aResult)
case eIntID_ColorPickerAvailable:
aResult = 1;
break;
case eIntID_ContextMenuOffsetVertical:
aResult = -6;
break;
case eIntID_ContextMenuOffsetHorizontal:
aResult = 1;
break;
default:
aResult = 0;
res = NS_ERROR_FAILURE;

View File

@ -392,6 +392,11 @@ nsLookAndFeel::GetIntImpl(IntID aID, int32_t &aResult)
aResult = atoi(propValue);
break;
}
case eIntID_ContextMenuOffsetVertical:
case eIntID_ContextMenuOffsetHorizontal:
aResult = 2;
break;
default:
aResult = 0;

View File

@ -667,6 +667,10 @@ nsLookAndFeel::GetIntImpl(IntID aID, int32_t &aResult)
case eIntID_ColorPickerAvailable:
aResult = 1;
break;
case eIntID_ContextMenuOffsetVertical:
case eIntID_ContextMenuOffsetHorizontal:
aResult = 2;
break;
default:
aResult = 0;
res = NS_ERROR_FAILURE;

View File

@ -117,6 +117,12 @@ nsLookAndFeelIntPref nsXPLookAndFeel::sIntPrefs[] =
{ "ui.physicalHomeButton",
eIntID_PhysicalHomeButton,
false, 0 },
{ "ui.contextMenuOffsetVertical",
eIntID_ContextMenuOffsetVertical,
false, 0 },
{ "ui.contextMenuOffsetHorizontal",
eIntID_ContextMenuOffsetHorizontal,
false, 0 }
};
nsLookAndFeelFloatPref nsXPLookAndFeel::sFloatPrefs[] =

View File

@ -373,6 +373,11 @@ nsLookAndFeel::GetIntImpl(IntID aID, int32_t &aResult)
case eIntID_ScrollbarButtonAutoRepeatBehavior:
aResult = 0;
break;
case eIntID_ContextMenuOffsetVertical:
case eIntID_ContextMenuOffsetHorizontal:
aResult = 2;
break;
default:
aResult = 0;

View File

@ -345,6 +345,10 @@ nsLookAndFeel::GetIntImpl(IntID aID, int32_t &aResult)
case eIntID_SpellCheckerUnderlineStyle:
aResult = NS_STYLE_TEXT_DECORATION_STYLE_DOTTED;
break;
case eIntID_ContextMenuOffsetVertical:
case eIntID_ContextMenuOffsetHorizontal:
aResult = 2;
break;
default:
aResult = 0;
res = NS_ERROR_FAILURE;

View File

@ -487,6 +487,10 @@ nsLookAndFeel::GetIntImpl(IntID aID, int32_t &aResult)
case eIntID_ScrollbarFadeDuration:
aResult = 350;
break;
case eIntID_ContextMenuOffsetVertical:
case eIntID_ContextMenuOffsetHorizontal:
aResult = 2;
break;
default:
aResult = 0;
res = NS_ERROR_FAILURE;