From bbe36d6bf70e36b15d7a430e578f4df9547be364 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Mon, 19 Dec 2022 16:15:52 +0000 Subject: [PATCH] Bug 1805694 - Simplify menulist popup layout. r=tnikkel I decided to split this up from bug 1805414 because it's only tangentially related to the removal of nsMenuFrame. We have this sizetopopup attribute and behavior that allows menulists to be sized to the contents of the popup. The popup also expands to the menulist rect. This means that layout is by somewhat cyclic and we need to go out of our way to support it. However, I think we only care about the first behavior. We don't have many non-intrinsically-sized menulists, and if we need we can use HTML popup, but I'd like to fix that eventually, so I'd rather leave them in, they're harmless). Differential Revision: https://phabricator.services.mozilla.com/D164693 --- dom/xul/XULPopupElement.cpp | 2 +- .../xul/menulist-shrinkwrap-1-ref.xhtml | 10 -- .../reftests/xul/menulist-shrinkwrap-1.xhtml | 10 -- .../xul/menulist-shrinkwrap-2-ref.xhtml | 25 --- .../reftests/xul/menulist-shrinkwrap-2.xhtml | 25 --- layout/reftests/xul/reftest.list | 3 - layout/xul/nsMenuFrame.cpp | 82 +-------- layout/xul/nsMenuFrame.h | 10 +- layout/xul/nsMenuPopupFrame.cpp | 163 +++++++----------- layout/xul/nsMenuPopupFrame.h | 30 ++-- layout/xul/nsXULPopupManager.cpp | 12 +- .../content/tests/chrome/test_menulist.xhtml | 4 - .../tests/chrome/test_menulist_paging.xhtml | 19 +- .../tests/chrome/test_menulist_position.xhtml | 2 +- 14 files changed, 101 insertions(+), 296 deletions(-) delete mode 100644 layout/reftests/xul/menulist-shrinkwrap-1-ref.xhtml delete mode 100644 layout/reftests/xul/menulist-shrinkwrap-1.xhtml delete mode 100644 layout/reftests/xul/menulist-shrinkwrap-2-ref.xhtml delete mode 100644 layout/reftests/xul/menulist-shrinkwrap-2.xhtml diff --git a/dom/xul/XULPopupElement.cpp b/dom/xul/XULPopupElement.cpp index e1c86bfc72d8..5e8e9b27584d 100644 --- a/dom/xul/XULPopupElement.cpp +++ b/dom/xul/XULPopupElement.cpp @@ -187,7 +187,7 @@ void XULPopupElement::SizeTo(int32_t aWidth, int32_t aHeight) { // with notifications set to true so that the popuppositioned event is fired. nsMenuPopupFrame* menuPopupFrame = do_QueryFrame(GetPrimaryFrame()); if (menuPopupFrame && menuPopupFrame->PopupState() == ePopupShown) { - menuPopupFrame->SetPopupPosition(nullptr, false, false); + menuPopupFrame->SetPopupPosition(false); } } diff --git a/layout/reftests/xul/menulist-shrinkwrap-1-ref.xhtml b/layout/reftests/xul/menulist-shrinkwrap-1-ref.xhtml deleted file mode 100644 index 06449e31faa6..000000000000 --- a/layout/reftests/xul/menulist-shrinkwrap-1-ref.xhtml +++ /dev/null @@ -1,10 +0,0 @@ - - - - - - - - - - diff --git a/layout/reftests/xul/menulist-shrinkwrap-1.xhtml b/layout/reftests/xul/menulist-shrinkwrap-1.xhtml deleted file mode 100644 index 80908dc3655a..000000000000 --- a/layout/reftests/xul/menulist-shrinkwrap-1.xhtml +++ /dev/null @@ -1,10 +0,0 @@ - - - - - - - - - - diff --git a/layout/reftests/xul/menulist-shrinkwrap-2-ref.xhtml b/layout/reftests/xul/menulist-shrinkwrap-2-ref.xhtml deleted file mode 100644 index e024cb67f594..000000000000 --- a/layout/reftests/xul/menulist-shrinkwrap-2-ref.xhtml +++ /dev/null @@ -1,25 +0,0 @@ - - - - - - - - - - - - - - - - - diff --git a/layout/reftests/xul/menulist-shrinkwrap-2.xhtml b/layout/reftests/xul/menulist-shrinkwrap-2.xhtml deleted file mode 100644 index 3b8db2a6db3b..000000000000 --- a/layout/reftests/xul/menulist-shrinkwrap-2.xhtml +++ /dev/null @@ -1,25 +0,0 @@ - - - - - - - - - - - - - - - - - diff --git a/layout/reftests/xul/reftest.list b/layout/reftests/xul/reftest.list index 862e002bcbb1..8b2c36061baf 100644 --- a/layout/reftests/xul/reftest.list +++ b/layout/reftests/xul/reftest.list @@ -2,9 +2,6 @@ fuzzy-if(cocoaWidget&&!nativeThemePref,0-7,0-2) == chrome://reftest/content/xul/css-grid-with-xul-item-1.xhtml chrome://reftest/content/xul/css-grid-with-xul-item-1-ref.xhtml == chrome://reftest/content/xul/menuitem-key.xhtml chrome://reftest/content/xul/menuitem-key-ref.xhtml -# these random-if(Android) are due to differences between Android Native & Xul, see bug 732569 -random-if(Android) fuzzy-if(!nativeThemePref,0-2,0-8) == chrome://reftest/content/xul/menulist-shrinkwrap-1.xhtml chrome://reftest/content/xul/menulist-shrinkwrap-1-ref.xhtml -random-if(Android) fuzzy-if(!nativeThemePref,0-2,0-8) == chrome://reftest/content/xul/menulist-shrinkwrap-2.xhtml chrome://reftest/content/xul/menulist-shrinkwrap-2-ref.xhtml # accesskeys are not normally displayed on Mac, so set a pref to enable them pref(ui.key.menuAccessKey,18) == chrome://reftest/content/xul/accesskey.xhtml chrome://reftest/content/xul/accesskey-ref.xhtml fuzzy-if(xulRuntime.widgetToolkit=="gtk",0-1,0-11) fuzzy-if(winWidget&&!nativeThemePref,0-1,0-1) == chrome://reftest/content/xul/tree-row-outline-1.xhtml chrome://reftest/content/xul/tree-row-outline-1-ref.xhtml # win8: bug 1254832 diff --git a/layout/xul/nsMenuFrame.cpp b/layout/xul/nsMenuFrame.cpp index 31b9c0074987..432164c7e947 100644 --- a/layout/xul/nsMenuFrame.cpp +++ b/layout/xul/nsMenuFrame.cpp @@ -646,36 +646,13 @@ void nsMenuFrame::CloseMenu(bool aDeselectMenu) { pm->HidePopup(GetPopup()->GetContent(), false, aDeselectMenu, true, false); } -bool nsMenuFrame::IsSizedToPopup(nsIContent* aContent, bool aRequireAlways) { - MOZ_ASSERT(aContent->IsElement()); - nsAutoString sizedToPopup; - aContent->AsElement()->GetAttr(kNameSpaceID_None, nsGkAtoms::sizetopopup, - sizedToPopup); - bool sizedToPopupSetToPref = - sizedToPopup.EqualsLiteral("pref") || - (sizedToPopup.IsEmpty() && aContent->IsXULElement(nsGkAtoms::menulist)); - return sizedToPopup.EqualsLiteral("always") || - (!aRequireAlways && sizedToPopupSetToPref); -} - -nsSize nsMenuFrame::GetXULMinSize(nsBoxLayoutState& aBoxLayoutState) { - nsSize size = nsBoxFrame::GetXULMinSize(aBoxLayoutState); - DISPLAY_MIN_SIZE(this, size); - - if (IsSizedToPopup(mContent, true)) SizeToPopup(aBoxLayoutState, size); - - return size; -} - NS_IMETHODIMP nsMenuFrame::DoXULLayout(nsBoxLayoutState& aState) { // lay us out nsresult rv = nsBoxFrame::DoXULLayout(aState); - nsMenuPopupFrame* popupFrame = GetPopup(); - if (popupFrame) { - bool sizeToPopup = IsSizedToPopup(mContent, false); - popupFrame->LayoutPopup(aState, this, sizeToPopup); + if (nsMenuPopupFrame* popupFrame = GetPopup()) { + popupFrame->LayoutPopup(aState); } return rv; @@ -1040,61 +1017,6 @@ void nsMenuFrame::AppendFrames(ChildListID aListID, nsFrameList&& aFrameList) { nsBoxFrame::AppendFrames(aListID, std::move(aFrameList)); } -bool nsMenuFrame::SizeToPopup(nsBoxLayoutState& aState, nsSize& aSize) { - if (!IsXULCollapsed()) { - bool widthSet, heightSet; - nsSize tmpSize(-1, 0); - nsIFrame::AddXULPrefSize(this, tmpSize, widthSet, heightSet); - if (!widthSet && GetXULFlex() == 0) { - nsMenuPopupFrame* popupFrame = GetPopup(); - if (!popupFrame) return false; - tmpSize = popupFrame->GetXULPrefSize(aState); - - // Produce a size such that: - // (1) the menu and its popup can be the same width - // (2) there's enough room in the menu for the content and its - // border-padding - // (3) there's enough room in the popup for the content and its - // scrollbar - nsMargin borderPadding; - GetXULBorderAndPadding(borderPadding); - - // if there is a scroll frame, add the desired width of the scrollbar as - // well - nsIScrollableFrame* scrollFrame = popupFrame->GetScrollFrame(popupFrame); - nscoord scrollbarWidth = 0; - if (scrollFrame) { - scrollbarWidth = - scrollFrame->GetDesiredScrollbarSizes(&aState).LeftRight(); - } - - aSize.width = - tmpSize.width + std::max(borderPadding.LeftRight(), scrollbarWidth); - - return true; - } - } - - return false; -} - -nsSize nsMenuFrame::GetXULPrefSize(nsBoxLayoutState& aState) { - nsSize size = nsBoxFrame::GetXULPrefSize(aState); - DISPLAY_PREF_SIZE(this, size); - - // If we are using sizetopopup="always" then - // nsBoxFrame will already have enforced the minimum size - if (!IsSizedToPopup(mContent, true) && IsSizedToPopup(mContent, false) && - SizeToPopup(aState, size)) { - // We now need to ensure that size is within the min - max range. - nsSize minSize = nsBoxFrame::GetXULMinSize(aState); - nsSize maxSize = GetXULMaxSize(aState); - size = XULBoundsCheck(minSize, size, maxSize); - } - - return size; -} - NS_IMETHODIMP nsMenuFrame::GetActiveChild(dom::Element** aResult) { nsMenuPopupFrame* popupFrame = GetPopup(); diff --git a/layout/xul/nsMenuFrame.h b/layout/xul/nsMenuFrame.h index 72df29a7ce11..5fcf14ffeace 100644 --- a/layout/xul/nsMenuFrame.h +++ b/layout/xul/nsMenuFrame.h @@ -86,8 +86,6 @@ class nsMenuFrame final : public nsBoxFrame, public nsIReflowCallback { NS_DECL_FRAMEARENA_HELPERS(nsMenuFrame) NS_IMETHOD DoXULLayout(nsBoxLayoutState& aBoxLayoutState) override; - virtual nsSize GetXULMinSize(nsBoxLayoutState& aBoxLayoutState) override; - virtual nsSize GetXULPrefSize(nsBoxLayoutState& aBoxLayoutState) override; virtual void Init(nsIContent* aContent, nsContainerFrame* aParent, nsIFrame* aPrevInFlow) override; @@ -196,8 +194,6 @@ class nsMenuFrame final : public nsBoxFrame, public nsIReflowCallback { } #endif - static bool IsSizedToPopup(nsIContent* aContent, bool aRequireAlways); - // nsIReflowCallback virtual bool ReflowFinished() override; virtual void ReflowCallbackCanceled() override; @@ -235,12 +231,10 @@ class nsMenuFrame final : public nsBoxFrame, public nsIReflowCallback { void Execute(mozilla::WidgetGUIEvent* aEvent); // This method can destroy the frame - virtual nsresult AttributeChanged(int32_t aNameSpaceID, nsAtom* aAttribute, - int32_t aModType) override; + nsresult AttributeChanged(int32_t aNameSpaceID, nsAtom* aAttribute, + int32_t aModType) override; virtual ~nsMenuFrame() = default; - bool SizeToPopup(nsBoxLayoutState& aState, nsSize& aSize); - bool ShouldBlink(); void StartBlinking(); void StopBlinking(); diff --git a/layout/xul/nsMenuPopupFrame.cpp b/layout/xul/nsMenuPopupFrame.cpp index 76504613587d..d003ecb77dbb 100644 --- a/layout/xul/nsMenuPopupFrame.cpp +++ b/layout/xul/nsMenuPopupFrame.cpp @@ -12,6 +12,7 @@ #include "mozilla/ComputedStyle.h" #include "nsCSSRendering.h" #include "nsNameSpaceManager.h" +#include "nsIFrameInlines.h" #include "nsViewManager.h" #include "nsWidgetsCID.h" #include "nsMenuFrame.h" @@ -152,33 +153,10 @@ bool nsMenuPopupFrame::ShouldCreateWidgetUpfront() const { // always generated right away. return mContent->AsElement()->HasAttr(nsGkAtoms::type); } - // Generate the widget up-front if the following is true: - // - // - If the parent menu is a unless its sizetopopup is set to - // "none". - // - For other elements, if the parent menu has a sizetopopup attribute. - // - // In these cases the size of the parent menu is dependent on the size of the - // popup, so the widget needs to exist in order to calculate this size. - nsIContent* parentContent = mContent->GetParent(); - if (!parentContent) { - return true; - } - if (parentContent->IsXULElement(nsGkAtoms::menulist)) { - Element* parent = parentContent->AsElement(); - nsAutoString sizedToPopup; - if (!parent->GetAttr(nsGkAtoms::sizetopopup, sizedToPopup)) { - // No prop set, generate child frames normally for the default value - // ("pref"). - return true; - } - // Don't generate child frame only if the property is set to none. - return !sizedToPopup.EqualsLiteral("none"); - } - - return parentContent->IsElement() && - parentContent->AsElement()->HasAttr(nsGkAtoms::sizetopopup); + // Generate the widget up-front if the parent menu is a unless its + // sizetopopup is set to "none". + return ShouldExpandToInflowParentOrAnchor(); } void nsMenuPopupFrame::Init(nsIContent* aContent, nsContainerFrame* aParent, @@ -552,7 +530,7 @@ void nsMenuPopupFrame::Reflow(nsPresContext* aPresContext, nsBoxLayoutState state(aPresContext, aReflowInput.mRenderingContext, &aReflowInput, aReflowInput.mReflowDepth); - LayoutPopup(state, nullptr, false); + LayoutPopup(state); const auto wm = GetWritingMode(); LogicalSize boxSize = GetLogicalSize(wm); @@ -562,14 +540,11 @@ void nsMenuPopupFrame::Reflow(nsPresContext* aPresContext, FinishAndStoreOverflow(&aDesiredSize, aReflowInput.mStyleDisplay); } -void nsMenuPopupFrame::LayoutPopup(nsBoxLayoutState& aState, - nsIFrame* aParentMenu, bool aSizedToPopup) { +void nsMenuPopupFrame::LayoutPopup(nsBoxLayoutState& aState) { if (IsNativeMenu()) { return; } - mSizedToPopup = aSizedToPopup; - SchedulePaint(); bool shouldPosition = [&] { @@ -586,11 +561,19 @@ void nsMenuPopupFrame::LayoutPopup(nsBoxLayoutState& aState, bool isOpen = IsOpen(); if (!isOpen) { - // if the popup is not open, only do layout while showing or if the menu - // is sized to the popup shouldPosition = - (mPopupState == ePopupShowing || mPopupState == ePopupPositioning); - if (!shouldPosition && !aSizedToPopup) { + mPopupState == ePopupShowing || mPopupState == ePopupPositioning; + + // If the popup is not open, only do layout while showing or if we're a + // menulist. + // + // This is needed because the SelectParent code wants to limit the height of + // the popup before opening it. + // + // TODO(emilio): We should consider adding a way to do that more reliably + // instead, but this preserves existing behavior. + const bool needsLayout = shouldPosition || IsMenuList(); + if (!needsLayout) { RemoveStateBits(NS_FRAME_FIRST_REFLOW); return; } @@ -616,9 +599,20 @@ void nsMenuPopupFrame::LayoutPopup(nsBoxLayoutState& aState, nsSize prefSize = GetXULPrefSize(aState); nsSize minSize = GetXULMinSize(aState); nsSize maxSize = GetXULMaxSize(aState); - - if (aSizedToPopup) { - prefSize.width = aParentMenu->GetRect().width; + if (ShouldExpandToInflowParentOrAnchor()) { + nscoord menuListOrAnchorWidth = 0; + if (nsIFrame* menuList = GetInFlowParent()) { + menuListOrAnchorWidth = menuList->GetRect().width; + } + if (mAnchorType == MenuPopupAnchorType_Rect) { + menuListOrAnchorWidth = + std::max(menuListOrAnchorWidth, mScreenRect.width); + } + // Input margin doesn't have contents, so account for it for popup sizing + // purposes. + menuListOrAnchorWidth += + 2 * StyleUIReset()->mMozWindowInputRegionMargin.ToAppUnits(); + prefSize.width = std::max(prefSize.width, menuListOrAnchorWidth); } prefSize = XULBoundsCheck(minSize, prefSize, maxSize); @@ -636,7 +630,7 @@ void nsMenuPopupFrame::LayoutPopup(nsBoxLayoutState& aState, bool needCallback = false; if (shouldPosition) { - SetPopupPosition(aParentMenu, false, aSizedToPopup); + SetPopupPosition(false); needCallback = true; } @@ -648,21 +642,19 @@ void nsMenuPopupFrame::LayoutPopup(nsBoxLayoutState& aState, // real height for its inline element, but does once it is laid out. // This is bug 228673 which doesn't have a simple fix. bool rePosition = shouldPosition && (mPosition == POPUPPOSITION_SELECTION); - if (!aParentMenu) { - nsSize newsize = GetSize(); - if (newsize.width > bounds.width || newsize.height > bounds.height) { - // the size after layout was larger than the preferred size, - // so set the preferred size accordingly - mPrefSize = newsize; - if (isOpen) { - rePosition = true; - needCallback = true; - } + nsSize newsize = GetSize(); + if (newsize.width > bounds.width || newsize.height > bounds.height) { + // the size after layout was larger than the preferred size, + // so set the preferred size accordingly + mPrefSize = newsize; + if (isOpen) { + rePosition = true; + needCallback = true; } } if (rePosition) { - SetPopupPosition(aParentMenu, false, aSizedToPopup); + SetPopupPosition(false); } nsPresContext* pc = PresContext(); @@ -722,29 +714,35 @@ void nsMenuPopupFrame::LayoutPopup(nsBoxLayoutState& aState, if (needCallback && !mReflowCallbackData.mPosted) { pc->PresShell()->PostReflowCallback(this); - mReflowCallbackData.MarkPosted(aParentMenu, openChanged); + mReflowCallbackData.MarkPosted(openChanged); } } bool nsMenuPopupFrame::ReflowFinished() { - SetPopupPosition(mReflowCallbackData.mAnchor, false, mSizedToPopup); + SetPopupPosition(false); mReflowCallbackData.Clear(); return false; } void nsMenuPopupFrame::ReflowCallbackCanceled() { mReflowCallbackData.Clear(); } -bool nsMenuPopupFrame::IsMenuList() { - nsIFrame* parentMenu = GetParent(); - return (parentMenu && parentMenu->GetContent() && - parentMenu->GetContent()->IsXULElement(nsGkAtoms::menulist)); +bool nsMenuPopupFrame::IsMenuList() const { + return mContent->GetParent() && + mContent->GetParent()->IsXULElement(nsGkAtoms::menulist); +} + +bool nsMenuPopupFrame::ShouldExpandToInflowParentOrAnchor() const { + return IsMenuList() && !mContent->GetParent()->AsElement()->AttrValueIs( + kNameSpaceID_None, nsGkAtoms::sizetopopup, + nsGkAtoms::none, eCaseMatters); } nsIContent* nsMenuPopupFrame::GetTriggerContent( nsMenuPopupFrame* aMenuPopupFrame) { while (aMenuPopupFrame) { - if (aMenuPopupFrame->mTriggerContent) + if (aMenuPopupFrame->mTriggerContent) { return aMenuPopupFrame->mTriggerContent; + } // check up the menu hierarchy until a popup with a trigger node is found nsMenuFrame* menuFrame = do_QueryFrame(aMenuPopupFrame->GetParent()); @@ -1437,8 +1435,7 @@ static nsIFrame* MaybeDelegatedAnchorFrame(nsIFrame* aFrame) { return aFrame; } -nsresult nsMenuPopupFrame::SetPopupPosition(nsIFrame* aAnchorFrame, - bool aIsMove, bool aSizedToPopup) { +nsresult nsMenuPopupFrame::SetPopupPosition(bool aIsMove) { // If this is due to a move, return early if the popup hasn't been laid out // yet. On Windows, this can happen when using a drag popup before it opens. if (aIsMove && (mPrefSize.width == -1 || mPrefSize.height == -1)) { @@ -1456,7 +1453,7 @@ nsresult nsMenuPopupFrame::SetPopupPosition(nsIFrame* aAnchorFrame, nsRect anchorRect; bool anchored = IsAnchored(); - if (anchored || aSizedToPopup) { + if (anchored) { // In order to deal with transforms, we need the root prescontext: nsPresContext* rootPresContext = presContext->GetRootPresContext(); @@ -1474,19 +1471,15 @@ nsresult nsMenuPopupFrame::SetPopupPosition(nsIFrame* aAnchorFrame, // If that wasn't specified either, use the root frame. Note that // mAnchorContent might be a different document so its presshell must be // used. - if (aAnchorFrame) { - aAnchorFrame = MaybeDelegatedAnchorFrame(aAnchorFrame); - } else { - aAnchorFrame = GetAnchorFrame(); - if (!aAnchorFrame) { - aAnchorFrame = rootFrame; - if (!aAnchorFrame) { - return NS_OK; - } + nsIFrame* anchorFrame = GetAnchorFrame(); + if (!anchorFrame) { + anchorFrame = rootFrame; + if (!anchorFrame) { + return NS_OK; } } - anchorRect = ComputeAnchorRect(rootPresContext, aAnchorFrame); + anchorRect = ComputeAnchorRect(rootPresContext, anchorFrame); } } @@ -1497,23 +1490,7 @@ nsresult nsMenuPopupFrame::SetPopupPosition(nsIFrame* aAnchorFrame, { NS_ASSERTION(mPrefSize.width >= 0 || mPrefSize.height >= 0, "preferred size of popup not set"); - nsSize newSize = mPrefSize; - if (aSizedToPopup) { - // Input margin doesn't have contents, so account for it for popup sizing - // purposes. - const nscoord inputMargin = - StyleUIReset()->mMozWindowInputRegionMargin.ToAppUnits(); - newSize.width = anchorRect.width + 2 * inputMargin; - // If we're anchoring to a rect, and the rect is smaller than the - // preferred size of the popup, change its width accordingly. - if (mAnchorType == MenuPopupAnchorType_Rect) { - newSize.width = std::max(newSize.width, mPrefSize.width); - } - - // Pref size is already constrained by LayoutPopup(). - ConstrainSizeForWayland(newSize); - } - mRect.SizeTo(newSize); + mRect.SizeTo(mPrefSize); } // the screen position in app units where the popup should appear @@ -1740,12 +1717,6 @@ nsresult nsMenuPopupFrame::SetPopupPosition(nsIFrame* aAnchorFrame, // Now that we've positioned the view, sync up the frame's origin. nsBoxFrame::SetPosition(viewPoint - GetParent()->GetOffsetTo(rootFrame)); - if (aSizedToPopup) { - nsBoxLayoutState state(PresContext()); - // XXXndeakin can parentSize.width still extend outside? - SetXULBounds(state, mRect); - } - // If the popup is in the positioned state or if it is shown and the position // or size changed, dispatch a popuppositioned event if the popup wants it. nsIntRect newRect(screenPoint.x, screenPoint.y, mRect.width, mRect.height); @@ -2448,7 +2419,7 @@ void nsMenuPopupFrame::MoveTo(const CSSPoint& aPos, bool aUpdateAttrs, mAnchorType = MenuPopupAnchorType_Point; } - SetPopupPosition(nullptr, true, mSizedToPopup); + SetPopupPosition(true); RefPtr popup = mContent->AsElement(); if (aUpdateAttrs && @@ -2473,7 +2444,7 @@ void nsMenuPopupFrame::MoveToAnchor(nsIContent* aAnchorContent, mPopupState = oldstate; // Pass false here so that flipping and adjusting to fit on the screen happen. - SetPopupPosition(nullptr, false, false); + SetPopupPosition(false); } int8_t nsMenuPopupFrame::GetAlignmentPosition() const { @@ -2664,7 +2635,7 @@ void nsMenuPopupFrame::CheckForAnchorChange(nsRect& aRect) { // If the rectangles are different, move the popup. if (!anchorRect.IsEqualEdges(aRect)) { aRect = anchorRect; - SetPopupPosition(nullptr, true, false); + SetPopupPosition(true); } } diff --git a/layout/xul/nsMenuPopupFrame.h b/layout/xul/nsMenuPopupFrame.h index dd94609fdd69..9c4ee513e8f4 100644 --- a/layout/xul/nsMenuPopupFrame.h +++ b/layout/xul/nsMenuPopupFrame.h @@ -200,6 +200,9 @@ class nsMenuPopupFrame final : public nsBoxFrame, // Whether we should create a widget on Init(). bool ShouldCreateWidgetUpfront() const; + // Whether we should expand the menu to take the size of the parent menulist. + bool ShouldExpandToInflowParentOrAnchor() const; + // Returns true if the popup is a panel with the noautohide attribute set to // true. These panels do not roll up automatically. bool IsNoAutoHide() const; @@ -218,16 +221,13 @@ class nsMenuPopupFrame final : public nsBoxFrame, // layout, position and display the popup as needed MOZ_CAN_RUN_SCRIPT_BOUNDARY - void LayoutPopup(nsBoxLayoutState& aState, nsIFrame* aParentMenu, - bool aSizedToPopup); + void LayoutPopup(nsBoxLayoutState& aState); - // Set the position of the popup either relative to the anchor aAnchorFrame - // (or the frame for mAnchorContent if aAnchorFrame is null), anchored at a + // Set the position of the popup relative to the anchor content, anchored at a // rectangle, or at a specific point if a screen position is set. The popup // will be adjusted so that it is on screen. If aIsMove is true, then the // popup is being moved, and should not be flipped. - nsresult SetPopupPosition(nsIFrame* aAnchorFrame, bool aIsMove, - bool aSizedToPopup); + nsresult SetPopupPosition(bool aIsMove); // called when the Enter key is pressed while the popup is open. This will // just pass the call down to the current menu, if any. If a current menu @@ -254,7 +254,7 @@ class nsMenuPopupFrame final : public nsBoxFrame, bool IsMouseTransparent() const; // Return true if the popup is for a menulist. - bool IsMenuList(); + bool IsMenuList() const; bool IsDragSource() const { return mIsDragSource; } void SetIsDragSource(bool aIsDragSource) { mIsDragSource = aIsDragSource; } @@ -571,10 +571,6 @@ class nsMenuPopupFrame final : public nsBoxFrame, // we stop updating the anchor so that we can end up with a stable position. bool mPositionedByMoveToRect = false; - // Store SizedToPopup attribute for MoveTo call to avoid - // unwanted popup resize there. - bool mSizedToPopup = false; - // If the panel prefers to "slide" rather than resize, then the arrow gets // positioned at this offset (along either the x or y axis, depending on // mPosition) @@ -596,21 +592,17 @@ class nsMenuPopupFrame final : public nsBoxFrame, FlipType mFlip; // Whether to flip struct ReflowCallbackData { - ReflowCallbackData() - : mPosted(false), mAnchor(nullptr), mIsOpenChanged(false) {} - void MarkPosted(nsIFrame* aAnchor, bool aIsOpenChanged) { + ReflowCallbackData() = default; + void MarkPosted(bool aIsOpenChanged) { mPosted = true; - mAnchor = aAnchor; mIsOpenChanged = aIsOpenChanged; } void Clear() { mPosted = false; - mAnchor = nullptr; mIsOpenChanged = false; } - bool mPosted; - nsIFrame* mAnchor; - bool mIsOpenChanged; + bool mPosted = false; + bool mIsOpenChanged = false; }; ReflowCallbackData mReflowCallbackData; diff --git a/layout/xul/nsXULPopupManager.cpp b/layout/xul/nsXULPopupManager.cpp index cbc2ad8e37cb..c134858dcb07 100644 --- a/layout/xul/nsXULPopupManager.cpp +++ b/layout/xul/nsXULPopupManager.cpp @@ -547,7 +547,7 @@ void nsXULPopupManager::AdjustPopupsOnWindowChange( } for (int32_t l = list.Length() - 1; l >= 0; l--) { - list[l]->SetPopupPosition(nullptr, true, false); + list[l]->SetPopupPosition(true); } } @@ -600,7 +600,7 @@ void nsXULPopupManager::PopupMoved(nsIFrame* aFrame, nsIntPoint aPnt, // the specified screen coordinates. if (menuPopupFrame->IsAnchored() && menuPopupFrame->PopupLevel() == ePopupLevelParent) { - menuPopupFrame->SetPopupPosition(nullptr, true, false); + menuPopupFrame->SetPopupPosition(true); } else { CSSPoint cssPos = LayoutDeviceIntPoint::FromUnknownPoint(aPnt) / menuPopupFrame->PresContext()->CSSToDevPixelScale(); @@ -676,11 +676,11 @@ nsMenuChainItem* nsXULPopupManager::GetTopVisibleMenu() { void nsXULPopupManager::SetActiveMenuBar(nsMenuBarFrame* aMenuBar, bool aActivate) { - if (aActivate) + if (aActivate) { mActiveMenuBar = aMenuBar; - else if (mActiveMenuBar == aMenuBar) + } else if (mActiveMenuBar == aMenuBar) { mActiveMenuBar = nullptr; - + } UpdateKeyboardListeners(); } @@ -903,7 +903,7 @@ void nsXULPopupManager::OnNativeMenuClosed() { RefPtr kungFuDeathGrip(this); bool shouldHideChain = - (mNativeMenuActivatedItemCloseMenuMode == Some(CloseMenuMode_Auto)); + mNativeMenuActivatedItemCloseMenuMode == Some(CloseMenuMode_Auto); nsCOMPtr popup = mNativeMenu->Element(); nsMenuPopupFrame* popupFrame = GetPopupFrameForContent(popup, true); diff --git a/toolkit/content/tests/chrome/test_menulist.xhtml b/toolkit/content/tests/chrome/test_menulist.xhtml index 16472c72039a..9e959c1cc5d1 100644 --- a/toolkit/content/tests/chrome/test_menulist.xhtml +++ b/toolkit/content/tests/chrome/test_menulist.xhtml @@ -110,10 +110,6 @@ function testtag_menulist_UI_finish(element) test_nsIDOMXULSelectControlElement(element, "menuitem", null); - // bug 566154, the menulist width should account for vertical scrollbar - ok(document.getElementById("menulist-size").getBoundingClientRect().width >= 210, - "menulist popup width includes scrollbar width"); - $("menulist").open = true; } diff --git a/toolkit/content/tests/chrome/test_menulist_paging.xhtml b/toolkit/content/tests/chrome/test_menulist_paging.xhtml index 8aded9937e45..2b3d53964446 100644 --- a/toolkit/content/tests/chrome/test_menulist_paging.xhtml +++ b/toolkit/content/tests/chrome/test_menulist_paging.xhtml @@ -3,7 +3,7 @@ @@ -93,8 +93,13 @@ let tests = [ { list: "menulist4", initial: 5, scroll: 2, downs: [], ups: [] } ]; -function startTest() -{ +let gMeasured = false; +function measureMenuItemHeightIfNeeded() { + if (gMeasured) { + return; + } + gMeasured = true; + let popup = document.getElementById("menulist-popup1"); let menuitemHeight = popup.firstChild.getBoundingClientRect().height; @@ -115,23 +120,21 @@ function startTest() document.getElementById("menulist-popup2").style.height = height + "px"; document.getElementById("menulist-popup3").style.height = height + "px"; document.getElementById("menulist-popup4").style.height = height + "px"; - - runTest(); } -function runTest() -{ +function runTest() { if (!tests.length) { SimpleTest.finish(); return; } - test = tests.shift(); document.getElementById(test.list).open = true; } function menulistShown() { + measureMenuItemHeightIfNeeded(); + let menulist = document.getElementById(test.list); is(menulist.activeChild.label, menulist.getItemAtIndex(test.initial).label, test.list + " initial selection"); diff --git a/toolkit/content/tests/chrome/test_menulist_position.xhtml b/toolkit/content/tests/chrome/test_menulist_position.xhtml index cddf48e657df..ec44b1e19fc8 100644 --- a/toolkit/content/tests/chrome/test_menulist_position.xhtml +++ b/toolkit/content/tests/chrome/test_menulist_position.xhtml @@ -94,7 +94,7 @@ function popupHidden() - +