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 <select> instead, which does have that
behavior.

Simplify the setup to make sizetopopup only apply to menulists (we don't
have non-menulist usage anyways), and only work in the "popup depends on
the menulist" direction, not the other way around.

This will allow making the popup a regular out-of-flow element. The
change to test_menulist_paging is not needed (I restored the behavior of
eagerly layout menulists, to fix the <select> 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

@@ -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);
   }
 }
 

layout/reftests/xul/menulist-shrinkwrap-1-ref.xhtml

@@ -1,10 +0,0 @@
-<?xml version="1.0"?>
-<?xml-stylesheet href="chrome://global/skin/global.css" type="text/css"?>
-<window xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul" align="start">
-  <menulist style="color: transparent">
-    <menupopup>
-      <menuitem value="1" label="short" />
-      <menuitem value="2" label="long item" />
-    </menupopup>
-  </menulist>
-</window>

layout/reftests/xul/menulist-shrinkwrap-1.xhtml

@@ -1,10 +0,0 @@
-<?xml version="1.0"?>
-<?xml-stylesheet href="chrome://global/skin/global.css" type="text/css"?>
-<window xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul" align="start">
-  <menulist style="color: transparent">
-    <menupopup>
-      <menuitem value="1" label="long item" />
-      <menuitem value="2" label="short" />
-    </menupopup>
-  </menulist>
-</window>

layout/reftests/xul/menulist-shrinkwrap-2-ref.xhtml

@@ -1,25 +0,0 @@
-<?xml version="1.0"?>
-<?xml-stylesheet href="chrome://global/skin/global.css" type="text/css"?>
-<window xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul">
-  <style xmlns="http://www.w3.org/1999/xhtml">
-    <![CDATA[
-      menulist, menuitem, label { font: Menu }
-      #measurelabel { color: transparent }
-    ]]>
-  </style>
-  <stack>
-    <vbox>
-      <menulist native="true">
-        <menupopup>
-          <menuitem value="1" label="long item" />
-          <menuitem value="2" label="short" />
-        </menupopup>
-      </menulist>
-    </vbox>
-    <hbox>
-      <label id="measurelabel" value="long item" />
-      <!-- cover up the right edge of the above -->
-      <box flex="1" style="background: white" />
-    </hbox>
-  </stack>
-</window>

layout/reftests/xul/menulist-shrinkwrap-2.xhtml

@@ -1,25 +0,0 @@
-<?xml version="1.0"?>
-<?xml-stylesheet href="chrome://global/skin/global.css" type="text/css"?>
-<window xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul">
-  <style xmlns="http://www.w3.org/1999/xhtml">
-    <![CDATA[
-      menulist, menuitem, label { font: Menu }
-      #measurelabel { color: transparent }
-    ]]>
-  </style>
-  <stack>
-    <vbox align="start">
-      <menulist native="true">
-        <menupopup>
-          <menuitem value="1" label="long item" />
-          <menuitem value="2" label="short" />
-        </menupopup>
-      </menulist>
-    </vbox>
-    <hbox>
-      <label id="measurelabel" value="long item" />
-      <!-- cover up the right edge of the above -->
-      <box flex="1" style="background: white" />
-    </hbox>
-  </stack>
-</window>

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

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 (nsMenuPopupFrame* popupFrame = GetPopup()) {
-  if (popupFrame) {
+    popupFrame->LayoutPopup(aState);
-    bool sizeToPopup = IsSizedToPopup(mContent, false);
-    popupFrame->LayoutPopup(aState, this, sizeToPopup);
   }
 
   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();

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,
+  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();

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 <menulist> 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)) {
+  // Generate the widget up-front if the parent menu is a <menulist> unless its
-    Element* parent = parentContent->AsElement();
+  // sizetopopup is set to "none".
-    nsAutoString sizedToPopup;
+  return ShouldExpandToInflowParentOrAnchor();
-    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);
 }
 
 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,
+void nsMenuPopupFrame::LayoutPopup(nsBoxLayoutState& aState) {
-                                   nsIFrame* aParentMenu, bool aSizedToPopup) {
   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);
+        mPopupState == ePopupShowing || mPopupState == ePopupPositioning;
-    if (!shouldPosition && !aSizedToPopup) {
+
+    // 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 (ShouldExpandToInflowParentOrAnchor()) {
-  if (aSizedToPopup) {
+    nscoord menuListOrAnchorWidth = 0;
-    prefSize.width = aParentMenu->GetRect().width;
+    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,7 +642,6 @@ 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,
@@ -659,10 +652,9 @@ void nsMenuPopupFrame::LayoutPopup(nsBoxLayoutState& aState,
       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() {
+bool nsMenuPopupFrame::IsMenuList() const {
-  nsIFrame* parentMenu = GetParent();
+  return mContent->GetParent() &&
-  return (parentMenu && parentMenu->GetContent() &&
+         mContent->GetParent()->IsXULElement(nsGkAtoms::menulist);
-          parentMenu->GetContent()->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,
+nsresult nsMenuPopupFrame::SetPopupPosition(bool aIsMove) {
-                                            bool aIsMove, bool aSizedToPopup) {
   // 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) {
+      nsIFrame* anchorFrame = GetAnchorFrame();
-        aAnchorFrame = MaybeDelegatedAnchorFrame(aAnchorFrame);
+      if (!anchorFrame) {
-      } else {
+        anchorFrame = rootFrame;
-        aAnchorFrame = GetAnchorFrame();
+        if (!anchorFrame) {
-        if (!aAnchorFrame) {
-          aAnchorFrame = rootFrame;
-          if (!aAnchorFrame) {
           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;
+    mRect.SizeTo(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);
   }
 
   // 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<Element> 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);
   }
 }
 

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,
+  void LayoutPopup(nsBoxLayoutState& aState);
-                   bool aSizedToPopup);
 
-  // Set the position of the popup either relative to the anchor aAnchorFrame
+  // Set the position of the popup relative to the anchor content, anchored at a
-  // (or the frame for mAnchorContent if aAnchorFrame is null), 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,
+  nsresult SetPopupPosition(bool aIsMove);
-                            bool aSizedToPopup);
 
   // 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()
+    ReflowCallbackData() = default;
-        : mPosted(false), mAnchor(nullptr), mIsOpenChanged(false) {}
+    void MarkPosted(bool aIsOpenChanged) {
-    void MarkPosted(nsIFrame* aAnchor, bool aIsOpenChanged) {
       mPosted = true;
-      mAnchor = aAnchor;
       mIsOpenChanged = aIsOpenChanged;
     }
     void Clear() {
       mPosted = false;
-      mAnchor = nullptr;
       mIsOpenChanged = false;
     }
-    bool mPosted;
+    bool mPosted = false;
-    nsIFrame* mAnchor;
+    bool mIsOpenChanged = false;
-    bool mIsOpenChanged;
   };
   ReflowCallbackData mReflowCallbackData;
 

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<nsXULPopupManager> kungFuDeathGrip(this);
 
   bool shouldHideChain =
-      (mNativeMenuActivatedItemCloseMenuMode == Some(CloseMenuMode_Auto));
+      mNativeMenuActivatedItemCloseMenuMode == Some(CloseMenuMode_Auto);
 
   nsCOMPtr<nsIContent> popup = mNativeMenu->Element();
   nsMenuPopupFrame* popupFrame = GetPopupFrameForContent(popup, true);

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;
 }
 

toolkit/content/tests/chrome/test_menulist_paging.xhtml

@@ -3,7 +3,7 @@
 <?xml-stylesheet href="chrome://mochikit/content/tests/SimpleTest/test.css" type="text/css"?>
 
 <window title="Menulist Tests"
-        onload="setTimeout(startTest, 0);"
+        onload="setTimeout(runTest, 0);"
         onpopupshown="menulistShown()" onpopuphidden="runTest()"
         xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul">
 
@@ -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");
 

toolkit/content/tests/chrome/test_menulist_position.xhtml

@@ -94,7 +94,7 @@ function popupHidden()
 </script>
 
 <hbox align="center" pack="center" style="margin-top: 140px;">
-  <menulist id="menulist" onpopupshown="popupShown();" onpopuphidden="popupHidden();" native="true">
+  <menulist style="width: 200px" id="menulist" onpopupshown="popupShown();" onpopuphidden="popupHidden();" native="true">
     <menupopup style="max-height: 90px;">
       <menuitem label="One"/>
       <menuitem label="Two"/>