From 25d176355363a6705428bea1c91759490243b2d0 Mon Sep 17 00:00:00 2001 From: "Carsten \"Tomcat\" Book" Date: Fri, 31 Jul 2015 14:49:40 +0200 Subject: [PATCH] Backed out changeset 7edc58c272f1 (bug 1075089) for causing OS X 10.6 marionette failures in test_single_finger_desktop.py testSingleFingerMouse.test_double_tap on a CLOSED TREE --- layout/xul/nsMenuPopupFrame.cpp | 41 ++++++++----------- layout/xul/nsMenuPopupFrame.h | 7 ++++ toolkit/content/tests/chrome/popup_trigger.js | 23 ++++------- .../content/tests/chrome/test_bug624329.xul | 19 +-------- .../tests/chrome/test_contextmenu_list.xul | 36 ++++++---------- .../content/tests/chrome/window_largemenu.xul | 18 +++----- widget/LookAndFeel.h | 9 +--- widget/android/nsLookAndFeel.cpp | 5 --- widget/cocoa/nsLookAndFeel.mm | 6 --- widget/gonk/nsLookAndFeel.cpp | 5 --- widget/gtk/nsLookAndFeel.cpp | 4 -- widget/nsXPLookAndFeel.cpp | 6 --- widget/qt/nsLookAndFeel.cpp | 5 --- widget/uikit/nsLookAndFeel.mm | 4 -- widget/windows/nsLookAndFeel.cpp | 4 -- 15 files changed, 52 insertions(+), 140 deletions(-) diff --git a/layout/xul/nsMenuPopupFrame.cpp b/layout/xul/nsMenuPopupFrame.cpp index 31a44194d9c..5a1b10c5325 100644 --- a/layout/xul/nsMenuPopupFrame.cpp +++ b/layout/xul/nsMenuPopupFrame.cpp @@ -60,11 +60,6 @@ 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 = @@ -1175,7 +1170,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 - std::max(aOffsetForContextMenu, 0); + nscoord newScreenPoint = startpos - aSize - aMarginBegin - aOffsetForContextMenu; if (newScreenPoint != aScreenPoint) { *aFlipSide = true; aScreenPoint = newScreenPoint; @@ -1321,7 +1316,7 @@ nsMenuPopupFrame::SetPopupPosition(nsIFrame* aAnchorFrame, bool aIsMove, bool aS nsRect rootScreenRect = rootFrame->GetScreenRectInAppUnits(); nsDeviceContext* devContext = presContext->DeviceContext(); - nsPoint offsetForContextMenu; + nscoord offsetForContextMenu = 0; bool isNoAutoHide = IsNoAutoHide(); nsPopupLevel popupLevel = PopupLevel(isNoAutoHide); @@ -1382,17 +1377,13 @@ nsMenuPopupFrame::SetPopupPosition(nsIFrame* aAnchorFrame, bool aIsMove, bool aS // coordinates. int32_t factor = devContext->AppUnitsPerDevPixelAtUnitFullZoom(); - // 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. + // 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. if (mAdjustOffsetForContextMenu) { - 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); + int32_t offsetForContextMenuDev = + nsPresContext::CSSPixelsToAppUnits(CONTEXT_MENU_OFFSET_PIXELS) / factor; + offsetForContextMenu = presContext->DevPixelsToAppUnits(offsetForContextMenuDev); } // next, convert into app units accounting for the zoom @@ -1403,8 +1394,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.x, - margin.top + offsetForContextMenu.y); + screenPoint.MoveBy(margin.left + offsetForContextMenu, + margin.top + offsetForContextMenu); // screen positioned popups can be flipped vertically but never horizontally vFlip = FlipStyle_Outside; @@ -1448,7 +1439,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.x, hFlip, + margin.left, margin.right, offsetForContextMenu, hFlip, &mHFlip); } if (slideVertical) { @@ -1457,7 +1448,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.y, vFlip, + margin.top, margin.bottom, offsetForContextMenu, vFlip, &mVFlip); } @@ -2167,10 +2158,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) { - margin.left += nsPresContext::CSSPixelsToAppUnits(LookAndFeel::GetInt( - LookAndFeel::eIntID_ContextMenuOffsetHorizontal)); - margin.top += nsPresContext::CSSPixelsToAppUnits(LookAndFeel::GetInt( - LookAndFeel::eIntID_ContextMenuOffsetVertical)); + nscoord offsetForContextMenu = + nsPresContext::CSSPixelsToAppUnits(CONTEXT_MENU_OFFSET_PIXELS); + margin.left += offsetForContextMenu; + margin.top += offsetForContextMenu; } nsPresContext* presContext = PresContext(); diff --git a/layout/xul/nsMenuPopupFrame.h b/layout/xul/nsMenuPopupFrame.h index d240a34d62d..a8d7878c46b 100644 --- a/layout/xul/nsMenuPopupFrame.h +++ b/layout/xul/nsMenuPopupFrame.h @@ -122,6 +122,13 @@ 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; diff --git a/toolkit/content/tests/chrome/popup_trigger.js b/toolkit/content/tests/chrome/popup_trigger.js index c9c430e2c75..4bfa9a3cc9e 100644 --- a/toolkit/content/tests/chrome/popup_trigger.js +++ b/toolkit/content/tests/chrome/popup_trigger.js @@ -507,11 +507,9 @@ var popupTests = [ } } - var openX = 8; - var openY = 16; var rect = gMenuPopup.getBoundingClientRect(); - is(rect.left, openX + (platformIsMac() ? 1 : 2), testname + " left"); - is(rect.top, openY + (platformIsMac() ? -6 : 2), testname + " top"); + is(rect.left, 10, testname + " left"); + is(rect.top, 18, testname + " top"); ok(rect.right, testname + " right is " + rect.right); ok(rect.bottom, testname + " bottom is " + rect.bottom); } @@ -751,7 +749,7 @@ var popupTests = [ autohide: "thepopup", test: function(testname, step) { gTrigger.focus(); - synthesizeKey("VK_DOWN", { altKey: !platformIsMac() }); + synthesizeKey("VK_DOWN", { altKey: (navigator.platform.indexOf("Mac") == -1) }); }, result: function(testname, step) { checkOpen("trigger", testname); @@ -764,7 +762,7 @@ var popupTests = [ events: [ "popupshowing thepopup", "popupshown thepopup" ], test: function(testname, step) { gTrigger.focus(); - synthesizeKey("VK_UP", { altKey: !platformIsMac() }); + synthesizeKey("VK_UP", { altKey: (navigator.platform.indexOf("Mac") == -1) }); }, result: function(testname, step) { checkOpen("trigger", testname); @@ -791,7 +789,7 @@ var popupTests = [ autohide: "thepopup", test: function(testname, step) { gTrigger.focus(); - synthesizeKey(platformIsMac() ? " " : "VK_F4", { }); + synthesizeKey((navigator.platform.indexOf("Mac") == -1) ? "VK_F4" : " ", { }); }, result: function(testname, step) { checkOpen("trigger", testname); @@ -804,10 +802,10 @@ var popupTests = [ condition: function() { return gIsMenu; }, test: function(testname, step) { gTrigger.focus(); - if (platformIsMac()) - synthesizeKey("VK_F4", { altKey: true }); - else + if (navigator.platform.indexOf("Mac") == -1) synthesizeKey("", { metaKey: true }); + else + synthesizeKey("VK_F4", { altKey: true }); }, result: function(testname, step) { checkClosed("trigger", testname); @@ -852,8 +850,3 @@ var popupTests = [ } ]; - -function platformIsMac() -{ - return navigator.platform.indexOf("Mac") > -1; -} diff --git a/toolkit/content/tests/chrome/test_bug624329.xul b/toolkit/content/tests/chrome/test_bug624329.xul index 46ee919e9ac..c37e7edd9b4 100644 --- a/toolkit/content/tests/chrome/test_bug624329.xul +++ b/toolkit/content/tests/chrome/test_bug624329.xul @@ -103,23 +103,8 @@ function openContextMenu() { var x = menubox.screenX - winbox.screenX; var y = menubox.screenY - winbox.screenY; - - 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, + "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); diff --git a/toolkit/content/tests/chrome/test_contextmenu_list.xul b/toolkit/content/tests/chrome/test_contextmenu_list.xul index 157831a5816..9fff789a4e7 100644 --- a/toolkit/content/tests/chrome/test_contextmenu_list.xul +++ b/toolkit/content/tests/chrome/test_contextmenu_list.xul @@ -189,7 +189,7 @@ function checkContextMenu(event) // context menu from mouse click switch (gTestId) { case -1: - var expected = gSelectionStep == 2 ? 1 : (platformIsMac() ? 3 : 0); + var expected = gSelectionStep == 2 ? 1 : (navigator.platform.indexOf("Mac") >= 0 ? 3 : 0); is($(gTestElement).selectedIndex, expected, "index after click " + gSelectionStep); break; case 0: @@ -222,57 +222,47 @@ 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 + contextMenuOffsetX, + isRoundedX(menurect.left, itemrect.left + 2, "list selection keyboard left"); - isRoundedY(menurect.top, itemrect.bottom + contextMenuOffsetY, + isRoundedY(menurect.top, itemrect.bottom + 2, "list selection keyboard top"); } else { var tree = $("tree"); var bodyrect = $("treechildren").getBoundingClientRect(); - isRoundedX(menurect.left, bodyrect.left + contextMenuOffsetX, + isRoundedX(menurect.left, bodyrect.left + 2, "tree selection keyboard left"); isRoundedY(menurect.top, bodyrect.top + - tree.treeBoxObject.rowHeight * 3 + contextMenuOffsetY, + tree.treeBoxObject.rowHeight * 3 + 2, "tree selection keyboard top"); } } else if (gTestId == 1) { - // activating a context menu with the mouse from position (7, 4). + // 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. var elementrect = $(gTestElement).getBoundingClientRect(); - isRoundedX(menurect.left, elementrect.left + 7 + contextMenuOffsetX, + isRoundedX(menurect.left, elementrect.left + 9, gTestElement + " mouse left"); - isRoundedY(menurect.top, elementrect.top + 4 + contextMenuOffsetY, + isRoundedY(menurect.top, elementrect.top + 6, gTestElement + " mouse top"); } else { var elementrect = $(gTestElement).getBoundingClientRect(); - isRoundedX(menurect.left, elementrect.left + contextMenuOffsetX, + isRoundedX(menurect.left, elementrect.left + 2, gTestElement + " no selection keyboard left"); - isRoundedY(menurect.top, elementrect.bottom + contextMenuOffsetY, + isRoundedY(menurect.top, elementrect.bottom + 2, gTestElement + " no selection keyboard top"); } $("themenu").hidePopup(); } -function platformIsMac() -{ - return navigator.platform.indexOf("Mac") > -1; -} - ]]> diff --git a/toolkit/content/tests/chrome/window_largemenu.xul b/toolkit/content/tests/chrome/window_largemenu.xul index 1ba74f0e54d..305c9910d4e 100644 --- a/toolkit/content/tests/chrome/window_largemenu.xul +++ b/toolkit/content/tests/chrome/window_largemenu.xul @@ -214,17 +214,14 @@ function contextMenuPopupShown() var popup = document.getElementById("popup"); var rect = popup.getBoundingClientRect(); var labelrect = document.getElementById("label").getBoundingClientRect(); - - // 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"); + + is(rect.left, labelrect.left + 6, gTests[gTestIndex] + " left"); switch (gTests[gTestIndex]) { case "context menu enough space below": - is(rect.top, labelrect.top + clickY + (platformIsMac() ? -6 : 2), gTests[gTestIndex] + " top"); + is(rect.top, labelrect.top + 6, gTests[gTestIndex] + " top"); break; case "context menu more space above": - is(rect.top, labelrect.top + clickY - rect.height - (platformIsMac() ? 0 : 2), gTests[gTestIndex] + " top"); + is(rect.top, labelrect.top - rect.height + 2, gTests[gTestIndex] + " top"); break; case "context menu too big either side": [, gScreenY] = getScreenXY(document.documentElement); @@ -284,7 +281,7 @@ function testPopupMovement() var screenX, screenY, buttonScreenX, buttonScreenY; var rect = popup.getBoundingClientRect(); - var overlapOSChrome = !platformIsMac(); + var overlapOSChrome = (navigator.platform.indexOf("Mac") == -1); popup.moveTo(1, 1); [screenX, screenY] = getScreenXY(popup); @@ -348,11 +345,6 @@ function testPopupMovement() popup.hidePopup(); } -function platformIsMac() -{ - return navigator.platform.indexOf("Mac") > -1; -} - window.opener.wrappedJSObject.SimpleTest.waitForFocus(runTests, window); ]]> diff --git a/widget/LookAndFeel.h b/widget/LookAndFeel.h index ed9995553c4..98816b2d3ea 100644 --- a/widget/LookAndFeel.h +++ b/widget/LookAndFeel.h @@ -404,14 +404,7 @@ public: * Overlay scrollbar animation constants. */ eIntID_ScrollbarFadeBeginDelay, - eIntID_ScrollbarFadeDuration, - - /** - * Distance in pixels to offset the context menu from the cursor - * on open. - */ - eIntID_ContextMenuOffsetVertical, - eIntID_ContextMenuOffsetHorizontal + eIntID_ScrollbarFadeDuration }; /** diff --git a/widget/android/nsLookAndFeel.cpp b/widget/android/nsLookAndFeel.cpp index 6b260e6ec15..908a77e87ca 100644 --- a/widget/android/nsLookAndFeel.cpp +++ b/widget/android/nsLookAndFeel.cpp @@ -415,11 +415,6 @@ 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; diff --git a/widget/cocoa/nsLookAndFeel.mm b/widget/cocoa/nsLookAndFeel.mm index 463496bd880..424f8754162 100644 --- a/widget/cocoa/nsLookAndFeel.mm +++ b/widget/cocoa/nsLookAndFeel.mm @@ -473,12 +473,6 @@ 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; diff --git a/widget/gonk/nsLookAndFeel.cpp b/widget/gonk/nsLookAndFeel.cpp index 120f257df40..3aae4eae86e 100644 --- a/widget/gonk/nsLookAndFeel.cpp +++ b/widget/gonk/nsLookAndFeel.cpp @@ -392,11 +392,6 @@ nsLookAndFeel::GetIntImpl(IntID aID, int32_t &aResult) aResult = atoi(propValue); break; } - - case eIntID_ContextMenuOffsetVertical: - case eIntID_ContextMenuOffsetHorizontal: - aResult = 2; - break; default: aResult = 0; diff --git a/widget/gtk/nsLookAndFeel.cpp b/widget/gtk/nsLookAndFeel.cpp index d6778f054d4..06972e67056 100644 --- a/widget/gtk/nsLookAndFeel.cpp +++ b/widget/gtk/nsLookAndFeel.cpp @@ -667,10 +667,6 @@ 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; diff --git a/widget/nsXPLookAndFeel.cpp b/widget/nsXPLookAndFeel.cpp index 281a0addbba..cef5ebf5f89 100644 --- a/widget/nsXPLookAndFeel.cpp +++ b/widget/nsXPLookAndFeel.cpp @@ -117,12 +117,6 @@ nsLookAndFeelIntPref nsXPLookAndFeel::sIntPrefs[] = { "ui.physicalHomeButton", eIntID_PhysicalHomeButton, false, 0 }, - { "ui.contextMenuOffsetVertical", - eIntID_ContextMenuOffsetVertical, - false, 0 }, - { "ui.contextMenuOffsetHorizontal", - eIntID_ContextMenuOffsetHorizontal, - false, 0 } }; nsLookAndFeelFloatPref nsXPLookAndFeel::sFloatPrefs[] = diff --git a/widget/qt/nsLookAndFeel.cpp b/widget/qt/nsLookAndFeel.cpp index 50d943e8dc2..890e224d195 100644 --- a/widget/qt/nsLookAndFeel.cpp +++ b/widget/qt/nsLookAndFeel.cpp @@ -373,11 +373,6 @@ 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; diff --git a/widget/uikit/nsLookAndFeel.mm b/widget/uikit/nsLookAndFeel.mm index bb593eb51e9..c28f52c0313 100644 --- a/widget/uikit/nsLookAndFeel.mm +++ b/widget/uikit/nsLookAndFeel.mm @@ -345,10 +345,6 @@ 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; diff --git a/widget/windows/nsLookAndFeel.cpp b/widget/windows/nsLookAndFeel.cpp index f50b5f2f3da..e4177d57bfd 100644 --- a/widget/windows/nsLookAndFeel.cpp +++ b/widget/windows/nsLookAndFeel.cpp @@ -487,10 +487,6 @@ 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;