diff --git a/dom/events/test/clipboard/browser_navigator_clipboard_clickjacking.js b/dom/events/test/clipboard/browser_navigator_clipboard_clickjacking.js index cd3e97f27450..fa8d847eea1b 100644 --- a/dom/events/test/clipboard/browser_navigator_clipboard_clickjacking.js +++ b/dom/events/test/clipboard/browser_navigator_clipboard_clickjacking.js @@ -63,7 +63,7 @@ add_task(async function test_paste_button_clickjacking() { ); const pasteButtonIsHidden = promisePasteButtonIsHidden(); - EventUtils.synthesizeKey(accesskey, {}, window); + pastePopup.activateItem(pasteButton); await pasteButtonIsHidden; }); }); diff --git a/dom/events/test/clipboard/head.js b/dom/events/test/clipboard/head.js index 73404ca5350b..53adf3de555c 100644 --- a/dom/events/test/clipboard/head.js +++ b/dom/events/test/clipboard/head.js @@ -70,8 +70,9 @@ function promisePasteButtonIsHidden() { function promiseClickPasteButton() { const pasteButton = document.getElementById(kPasteMenuItemId); - let promise = BrowserTestUtils.waitForEvent(pasteButton, "click"); - EventUtils.synthesizeMouseAtCenter(pasteButton, {}); + const popup = document.getElementById(kPasteMenuPopupId); + let promise = BrowserTestUtils.waitForEvent(pasteButton, "command"); + popup.activateItem(pasteButton); return promise; } @@ -95,8 +96,8 @@ function getMouseCoordsRelativeToScreenInDevicePixels() { function isCloselyLeftOnTopOf(aCoordsP1, aCoordsP2, aDelta = 10) { return ( - Math.abs(aCoordsP2.x - aCoordsP1.x) < aDelta && - Math.abs(aCoordsP2.y - aCoordsP1.y) < aDelta + Math.abs(aCoordsP2.x - aCoordsP1.x) <= aDelta && + Math.abs(aCoordsP2.y - aCoordsP1.y) <= aDelta ); } diff --git a/dom/xul/XULPopupElement.cpp b/dom/xul/XULPopupElement.cpp index d60173900ec1..d8fcb5c65ae9 100644 --- a/dom/xul/XULPopupElement.cpp +++ b/dom/xul/XULPopupElement.cpp @@ -66,20 +66,20 @@ void XULPopupElement::OpenPopup(Element* aAnchorElement, } nsXULPopupManager* pm = nsXULPopupManager::GetInstance(); - if (pm) { - // As a special case for popups that are menus when no anchor or position - // are specified, open the popup with ShowMenu instead of ShowPopup so that - // the popup is aligned with the menu. - if (!aAnchorElement && position.IsEmpty() && GetPrimaryFrame()) { - if (auto* menu = GetContainingMenu()) { - pm->ShowMenu(menu, false); - return; - } - } - - pm->ShowPopup(this, aAnchorElement, position, aXPos, aYPos, aIsContextMenu, - aAttributesOverride, false, aTriggerEvent); + if (!pm) { + return; } + // As a special case for popups that are menus when no anchor or position + // are specified, open the popup with ShowMenu instead of ShowPopup so that + // the popup is aligned with the menu. + if (!aAnchorElement && position.IsEmpty() && GetPrimaryFrame()) { + if (auto* menu = GetContainingMenu()) { + pm->ShowMenu(menu, false); + return; + } + } + pm->ShowPopup(this, aAnchorElement, position, aXPos, aYPos, aIsContextMenu, + aAttributesOverride, false, aTriggerEvent); } void XULPopupElement::OpenPopupAtScreen(int32_t aXPos, int32_t aYPos, diff --git a/layout/xul/nsMenuPopupFrame.cpp b/layout/xul/nsMenuPopupFrame.cpp index 982a22841b30..4fcf71cb9603 100644 --- a/layout/xul/nsMenuPopupFrame.cpp +++ b/layout/xul/nsMenuPopupFrame.cpp @@ -480,7 +480,7 @@ void nsMenuPopupFrame::TweakMinPrefISize(nscoord& aSize) { if (nsIFrame* menuList = GetInFlowParent()) { menuListOrAnchorWidth = menuList->GetRect().width; } - if (mAnchorType == MenuPopupAnchorType_Rect) { + if (mAnchorType == MenuPopupAnchorType::Rect) { menuListOrAnchorWidth = std::max(menuListOrAnchorWidth, mScreenRect.width); } // Input margin doesn't have contents, so account for it for popup sizing @@ -768,9 +768,22 @@ void nsMenuPopupFrame::InitializePopup(nsIContent* aAnchorContent, mPopupState = ePopupShowing; mAnchorContent = aAnchorContent; + mAnchorType = aAnchorType; + const nscoord auPerCssPx = AppUnitsPerCSSPixel(); + const nsPoint pos = CSSPixel::ToAppUnits(CSSIntPoint(aXPos, aYPos)); + // When converted back to CSSIntRect it is (-1, -1, 0, 0) - as expected in + // nsXULPopupManager::Rollup + mScreenRect = nsRect(-auPerCssPx, -auPerCssPx, 0, 0); + mExtraMargin = pos; + // If we have no anchor node, anchor to the given position instead. + if (mAnchorType == MenuPopupAnchorType::Node && !aAnchorContent) { + mAnchorType = MenuPopupAnchorType::Point; + mScreenRect = nsRect( + pos + PresShell()->GetRootFrame()->GetScreenRectInAppUnits().TopLeft(), + nsSize()); + mExtraMargin = {}; + } mTriggerContent = aTriggerContent; - mXPos = aXPos; - mYPos = aYPos; mIsNativeMenu = false; mIsTopLevelContextMenu = false; mVFlip = false; @@ -780,12 +793,10 @@ void nsMenuPopupFrame::InitializePopup(nsIContent* aAnchorContent, mPositionedOffset = 0; mPositionedByMoveToRect = false; - mAnchorType = aAnchorType; - // if aAttributesOverride is true, then the popupanchor, popupalign and // position attributes on the override those values passed in. // If false, those attributes are only used if the values passed in are empty - if (aAnchorContent || aAnchorType == MenuPopupAnchorType_Rect) { + if (aAnchorContent || aAnchorType == MenuPopupAnchorType::Rect) { nsAutoString anchor, align, position; mContent->AsElement()->GetAttr(nsGkAtoms::popupanchor, anchor); mContent->AsElement()->GetAttr(nsGkAtoms::popupalign, align); @@ -796,8 +807,6 @@ void nsMenuPopupFrame::InitializePopup(nsIContent* aAnchorContent, // the offset is used to adjust the position from the anchor point if (anchor.IsEmpty() && align.IsEmpty() && position.IsEmpty()) position.Assign(aPosition); - else - mXPos = mYPos = 0; } else if (!aPosition.IsEmpty()) { position.Assign(aPosition); } @@ -855,9 +864,6 @@ void nsMenuPopupFrame::InitializePopup(nsIContent* aAnchorContent, InitPositionFromAnchorAlign(anchor, align); } } - // When converted back to CSSIntRect it is (-1, -1, 0, 0) - as expected in - // nsXULPopupManager::Rollup - mScreenRect = nsRect(-AppUnitsPerCSSPixel(), -AppUnitsPerCSSPixel(), 0, 0); if (aAttributesOverride) { // Use |left| and |top| dimension attributes to position the popup if @@ -893,9 +899,8 @@ void nsMenuPopupFrame::InitializePopupAtScreen(nsIContent* aTriggerContent, mAnchorContent = nullptr; mTriggerContent = aTriggerContent; mScreenRect = - nsRect(CSSPixel::ToAppUnits(aXPos), CSSPixel::ToAppUnits(aYPos), 0, 0); - mXPos = 0; - mYPos = 0; + nsRect(CSSPixel::ToAppUnits(CSSIntPoint(aXPos, aYPos)), nsSize()); + mExtraMargin = {}; mFlip = FlipFromAttribute(this); mPopupAnchor = POPUPALIGNMENT_NONE; mPopupAlignment = POPUPALIGNMENT_NONE; @@ -903,7 +908,7 @@ void nsMenuPopupFrame::InitializePopupAtScreen(nsIContent* aTriggerContent, mIsContextMenu = aIsContextMenu; mIsTopLevelContextMenu = aIsContextMenu; mIsNativeMenu = false; - mAnchorType = MenuPopupAnchorType_Point; + mAnchorType = MenuPopupAnchorType::Point; mPositionedOffset = 0; mPositionedByMoveToRect = false; } @@ -914,9 +919,8 @@ void nsMenuPopupFrame::InitializePopupAsNativeContextMenu( mPopupState = ePopupShowing; mAnchorContent = nullptr; mScreenRect = - nsRect(CSSPixel::ToAppUnits(aXPos), CSSPixel::ToAppUnits(aYPos), 0, 0); - mXPos = 0; - mYPos = 0; + nsRect(CSSPixel::ToAppUnits(CSSIntPoint(aXPos, aYPos)), nsSize()); + mExtraMargin = {}; mFlip = FlipType_Default; mPopupAnchor = POPUPALIGNMENT_NONE; mPopupAlignment = POPUPALIGNMENT_NONE; @@ -924,7 +928,7 @@ void nsMenuPopupFrame::InitializePopupAsNativeContextMenu( mIsContextMenu = true; mIsTopLevelContextMenu = true; mIsNativeMenu = true; - mAnchorType = MenuPopupAnchorType_Point; + mAnchorType = MenuPopupAnchorType::Point; mPositionedOffset = 0; mPositionedByMoveToRect = false; } @@ -934,7 +938,7 @@ void nsMenuPopupFrame::InitializePopupAtRect(nsIContent* aTriggerContent, const nsIntRect& aRect, bool aAttributesOverride) { InitializePopup(nullptr, aTriggerContent, aPosition, 0, 0, - MenuPopupAnchorType_Rect, aAttributesOverride); + MenuPopupAnchorType::Rect, aAttributesOverride); mScreenRect = ToAppUnits(aRect, AppUnitsPerCSSPixel()); } @@ -1447,7 +1451,7 @@ auto nsMenuPopupFrame::GetRects(const nsSize& aPrefSize) const -> Rects { result.mAnchorRect = result.mUntransformedAnchorRect = [&] { // If anchored to a rectangle, use that rectangle. Otherwise, determine // the rectangle from the anchor. - if (mAnchorType == MenuPopupAnchorType_Rect) { + if (mAnchorType == MenuPopupAnchorType::Rect) { return mScreenRect; } // if the frame is not specified, use the anchor node passed to OpenPopup. @@ -1455,7 +1459,7 @@ auto nsMenuPopupFrame::GetRects(const nsSize& aPrefSize) const -> Rects { // mAnchorContent might be a different document so its presshell must be // used. nsIFrame* anchorFrame = GetAnchorFrame(); - if (!anchorFrame) { + if (NS_WARN_IF(!anchorFrame)) { return rootScreenRect; } return ComputeAnchorRect(rootPc, anchorFrame); @@ -1467,7 +1471,7 @@ auto nsMenuPopupFrame::GetRects(const nsSize& aPrefSize) const -> Rects { // When doing this reposition, we want to move the popup to the side with // the most room. The combination of anchor and alignment dictate if we // readjust above/below or to the left/right. - if (mAnchorContent || mAnchorType == MenuPopupAnchorType_Rect) { + if (mAnchorContent || mAnchorType == MenuPopupAnchorType::Rect) { // move the popup according to the anchor and alignment. This will also // tell us which axis the popup is flush against in case we have to move // it around later. The AdjustPositionForAnchorAlign method accounts for @@ -1479,20 +1483,6 @@ auto nsMenuPopupFrame::GetRects(const nsSize& aPrefSize) const -> Rects { result.mUsedRect.MoveTo(result.mAnchorRect.TopLeft() + nsPoint(margin.left, margin.top)); } - - // mXPos and mYPos specify an additional offset passed to OpenPopup that - // should be added to the position. We also add the offset to the anchor - // pos so a later flip/resize takes the offset into account. - // FIXME(emilio): Wayland doesn't seem to be accounting for this offset - // anywhere, and it probably should. - { - nsPoint offset(CSSPixel::ToAppUnits(mXPos), CSSPixel::ToAppUnits(mYPos)); - if (IsDirectionRTL()) { - offset.x = -offset.x; - } - result.mUsedRect.MoveBy(offset); - result.mAnchorRect.MoveBy(offset); - } } else { // Not anchored, use mScreenRect result.mUsedRect.MoveTo(mScreenRect.TopLeft()); @@ -1730,12 +1720,12 @@ void nsMenuPopupFrame::PerformMove(const Rects& aRects) { // by the anchor are lost, but this is super-old behavior. const bool fixPositionToPoint = IsNoAutoHide() && (GetPopupLevel() != PopupLevel::Parent || - mAnchorType == MenuPopupAnchorType_Rect); + mAnchorType == MenuPopupAnchorType::Rect); if (fixPositionToPoint) { // Account for the margin that will end up being added to the screen // coordinate the next time SetPopupPosition is called. const auto& margin = GetMargin(); - mAnchorType = MenuPopupAnchorType_Point; + mAnchorType = MenuPopupAnchorType::Point; mScreenRect.x = aRects.mUsedRect.x - margin.left; mScreenRect.y = aRects.mUsedRect.y - margin.top; } @@ -1743,8 +1733,8 @@ void nsMenuPopupFrame::PerformMove(const Rects& aRects) { // For anchored popups that shouldn't follow the anchor, fix the original // anchor rect. if (IsAnchored() && !ShouldFollowAnchor() && !mUsedScreenRect.IsEmpty() && - mAnchorType != MenuPopupAnchorType_Rect) { - mAnchorType = MenuPopupAnchorType_Rect; + mAnchorType != MenuPopupAnchorType::Rect) { + mAnchorType = MenuPopupAnchorType::Rect; mScreenRect = aRects.mUntransformedAnchorRect; } @@ -2179,6 +2169,19 @@ nsMargin nsMenuPopupFrame::GetMargin() const { margin.top += auOffset; margin.bottom += auOffset; } + // TODO(emilio): We should consider make these properly mirrored (that is, + // changing -= to += here, and removing the rtl special case), but some tests + // rely on the old behavior of the anchor moving physically regardless of + // alignment... + margin.top += mExtraMargin.y; + margin.bottom -= mExtraMargin.y; + if (IsDirectionRTL()) { + margin.left -= mExtraMargin.x; + margin.right += mExtraMargin.x; + } else { + margin.left += mExtraMargin.x; + margin.right -= mExtraMargin.x; + } return margin; } @@ -2212,19 +2215,16 @@ void nsMenuPopupFrame::MoveTo(const CSSPoint& aPos, bool aUpdateAttrs, mPositionedByMoveToRect = aByMoveToRect; mScreenRect.MoveTo(appUnitsPos); - if (mAnchorType == MenuPopupAnchorType_Rect) { + if (mAnchorType == MenuPopupAnchorType::Rect) { // This ensures that the anchor width is still honored, to prevent it from // changing spuriously. mScreenRect.height = 0; // But we still need to make sure that our top left position ends up in // appUnitsPos. - mPopupAlignment = rtl ? POPUPALIGNMENT_TOPRIGHT : POPUPALIGNMENT_TOPLEFT; mPopupAnchor = rtl ? POPUPALIGNMENT_BOTTOMRIGHT : POPUPALIGNMENT_BOTTOMLEFT; - mXPos = mYPos = 0; - } else { - mAnchorType = MenuPopupAnchorType_Point; + mAnchorType = MenuPopupAnchorType::Point; } SetPopupPosition(true); @@ -2248,7 +2248,7 @@ void nsMenuPopupFrame::MoveToAnchor(nsIContent* aAnchorContent, nsPopupState oldstate = mPopupState; InitializePopup(aAnchorContent, mTriggerContent, aPosition, aXPos, aYPos, - MenuPopupAnchorType_Node, aAttributesOverride); + MenuPopupAnchorType::Node, aAttributesOverride); // InitializePopup changed the state so reset it. mPopupState = oldstate; @@ -2344,7 +2344,7 @@ void nsMenuPopupFrame::CreatePopupView() { } bool nsMenuPopupFrame::ShouldFollowAnchor() const { - if (mAnchorType != MenuPopupAnchorType_Node || !mAnchorContent) { + if (mAnchorType != MenuPopupAnchorType::Node || !mAnchorContent) { return false; } diff --git a/layout/xul/nsMenuPopupFrame.h b/layout/xul/nsMenuPopupFrame.h index eed6b54d3a1e..107f3c81906f 100644 --- a/layout/xul/nsMenuPopupFrame.h +++ b/layout/xul/nsMenuPopupFrame.h @@ -63,10 +63,10 @@ enum FlipType { FlipType_Slide = 3 // allow the arrow to "slide" instead of resizing }; -enum MenuPopupAnchorType { - MenuPopupAnchorType_Node = 0, // anchored to a node - MenuPopupAnchorType_Point = 1, // unanchored and positioned at a screen point - MenuPopupAnchorType_Rect = 2, // anchored at a screen rectangle +enum class MenuPopupAnchorType : uint8_t { + Node = 0, // anchored to a node + Point = 1, // unanchored, and positioned at a screen point + Rect = 2, // anchored at a screen rectangle }; // values are selected so that the direction can be flipped just by @@ -382,7 +382,7 @@ class nsMenuPopupFrame final : public nsBlockFrame { void PerformMove(const Rects&); // Return true if the popup is positioned relative to an anchor. - bool IsAnchored() const { return mAnchorType != MenuPopupAnchorType_Point; } + bool IsAnchored() const { return mAnchorType != MenuPopupAnchorType::Point; } // Return the anchor if there is one. nsIContent* GetAnchor() const { return mAnchorContent; } @@ -565,11 +565,11 @@ class nsMenuPopupFrame final : public nsBlockFrame { // would be before resizing. Computations are performed using this size. nsSize mPrefSize{-1, -1}; - // The position of the popup, in CSS pixels. - // The screen coordinates, if set to values other than -1, - // override mXPos and mYPos. - int32_t mXPos = 0; - int32_t mYPos = 0; + // A point with extra offsets to apply in the horizontal and vertical axes. We + // don't use an nsMargin because the values would be the same for the same + // axis. + nsPoint mExtraMargin; + nsRect mScreenRect; // Used for store rectangle which the popup is going to be anchored to, we // need that for Wayland. It's important that this rect is unflipped, and @@ -634,7 +634,7 @@ class nsMenuPopupFrame final : public nsBlockFrame { mutable nscoord mPositionedOffset = 0; // How the popup is anchored. - MenuPopupAnchorType mAnchorType = MenuPopupAnchorType_Node; + MenuPopupAnchorType mAnchorType = MenuPopupAnchorType::Node; nsRect mOverrideConstraintRect; diff --git a/layout/xul/nsXULPopupManager.cpp b/layout/xul/nsXULPopupManager.cpp index 69e5ecef412a..fe1d983b5b7f 100644 --- a/layout/xul/nsXULPopupManager.cpp +++ b/layout/xul/nsXULPopupManager.cpp @@ -855,7 +855,7 @@ void nsXULPopupManager::ShowMenu(nsIContent* aMenu, bool aSelectFirstItem) { // there is no trigger event for menus popupFrame->InitializePopup(aMenu, nullptr, position, 0, 0, - MenuPopupAnchorType_Node, true); + MenuPopupAnchorType::Node, true); PendingPopup pendingPopup(&popupFrame->PopupElement(), nullptr); BeginShowingPopup(pendingPopup, parentIsContextMenu, aSelectFirstItem); } @@ -897,7 +897,7 @@ void nsXULPopupManager::ShowPopup(Element* aPopup, nsIContent* aAnchorContent, nsCOMPtr triggerContent = pendingPopup.GetTriggerContent(); popupFrame->InitializePopup(aAnchorContent, triggerContent, aPosition, aXPos, - aYPos, MenuPopupAnchorType_Node, + aYPos, MenuPopupAnchorType::Node, aAttributesOverride); BeginShowingPopup(pendingPopup, aIsContextMenu, aSelectFirstItem); diff --git a/toolkit/modules/ClipboardContextMenu.sys.mjs b/toolkit/modules/ClipboardContextMenu.sys.mjs index d66a2f466d5b..d68e89c2530d 100644 --- a/toolkit/modules/ClipboardContextMenu.sys.mjs +++ b/toolkit/modules/ClipboardContextMenu.sys.mjs @@ -135,13 +135,12 @@ export var ClipboardContextMenu = { // on the same `_menupopup` object. If the popup is already open, // `openPopup` is a no-op. When the popup is clicked or dismissed both // actor parents will receive the corresponding event. - this._menupopup.openPopup( - null, - "overlap" /* options */, - mouseXInCSSPixels.value, - mouseYInCSSPixels.value, - true /* isContextMenu */ - ); + this._menupopup.openPopup(null, { + isContextMenu: true, + position: "overlap", + x: mouseXInCSSPixels.value, + y: mouseYInCSSPixels.value, + }); this._refreshDelayTimer(document); }); diff --git a/widget/tests/browser/browser_test_clipboard_contextmenu.js b/widget/tests/browser/browser_test_clipboard_contextmenu.js index 80e8c3328258..46d32623c9ec 100644 --- a/widget/tests/browser/browser_test_clipboard_contextmenu.js +++ b/widget/tests/browser/browser_test_clipboard_contextmenu.js @@ -45,8 +45,8 @@ async function waitForPasteContextMenu() { function promiseClickPasteButton() { info("Wait for clicking paste contextmenu"); const pasteButton = document.getElementById(kPasteMenuItemId); - let promise = BrowserTestUtils.waitForEvent(pasteButton, "click"); - EventUtils.synthesizeMouseAtCenter(pasteButton, {}); + let promise = BrowserTestUtils.waitForEvent(pasteButton, "command"); + document.getElementById(kPasteMenuPopupId).activateItem(pasteButton); return promise; }