Backed out changeset f11c529b2407 (bug 1805414) for failures on test_submenuClose.xhtml and nsMenuPopupFrame.cpp. CLOSED TREE

accessible/base/FocusManager.cpp

@@ -201,9 +201,7 @@ void FocusManager::ActiveItemChanged(LocalAccessible* aItem,
 #endif
 
   // Nothing changed, happens for XUL trees and HTML selects.
-  if (aItem && aItem == mActiveItem) {
-    return;
-  }
+  if (aItem && aItem == mActiveItem) return;
 
   mActiveItem = nullptr;
 

accessible/base/nsAccessibilityService.cpp

@@ -513,7 +513,7 @@ void nsAccessibilityService::ContentRangeInserted(PresShell* aPresShell,
 #ifdef A11Y_LOG
   if (logging::IsEnabled(logging::eTree)) {
     logging::MsgBegin("TREE", "content inserted; doc: %p", document);
-    logging::Node("container", aStartChild->GetParentNode());
+    logging::Node("container", aStartChild->GetParent());
     for (nsIContent* child = aStartChild; child != aEndChild;
          child = child->GetNextSibling()) {
       logging::Node("content", child);

accessible/generic/RootAccessible.cpp

@@ -7,7 +7,6 @@
 
 #include "mozilla/ArrayUtils.h"
 #include "mozilla/PresShell.h"  // for nsAccUtils::GetDocAccessibleFor()
-#include "nsXULPopupManager.h"
 
 #define CreateEvent CreateEventA
 
@@ -391,31 +390,6 @@ void RootAccessible::ProcessDOMEvent(Event* aDOMEvent, nsINode* aTarget) {
       nsEventShell::FireEvent(nsIAccessibleEvent::EVENT_MENUPOPUP_END,
                               accessible);
     }
-    if (auto* focus = FocusMgr()->FocusedLocalAccessible()) {
-      // Intentionally use the content tree, because Linux strips menupopups
-      // from the a11y tree so accessible might be an arbitrary ancestor.
-      if (focus->GetContent() &&
-          focus->GetContent()->IsShadowIncludingInclusiveDescendantOf(
-              aTarget)) {
-        // Move the focus to the topmost menu active content if any. The
-        // menu item in the parent menu will not fire a DOMMenuItemActive
-        // event if it's already active.
-        LocalAccessible* newActiveAccessible = nullptr;
-        if (auto* pm = nsXULPopupManager::GetInstance()) {
-          if (auto* content = pm->GetTopActiveMenuItemContent()) {
-            newActiveAccessible =
-                accessible->Document()->GetAccessible(content);
-          }
-        }
-        FocusMgr()->ActiveItemChanged(newActiveAccessible);
-#ifdef A11Y_LOG
-        if (logging::IsEnabled(logging::eFocus)) {
-          logging::ActiveItemChangeCausedBy("DOMMenuInactive",
-                                            newActiveAccessible);
-        }
-#endif
-      }
-    }
   } else if (eventType.EqualsLiteral("DOMMenuItemActive")) {
     RefPtr<AccEvent> event =
         new AccStateChangeEvent(accessible, states::ACTIVE, true);
@@ -572,9 +546,7 @@ void RootAccessible::HandlePopupShownEvent(LocalAccessible* aAccessible) {
 
 void RootAccessible::HandlePopupHidingEvent(nsINode* aPopupNode) {
   DocAccessible* document = nsAccUtils::GetDocAccessibleFor(aPopupNode);
-  if (!document) {
-    return;
-  }
+  if (!document) return;
 
   if (aPopupNode->IsAnyOfXULElements(nsGkAtoms::tooltip, nsGkAtoms::panel)) {
     document->ContentRemoved(aPopupNode->AsContent());
@@ -588,9 +560,7 @@ void RootAccessible::HandlePopupHidingEvent(nsINode* aPopupNode) {
   if (!popup) {
     LocalAccessible* popupContainer =
         document->GetContainerAccessible(aPopupNode);
-    if (!popupContainer) {
-      return;
-    }
+    if (!popupContainer) return;
 
     uint32_t childCount = popupContainer->ChildCount();
     for (uint32_t idx = 0; idx < childCount; idx++) {
@@ -603,14 +573,19 @@ void RootAccessible::HandlePopupHidingEvent(nsINode* aPopupNode) {
 
     // No popup no events. Focus is managed by DOM. This is a case for
     // menupopups of menus on Linux since there are no accessible for popups.
-    if (!popup) {
-      return;
-    }
+    if (!popup) return;
   }
 
   // In case of autocompletes and comboboxes fire state change event for
   // expanded state. Note, HTML form autocomplete isn't a subject of state
   // change event because they aren't autocompletes strictly speaking.
+  // When popup closes (except nested popups and menus) then fire focus event to
+  // where it was. The focus event is expected even if popup didn't take a
+  // focus.
+
+  static const uint32_t kNotifyOfFocus = 1;
+  static const uint32_t kNotifyOfState = 2;
+  uint32_t notifyOf = 0;
 
   // HTML select is target of popuphidding event. Otherwise get container
   // widget. No container widget means this is either tooltip or menupopup.
@@ -621,15 +596,41 @@ void RootAccessible::HandlePopupHidingEvent(nsINode* aPopupNode) {
   } else {
     widget = popup->ContainerWidget();
     if (!widget) {
-      if (!popup->IsMenuPopup()) {
-        return;
-      }
+      if (!popup->IsMenuPopup()) return;
+
       widget = popup;
     }
   }
 
-  // Fire expanded state change event.
   if (widget->IsCombobox()) {
+    // Fire focus for active combobox, otherwise the focus is managed by DOM
+    // focus notifications. Always fire state change event.
+    if (widget->IsActiveWidget()) notifyOf = kNotifyOfFocus;
+    notifyOf |= kNotifyOfState;
+  } else if (widget->IsMenuButton()) {
+    // Autocomplete (like searchbar) can be inactive when popup hiddens
+    notifyOf |= kNotifyOfFocus;
+  } else if (widget == popup) {
+    // Top level context menus and alerts.
+    // Ignore submenus and menubar. When submenu is closed then sumbenu
+    // container menuitem takes a focus via DOMMenuItemActive notification.
+    // For menubars processing we listen DOMMenubarActive/Inactive
+    // notifications.
+    notifyOf = kNotifyOfFocus;
+  }
+
+  // Restore focus to where it was.
+  if (notifyOf & kNotifyOfFocus) {
+    FocusMgr()->ActiveItemChanged(nullptr);
+#ifdef A11Y_LOG
+    if (logging::IsEnabled(logging::eFocus)) {
+      logging::ActiveItemChangeCausedBy("popuphiding", popup);
+    }
+#endif
+  }
+
+  // Fire expanded state change event.
+  if (notifyOf & kNotifyOfState) {
     RefPtr<AccEvent> event =
         new AccStateChangeEvent(widget, states::EXPANDED, false);
     document->FireDelayedEvent(event);

accessible/tests/mochitest/actions/test_general.xhtml

@@ -28,7 +28,11 @@
     //gA11yEventDumpToConsole = true;
     //enableLogging("tree,verbose"); // debug
 
-    SimpleTest.expectAssertions(0, 1);
+    if (navigator.platform.startsWith("Mac")) {
+      SimpleTest.expectAssertions(0, 1);
+    } else {
+      SimpleTest.expectAssertions(0, 1);
+    }
 
     function doTest()
     {
@@ -37,9 +41,10 @@
           ID: "menu",
           actionName: "click",
           events: CLICK_EVENTS,
-          // Wait for the submenu to show up.
+          // Wait for focus event, it guarantees us the submenu tree is created,
+          // that's necessary for next test.
           eventSeq: [
-            new invokerChecker(EVENT_SHOW, getNode("submenu"))
+            new invokerChecker(EVENT_FOCUS, getNode("menu"))
           ]
         },
         {

accessible/tests/mochitest/events/test_focus_contextmenu.xhtml

@@ -4,7 +4,7 @@
                  type="text/css"?>
 
 <window xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul"
-        title="Context menu focus testing">
+        title="Context nenu focus testing">
 
   <script src="chrome://mochikit/content/tests/SimpleTest/SimpleTest.js" />
   <script src="chrome://mochikit/content/tests/SimpleTest/EventUtils.js"/>
@@ -43,11 +43,8 @@
       gQueue = new eventQueue();
 
       gQueue.push(new synthFocus("button"));
-      gQueue.push(new synthContextMenu("button", [
-        new invokerChecker(EVENT_MENUPOPUP_START, "contextmenu"),
-        new invokerChecker("popupshown", "contextmenu"),
-      ]));
-      gQueue.push(new synthDownKey("button", new focusChecker("item1")));
+      gQueue.push(new synthContextMenu("button",
+                                       new invokerChecker(EVENT_MENUPOPUP_START, "contextmenu")));
       gQueue.push(new synthEscapeKey("contextmenu", new focusChecker("button")));
 
       gQueue.push(new synthContextMenu("button",

accessible/tests/mochitest/events/test_focus_menu.xhtml

@@ -19,7 +19,7 @@
           src="../events.js" />
 
   <script type="application/javascript">
-    // gA11yEventDumpToConsole = true; // debug stuff
+    //gA11yEventDumpToConsole = true; // debug stuff
 
     var gQueue = null;
     function doTests()
@@ -54,8 +54,8 @@
       //gQueue.push(new synthRightKey("apple",
       //                              [ new focusChecker("vehicle"),
       //                                new focusChecker("cycle")]));
-      gQueue.push(new synthMouseMove("vehicle", [ new focusChecker("vehicle"), new invokerChecker("popupshown", "vehiclePopup") ]));
-      gQueue.push(new synthDownKey("vehicle", new focusChecker("cycle")));
+      gQueue.push(new synthClick("vehicle", new focusChecker("vehicle")));
+      gQueue.push(new synthDownKey("cycle", new focusChecker("cycle")));
 
       // open submenu
       gQueue.push(new synthRightKey("cycle", new focusChecker("tricycle")));
@@ -100,7 +100,7 @@
           </menupopup>
         </menu>
         <menu id="vehicle" label="Vehicle">
-          <menupopup id="vehiclePopup">
+          <menupopup>
             <menu id="cycle" label="cycle">
               <menupopup>
                 <menuitem id="tricycle" label="tricycle"/>

accessible/tests/mochitest/events/test_menu.xhtml

@@ -14,12 +14,12 @@
   <script type="application/javascript"
           src="../events.js" />
 
-  <script><![CDATA[
+  <script type="application/javascript">
     function openFileMenu()
     {
       this.eventSeq = [
-        new invokerChecker(EVENT_MENU_START, "menubar"),
-        new invokerChecker("popupshown", "menupopup-file")
+        new invokerChecker(EVENT_MENU_START, getNode("menubar")),
+        new invokerChecker(EVENT_MENUPOPUP_START, getNode("menupopup-file"))
         // new invokerChecker(EVENT_FOCUS, getNode("menuitem-newtab")) intermitent failure
       ];
 
@@ -37,8 +37,8 @@
     function openEditMenu()
     {
       this.eventSeq = [
-        new invokerChecker("popuphidden", "menupopup-file"),
-        new invokerChecker("popupshown", "menupopup-edit"),
+        new invokerChecker(EVENT_MENUPOPUP_END, getNode("menupopup-file")),
+        new invokerChecker(EVENT_MENUPOPUP_START, getNode("menupopup-edit"))
         // new invokerChecker(EVENT_FOCUS, getNode("menuitem-undo")) intermitent failure
       ];
 
@@ -57,7 +57,8 @@
     {
       this.eventSeq = [
         //new invokerChecker(EVENT_FOCUS, document), intermitent failure
-        new invokerChecker("popuphidden", "menupopup-edit"),
+        new invokerChecker(EVENT_MENUPOPUP_END, getNode("menupopup-edit")),
+        new invokerChecker(EVENT_MENU_END, getNode("menubar"))
       ];
 
       this.invoke = function closeEditMenu_invoke()
@@ -67,7 +68,7 @@
 
       this.getID = function closeEditMenu_getID()
       {
-        return "close edit menu";
+        return "close edit menu, leave menubar";
       }
     }
 
@@ -110,7 +111,7 @@
     {
       this.eventSeq = [
         //new invokerChecker(EVENT_FOCUS, document), intermitent failure
-        new invokerChecker(EVENT_MENU_END, "menubar")
+        new invokerChecker(EVENT_MENU_END, getNode("menubar"))
       ];
 
       this.invoke = function leaveMenubar_invoke()
@@ -135,7 +136,7 @@
 
     function doTests()
     {
-      if (!WIN && !LINUX) {
+      if (!WIN) {
         todo(false, "Enable this test on other platforms.");
         SimpleTest.finish();
         return;
@@ -149,7 +150,6 @@
       gQueue.push(new openFileMenu());
       gQueue.push(new openEditMenu());
       gQueue.push(new closeEditMenu());
-      gQueue.push(new leaveMenubar());
 
       // Alt key is used to active menubar and focus menu item on Windows,
       // other platforms requires setting a ui.key.menuAccessKeyFocuses
@@ -165,7 +165,7 @@
 
     SimpleTest.waitForExplicitFinish();
     addA11yLoadEvent(doTests);
-  ]]></script>
+  </script>
 
   <hbox flex="1" style="overflow: auto;">
     <body xmlns="http://www.w3.org/1999/xhtml">

accessible/tests/mochitest/states.js

@@ -130,7 +130,7 @@ function testStates(
       state & STATE_FOCUSABLE,
       STATE_FOCUSABLE,
       false,
-      "Focused " + id + " must be focusable!"
+      "Focussed " + id + " must be focusable!"
     );
   }
 

accessible/tests/mochitest/treeupdate/test_contextmenu.xhtml

@@ -58,7 +58,7 @@
         return "select menuitem " + prettyName(aID);
       }
     }
-
+    
     function openSubMenu(aSubMenuID, aItemID, aMenuID, aTree)
     {
       this.eventSeq = [

accessible/xul/XULMenuAccessible.cpp

@@ -6,9 +6,6 @@
 #include "XULMenuAccessible.h"
 
 #include "LocalAccessible-inl.h"
-#include "XULMenuParentElement.h"
-#include "XULPopupElement.h"
-#include "mozilla/Assertions.h"
 #include "nsAccessibilityService.h"
 #include "nsAccUtils.h"
 #include "DocAccessible.h"
@@ -28,7 +25,6 @@
 #include "mozilla/LookAndFeel.h"
 #include "mozilla/dom/Document.h"
 #include "mozilla/dom/Element.h"
-#include "mozilla/dom/XULButtonElement.h"
 #include "mozilla/dom/KeyboardEventBinding.h"
 
 using namespace mozilla;
@@ -112,11 +108,12 @@ uint64_t XULMenuitemAccessible::NativeState() const {
 uint64_t XULMenuitemAccessible::NativeInteractiveState() const {
   if (NativelyUnavailable()) {
     // Note: keep in sinc with nsXULPopupManager::IsValidMenuItem() logic.
-    auto* button = dom::XULButtonElement::FromNode(GetContent());
     bool skipNavigatingDisabledMenuItem = true;
-    if (!button || !button->IsOnMenuBar()) {
-      skipNavigatingDisabledMenuItem = LookAndFeel::GetInt(
-          LookAndFeel::IntID::SkipNavigatingDisabledMenuItem);
+    nsMenuFrame* menuFrame = do_QueryFrame(GetFrame());
+    if (!menuFrame || !menuFrame->IsOnMenuBar()) {
+      skipNavigatingDisabledMenuItem =
+          LookAndFeel::GetInt(
+              LookAndFeel::IntID::SkipNavigatingDisabledMenuItem, 0) != 0;
     }
 
     if (skipNavigatingDisabledMenuItem) return states::UNAVAILABLE;
@@ -282,11 +279,28 @@ bool XULMenuitemAccessible::AreItemsOperable() const {
 }
 
 LocalAccessible* XULMenuitemAccessible::ContainerWidget() const {
-  if (auto* button = dom::XULButtonElement::FromNode(GetContent())) {
-    if (auto* popup = button->GetMenuParent()) {
-      // We use GetAccessibleOrContainer instead of just GetAccessible because
-      // we strip menupopups from the tree for ATK.
-      return mDoc->GetAccessibleOrContainer(popup);
+  nsMenuFrame* menuFrame = do_QueryFrame(GetFrame());
+  if (menuFrame) {
+    nsMenuParent* menuParent = menuFrame->GetMenuParent();
+    if (menuParent) {
+      nsBoxFrame* frame = nullptr;
+      if (menuParent->IsMenuBar()) {  // menubar menu
+        frame = static_cast<nsMenuBarFrame*>(menuParent);
+      } else if (menuParent->IsMenu()) {  // a menupopup or parent menu item
+        frame = static_cast<nsMenuPopupFrame*>(menuParent);
+      }
+      if (frame) {
+        nsIContent* content = frame->GetContent();
+        if (content) {
+          MOZ_ASSERT(mDoc);
+          // We use GetAccessibleOrContainer instead of just GetAccessible
+          // because we strip menupopups from the tree for ATK.
+          return mDoc->GetAccessibleOrContainer(content);
+        }
+      }
+
+      // otherwise it's different kind of popups (like panel or tooltip), it
+      // shouldn't be a real case.
     }
   }
   return nullptr;
@@ -321,11 +335,8 @@ bool XULMenuSeparatorAccessible::HasPrimaryAction() const { return false; }
 XULMenupopupAccessible::XULMenupopupAccessible(nsIContent* aContent,
                                                DocAccessible* aDoc)
     : XULSelectControlAccessible(aContent, aDoc) {
-  if (nsMenuPopupFrame* menuPopupFrame = do_QueryFrame(GetFrame())) {
-    if (menuPopupFrame->PopupType() == ePopupTypeMenu) {
-      mType = eMenuPopupType;
-    }
-  }
+  nsMenuPopupFrame* menuPopupFrame = do_QueryFrame(GetFrame());
+  if (menuPopupFrame && menuPopupFrame->IsMenu()) mType = eMenuPopupType;
 
   // May be the anonymous <menupopup> inside <menulist> (a combobox)
   auto* parent = mContent->GetParentElement();
@@ -414,34 +425,35 @@ LocalAccessible* XULMenupopupAccessible::ContainerWidget() const {
   DocAccessible* document = Document();
 
   nsMenuPopupFrame* menuPopupFrame = do_QueryFrame(GetFrame());
-  MOZ_ASSERT(menuPopupFrame);
-  if (!menuPopupFrame) {
-    return nullptr;
-  }
-
-  auto* cur = dom::XULPopupElement::FromNode(GetContent());
-  while (cur) {
-    auto* menu = cur->GetContainingMenu();
-    if (!menu) {
-      // <panel> / <tooltip> / etc.
+  while (menuPopupFrame) {
+    LocalAccessible* menuPopup =
+        document->GetAccessible(menuPopupFrame->GetContent());
+    if (!menuPopup) {  // shouldn't be a real case
       return nullptr;
     }
-    dom::XULMenuParentElement* parent = menu->GetMenuParent();
-    if (!parent) {
-      LocalAccessible* menuPopup = document->GetAccessible(cur);
-      MOZ_ASSERT(menuPopup);
-      return menuPopup ? menuPopup->LocalParent() : nullptr;
+
+    nsMenuFrame* menuFrame = do_QueryFrame(menuPopupFrame->GetParent());
+    if (!menuFrame) {  // context menu or popups
+      return nullptr;
     }
 
-    if (parent->IsMenuBar()) {
-      return document->GetAccessible(parent);
+    nsMenuParent* menuParent = menuFrame->GetMenuParent();
+    if (!menuParent) {  // menulist or menubutton
+      return menuPopup->LocalParent();
     }
 
-    cur = dom::XULPopupElement::FromNode(parent);
-    MOZ_ASSERT(cur, "Should be a popup");
+    if (menuParent->IsMenuBar()) {  // menubar menu
+      nsMenuBarFrame* menuBarFrame = static_cast<nsMenuBarFrame*>(menuParent);
+      return document->GetAccessible(menuBarFrame->GetContent());
+    }
+
+    // different kind of popups like panel or tooltip
+    if (!menuParent->IsMenu()) return nullptr;
+
+    menuPopupFrame = static_cast<nsMenuPopupFrame*>(menuParent);
   }
 
-  MOZ_ASSERT_UNREACHABLE("How did we get out of the loop without returning?");
+  MOZ_ASSERT_UNREACHABLE("Shouldn't be a real case.");
   return nullptr;
 }
 
@@ -471,12 +483,15 @@ bool XULMenubarAccessible::IsActiveWidget() const {
 bool XULMenubarAccessible::AreItemsOperable() const { return true; }
 
 LocalAccessible* XULMenubarAccessible::CurrentItem() const {
-  auto* content = dom::XULMenuParentElement::FromNode(GetContent());
-  MOZ_ASSERT(content);
-  if (!content || !content->GetActiveMenuChild()) {
-    return nullptr;
+  nsMenuBarFrame* menuBarFrame = do_QueryFrame(GetFrame());
+  if (menuBarFrame) {
+    nsMenuFrame* menuFrame = menuBarFrame->GetCurrentMenuItem();
+    if (menuFrame) {
+      nsIContent* menuItemNode = menuFrame->GetContent();
+      return mDoc->GetAccessible(menuItemNode);
+    }
   }
-  return mDoc->GetAccessible(content->GetActiveMenuChild());
+  return nullptr;
 }
 
 void XULMenubarAccessible::SetCurrentItem(const LocalAccessible* aItem) {

browser/components/places/tests/browser/browser_stayopenmenu.js

@@ -9,7 +9,6 @@ async function locateBookmarkAndTestCtrlClick(menupopup) {
     node => node.label == "Test1"
   );
   ok(testMenuitem, "Found test bookmark.");
-  ok(BrowserTestUtils.is_visible(testMenuitem), "Should be visible");
   let promiseTabOpened = BrowserTestUtils.waitForNewTab(gBrowser, null);
   EventUtils.synthesizeMouseAtCenter(testMenuitem, { accelKey: true });
   let newTab = await promiseTabOpened;
@@ -28,14 +27,12 @@ async function testContextmenu(menuitem) {
   });
   await promiseEvent;
   let promiseTabOpened = BrowserTestUtils.waitForNewTab(gBrowser, null);
-  let hidden = BrowserTestUtils.waitForEvent(cm, "popuphidden");
   cm.activateItem(doc.getElementById("placesContext_open:newtab"));
-  await hidden;
   let newTab = await promiseTabOpened;
   return newTab;
 }
 
-add_setup(async function() {
+add_task(async function test_setup() {
   // Ensure BMB is available in UI.
   let origBMBlocation = CustomizableUI.getPlacementOfWidget(
     "bookmarks-menu-button"

browser/components/pocket/test/browser_pocket_context_menu_action.js

@@ -32,7 +32,7 @@ add_task(async function() {
     "popupshown",
     true
   );
-  contextMenu.activateItem(contextPocket);
+  contextPocket.click();
   await pocketPanelShown;
   checkElements(true, ["customizationui-widget-panel"]);
 

browser/components/preferences/tests/browser_applications_selection.js

@@ -120,7 +120,8 @@ async function selectStandardOptions(itemToUse) {
     if (typeof item == "function") {
       item = item();
     }
-    popup.activateItem(item);
+    item.click();
+    popup.hidePopup();
     await popupHidden;
     return item;
   }

devtools/client/storage/test/browser_storage_delete.js

@@ -58,7 +58,7 @@ add_task(async function() {
 
     await waitForContextMenu(contextMenu, row[cellToClick], () => {
       info(`Opened context menu in ${treeItemName}, row '${rowName}'`);
-      contextMenu.activateItem(menuDeleteItem);
+      menuDeleteItem.click();
       const truncatedRowName = String(rowName)
         .replace(SEPARATOR_GUID, "-")
         .substr(0, 16);

devtools/client/storage/test/browser_storage_delete_all.js

@@ -84,7 +84,7 @@ add_task(async function() {
     const cell = getRowCells(rowName)[cellToClick];
     await waitForContextMenu(contextMenu, cell, () => {
       info(`Opened context menu in ${storeName}, row '${rowName}'`);
-      contextMenu.activateItem(menuDeleteAllItem);
+      menuDeleteAllItem.click();
     });
 
     await eventWait;

devtools/client/storage/test/browser_storage_delete_tree.js

@@ -79,7 +79,7 @@ add_task(async function() {
     ok(target, `tree item found in ${storeName}`);
     await waitForContextMenu(contextMenu, target, () => {
       info(`Opened tree context menu in ${storeName}`);
-      contextMenu.activateItem(menuDeleteAllItem);
+      menuDeleteAllItem.click();
     });
 
     await eventWait;

devtools/client/storage/test/browser_storage_delete_usercontextid.js

@@ -211,7 +211,7 @@ add_task(async function() {
 
     await waitForContextMenu(contextMenu, row[cellToClick], () => {
       info(`Opened context menu in ${treeItemName}, row '${rowName}'`);
-      contextMenu.activateItem(menuDeleteItem);
+      menuDeleteItem.click();
       const truncatedRowName = String(rowName)
         .replace(SEPARATOR_GUID, "-")
         .substr(0, 16);

devtools/client/webconsole/test/browser/browser_webconsole_network_messages_resend_request.js

@@ -39,7 +39,7 @@ add_task(async function task() {
   ok(openResendRequestMenuItem, "resend network request item is enabled");
 
   // Wait for message containing the resent request url
-  menuPopup.activateItem(openResendRequestMenuItem);
+  openResendRequestMenuItem.click();
   await waitFor(
     () => findMessagesByType(hud, documentUrl, ".network").length === 2
   );

dom/base/Element.cpp

@@ -681,18 +681,25 @@ nsIScrollableFrame* Element::GetScrollFrame(nsIFrame** aFrame,
       return nullptr;
     }
 
-    if (nsIScrollableFrame* scrollFrame = frame->GetScrollTargetFrame()) {
-      MOZ_ASSERT(!OwnerDoc()->IsScrollingElement(this),
-                 "How can we have a scrollframe if we're the "
-                 "scrollingElement for our document?");
-      return scrollFrame;
+    // menu frames implement GetScrollTargetFrame but we don't want
+    // to use it here.  Similar for comboboxes.
+    LayoutFrameType type = frame->Type();
+    if (type != LayoutFrameType::Menu &&
+        type != LayoutFrameType::ComboboxControl) {
+      nsIScrollableFrame* scrollFrame = frame->GetScrollTargetFrame();
+      if (scrollFrame) {
+        MOZ_ASSERT(!OwnerDoc()->IsScrollingElement(this),
+                   "How can we have a scrollframe if we're the "
+                   "scrollingElement for our document?");
+        return scrollFrame;
+      }
     }
   }
 
   Document* doc = OwnerDoc();
   // Note: This IsScrollingElement() call can flush frames, if we're the body of
   // a quirks mode document.
-  bool isScrollingElement = doc->IsScrollingElement(this);
+  bool isScrollingElement = OwnerDoc()->IsScrollingElement(this);
   // Now reget *aStyledFrame if the caller asked for it, because that frame
   // flush can kill it.
   if (aFrame) {

dom/base/nsContentUtils.cpp

@@ -6063,13 +6063,10 @@ bool nsContentUtils::IsInStableOrMetaStableState() {
 
 /* static */
 void nsContentUtils::HidePopupsInDocument(Document* aDocument) {
-  RefPtr<nsXULPopupManager> pm = nsXULPopupManager::GetInstance();
-  if (!pm || !aDocument) {
-    return;
-  }
-  nsCOMPtr<nsIDocShellTreeItem> docShellToHide = aDocument->GetDocShell();
-  if (docShellToHide) {
-    pm->HidePopupsInDocShell(docShellToHide);
+  nsXULPopupManager* pm = nsXULPopupManager::GetInstance();
+  if (pm && aDocument) {
+    nsCOMPtr<nsIDocShellTreeItem> docShellToHide = aDocument->GetDocShell();
+    if (docShellToHide) pm->HidePopupsInDocShell(docShellToHide);
   }
 }
 

dom/base/nsContentUtils.h

@@ -2081,8 +2081,7 @@ class nsContentUtils {
    * Hide any XUL popups associated with aDocument, including any documents
    * displayed in child frames. Does nothing if aDocument is null.
    */
-  MOZ_CAN_RUN_SCRIPT_BOUNDARY static void HidePopupsInDocument(
-      Document* aDocument);
+  static void HidePopupsInDocument(Document* aDocument);
 
   /**
    * Retrieve the current drag session, or null if no drag is currently occuring

dom/chrome-webidl/XULMenuElement.webidl

@@ -9,7 +9,6 @@
 interface XULMenuElement : XULElement {
   [HTMLConstructor] constructor();
 
-  [BinaryName="activeMenuChild"]
   attribute Element? activeChild;
 
   boolean handleKeyPress(KeyboardEvent keyEvent);

dom/events/EventStateManager.cpp

@@ -3128,13 +3128,21 @@ void EventStateManager::DecideGestureEvent(WidgetGestureNotifyEvent* aEvent,
       break;
     }
 
-    if (nsIScrollableFrame* scrollableFrame = do_QueryFrame(current)) {
+    nsIScrollableFrame* scrollableFrame = do_QueryFrame(current);
+    if (scrollableFrame) {
       if (current->IsFrameOfType(nsIFrame::eXULBox)) {
         displayPanFeedback = true;
 
         nsRect scrollRange = scrollableFrame->GetScrollRange();
         bool canScrollHorizontally = scrollRange.width > 0;
 
+        if (targetFrame->IsMenuFrame()) {
+          // menu frames report horizontal scroll when they have submenus
+          // and we don't want that
+          canScrollHorizontally = false;
+          displayPanFeedback = false;
+        }
+
         // Vertical panning has priority over horizontal panning, so
         // when vertical movement is possible we can just finish the loop.
         if (scrollRange.height > 0) {

dom/xul/XULButtonElement.cpp

@@ -5,45 +5,17 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "XULButtonElement.h"
-#include "XULMenuParentElement.h"
-#include "XULPopupElement.h"
-#include "mozilla/Assertions.h"
-#include "mozilla/Attributes.h"
 #include "mozilla/EventDispatcher.h"
 #include "mozilla/EventStateManager.h"
-#include "mozilla/LookAndFeel.h"
 #include "mozilla/TextEvents.h"
-#include "mozilla/TimeStamp.h"
 #include "mozilla/dom/DocumentInlines.h"
 #include "mozilla/dom/MouseEventBinding.h"
-#include "mozilla/dom/NameSpaceConstants.h"
-#include "mozilla/dom/AncestorIterator.h"
-#include "nsGkAtoms.h"
-#include "nsITimer.h"
-#include "nsLayoutUtils.h"
-#include "nsCaseTreatment.h"
 #include "nsChangeHint.h"
-#include "nsMenuBarFrame.h"
-#include "nsMenuPopupFrame.h"
-#include "nsPlaceholderFrame.h"
 #include "nsPresContext.h"
-#include "nsXULPopupManager.h"
 #include "nsIDOMXULButtonElement.h"
-#include "nsISound.h"
 
 namespace mozilla::dom {
 
-XULButtonElement::XULButtonElement(
-    already_AddRefed<mozilla::dom::NodeInfo>&& aNodeInfo)
-    : nsXULElement(std::move(aNodeInfo)),
-      mIsAlwaysMenu(IsAnyOfXULElements(nsGkAtoms::menu, nsGkAtoms::menulist,
-                                       nsGkAtoms::menuitem)) {}
-
-XULButtonElement::~XULButtonElement() {
-  StopBlinking();
-  KillMenuOpenTimer();
-}
-
 nsChangeHint XULButtonElement::GetAttributeChangeHint(const nsAtom* aAttribute,
                                                       int32_t aModType) const {
   if (aAttribute == nsGkAtoms::type &&
@@ -54,460 +26,13 @@ nsChangeHint XULButtonElement::GetAttributeChangeHint(const nsAtom* aAttribute,
   return nsXULElement::GetAttributeChangeHint(aAttribute, aModType);
 }
 
-// This global flag is used to record the timestamp when a menu was opened or
-// closed and is used to ignore the mousemove and mouseup events that would fire
-// on the menu after the mousedown occurred.
-static TimeStamp gMenuJustOpenedOrClosedTime = TimeStamp();
-
-void XULButtonElement::PopupOpened() {
-  if (!IsMenu()) {
-    return;
-  }
-  gMenuJustOpenedOrClosedTime = TimeStamp::Now();
-  SetAttr(kNameSpaceID_None, nsGkAtoms::open, u"true"_ns, true);
-}
-
-void XULButtonElement::PopupClosed(bool aDeselectMenu) {
-  if (!IsMenu()) {
-    return;
-  }
-  nsContentUtils::AddScriptRunner(
-      new nsUnsetAttrRunnable(this, nsGkAtoms::open));
-
-  if (aDeselectMenu) {
-    if (RefPtr<XULMenuParentElement> menu = GetMenuParent()) {
-      if (menu->GetActiveMenuChild() == this) {
-        menu->SetActiveMenuChild(nullptr);
-      }
-    }
-  }
-}
-
-bool XULButtonElement::IsMenuActive() const {
-  if (XULMenuParentElement* menu = GetMenuParent()) {
-    return menu->GetActiveMenuChild() == this;
-  }
-  return false;
-}
-
-void XULButtonElement::HandleEnterKeyPress(WidgetEvent& aEvent) {
-  if (IsDisabled()) {
-#ifdef XP_WIN
-    if (XULPopupElement* popup = GetContainingPopupElement()) {
-      if (nsXULPopupManager* pm = nsXULPopupManager::GetInstance()) {
-        pm->HidePopup(popup, /* aHideChain = */ true,
-                      /* aDeselectMenu = */ true, /* aAsynchronous = */ true,
-                      /* aIsCancel = */ false);
-      }
-    }
-#endif
-    return;
-  }
-  if (IsMenuPopupOpen()) {
-    return;
-  }
-  // The enter key press applies to us.
-  if (IsMenuItem()) {
-    ExecuteMenu(aEvent);
-  } else {
-    OpenMenuPopup(true);
-  }
-}
-
-bool XULButtonElement::IsMenuPopupOpen() {
-  nsMenuPopupFrame* popupFrame = GetMenuPopup(FlushType::None);
-  return popupFrame && popupFrame->IsOpen();
-}
-
-bool XULButtonElement::IsOnMenu() const {
-  if (XULMenuParentElement* menu = GetMenuParent()) {
-    return !menu->IsMenuBar();
-  }
-  return false;
-}
-
-bool XULButtonElement::IsOnMenuList() const {
-  if (XULMenuParentElement* menu = GetMenuParent()) {
-    return menu->GetParent() &&
-           menu->GetParent()->IsXULElement(nsGkAtoms::menulist);
-  }
-  return false;
-}
-
-bool XULButtonElement::IsOnMenuBar() const {
-  if (XULMenuParentElement* menu = GetMenuParent()) {
-    return menu->IsMenuBar();
-  }
-  return false;
-}
-
-nsMenuPopupFrame* XULButtonElement::GetContainingPopupWithoutFlushing() const {
-  if (XULPopupElement* popup = GetContainingPopupElement()) {
-    return do_QueryFrame(popup->GetPrimaryFrame());
-  }
-  return nullptr;
-}
-
-XULPopupElement* XULButtonElement::GetContainingPopupElement() const {
-  return XULPopupElement::FromNodeOrNull(GetMenuParent());
-}
-
-bool XULButtonElement::IsOnContextMenu() const {
-  if (nsMenuPopupFrame* popup = GetContainingPopupWithoutFlushing()) {
-    return popup->IsContextMenu();
-  }
-  return false;
-}
-
-void XULButtonElement::ToggleMenuState() {
-  if (IsMenuPopupOpen()) {
-    CloseMenuPopup(false);
-  } else {
-    OpenMenuPopup(false);
-  }
-}
-
-void XULButtonElement::KillMenuOpenTimer() {
-  if (mMenuOpenTimer) {
-    mMenuOpenTimer->Cancel();
-    mMenuOpenTimer = nullptr;
-  }
-}
-
-void XULButtonElement::OpenMenuPopup(bool aSelectFirstItem) {
-  nsXULPopupManager* pm = nsXULPopupManager::GetInstance();
-  if (!pm) {
-    return;
-  }
-
-  pm->KillMenuTimer();
-  if (!pm->MayShowMenu(this)) {
-    return;
-  }
-
-  if (RefPtr<XULMenuParentElement> parent = GetMenuParent()) {
-    parent->SetActiveMenuChild(this);
-  }
-
-  // Open the menu asynchronously.
-  OwnerDoc()->Dispatch(
-      TaskCategory::Other,
-      NS_NewRunnableFunction(
-          "AsyncOpenMenu", [self = RefPtr{this}, aSelectFirstItem] {
-            if (nsXULPopupManager* pm = nsXULPopupManager::GetInstance()) {
-              pm->ShowMenu(self, aSelectFirstItem);
-            }
-          }));
-}
-
-void XULButtonElement::CloseMenuPopup(bool aDeselectMenu) {
-  gMenuJustOpenedOrClosedTime = TimeStamp::Now();
-  // Close the menu asynchronously
-  nsXULPopupManager* pm = nsXULPopupManager::GetInstance();
-  if (!pm) {
-    return;
-  }
-  if (auto* popup = GetMenuPopupContent()) {
-    pm->HidePopup(popup, false, aDeselectMenu, true, false);
-  }
-}
-
-int32_t XULButtonElement::MenuOpenCloseDelay() const {
-  if (IsOnMenuBar()) {
-    return 0;
-  }
-  return LookAndFeel::GetInt(LookAndFeel::IntID::SubmenuDelay, 300);  // ms
-}
-
-void XULButtonElement::ExecuteMenu(Modifiers aModifiers, int16_t aButton,
-                                   bool aIsTrusted) {
-  MOZ_ASSERT(IsMenu());
-
-  StopBlinking();
-
-  auto menuType = GetMenuType();
-  if (NS_WARN_IF(!menuType)) {
-    return;
-  }
-
-  // Because the command event is firing asynchronously, a flag is needed to
-  // indicate whether user input is being handled. This ensures that a popup
-  // window won't get blocked.
-  const bool userinput = dom::UserActivation::IsHandlingUserInput();
-
-  // Flip "checked" state if we're a checkbox menu, or an un-checked radio menu.
-  bool needToFlipChecked = false;
-  if (*menuType == MenuType::Checkbox ||
-      (*menuType == MenuType::Radio && !GetXULBoolAttr(nsGkAtoms::checked))) {
-    needToFlipChecked = !AttrValueIs(kNameSpaceID_None, nsGkAtoms::autocheck,
-                                     nsGkAtoms::_false, eCaseMatters);
-  }
-
-  mDelayedMenuCommandEvent = new nsXULMenuCommandEvent(
-      this, aIsTrusted, aModifiers, userinput, needToFlipChecked, aButton);
-  StartBlinking();
-}
-
-void XULButtonElement::StopBlinking() {
-  if (mMenuBlinkTimer) {
-    if (auto* parent = GetMenuParent()) {
-      parent->LockMenuUntilClosed(false);
-    }
-    mMenuBlinkTimer->Cancel();
-    mMenuBlinkTimer = nullptr;
-  }
-  mDelayedMenuCommandEvent = nullptr;
-}
-
-void XULButtonElement::PassMenuCommandEventToPopupManager() {
-  if (mDelayedMenuCommandEvent) {
-    if (RefPtr<nsXULPopupManager> pm = nsXULPopupManager::GetInstance()) {
-      RefPtr<nsXULMenuCommandEvent> event = std::move(mDelayedMenuCommandEvent);
-      nsCOMPtr<nsIContent> content = this;
-      pm->ExecuteMenu(content, event);
-    }
-  }
-  mDelayedMenuCommandEvent = nullptr;
-}
-
-static constexpr int32_t kBlinkDelay = 67;  // milliseconds
-
-void XULButtonElement::StartBlinking() {
-  if (!LookAndFeel::GetInt(LookAndFeel::IntID::ChosenMenuItemsShouldBlink)) {
-    PassMenuCommandEventToPopupManager();
-    return;
-  }
-
-  // Blink off.
-  UnsetAttr(kNameSpaceID_None, nsGkAtoms::menuactive, true);
-  if (auto* parent = GetMenuParent()) {
-    // Make this menu ignore events from now on.
-    parent->LockMenuUntilClosed(true);
-  }
-
-  // Set up a timer to blink back on.
-  NS_NewTimerWithFuncCallback(
-      getter_AddRefs(mMenuBlinkTimer),
-      [](nsITimer*, void* aClosure) MOZ_CAN_RUN_SCRIPT_BOUNDARY {
-        RefPtr self = static_cast<XULButtonElement*>(aClosure);
-        if (auto* parent = self->GetMenuParent()) {
-          if (parent->GetActiveMenuChild() == self) {
-            // Restore the highlighting if we're still the active item.
-            self->SetAttr(kNameSpaceID_None, nsGkAtoms::menuactive, u"true"_ns,
-                          true);
-          }
-        }
-        // Reuse our timer to actually execute.
-        self->mMenuBlinkTimer->InitWithNamedFuncCallback(
-            [](nsITimer*, void* aClosure) MOZ_CAN_RUN_SCRIPT_BOUNDARY {
-              RefPtr self = static_cast<XULButtonElement*>(aClosure);
-              if (auto* parent = self->GetMenuParent()) {
-                parent->LockMenuUntilClosed(false);
-              }
-              self->PassMenuCommandEventToPopupManager();
-              self->StopBlinking();
-            },
-            aClosure, kBlinkDelay, nsITimer::TYPE_ONE_SHOT,
-            "XULButtonElement::ContinueBlinking");
-      },
-      this, kBlinkDelay, nsITimer::TYPE_ONE_SHOT,
-      "XULButtonElement::StartBlinking",
-      OwnerDoc()->EventTargetFor(TaskCategory::Other));
-}
-
-void XULButtonElement::UnbindFromTree(bool aNullParent) {
-  StopBlinking();
-  nsXULElement::UnbindFromTree(aNullParent);
-}
-
-void XULButtonElement::ExecuteMenu(WidgetEvent& aEvent) {
-  MOZ_ASSERT(IsMenu());
-  if (nsCOMPtr<nsISound> sound = do_GetService("@mozilla.org/sound;1")) {
-    sound->PlayEventSound(nsISound::EVENT_MENU_EXECUTE);
-  }
-
-  Modifiers modifiers = 0;
-  if (WidgetInputEvent* inputEvent = aEvent.AsInputEvent()) {
-    modifiers = inputEvent->mModifiers;
-  }
-
-  int16_t button = 0;
-  if (WidgetMouseEventBase* mouseEvent = aEvent.AsMouseEventBase()) {
-    button = mouseEvent->mButton;
-  }
-
-  ExecuteMenu(modifiers, button, aEvent.IsTrusted());
-}
-
-void XULButtonElement::PostHandleEventForMenus(
-    EventChainPostVisitor& aVisitor) {
-  auto* event = aVisitor.mEvent;
-
-  if (NS_WARN_IF(event->mOriginalTarget != this)) {
-    return;
-  }
-
-  if (auto* parent = GetMenuParent()) {
-    if (NS_WARN_IF(parent->IsLocked())) {
-      return;
-    }
-  }
-
-  // If a menu just opened, ignore the mouseup event that might occur after a
-  // the mousedown event that opened it. However, if a different mousedown event
-  // occurs, just clear this flag.
-  if (!gMenuJustOpenedOrClosedTime.IsNull()) {
-    if (event->mMessage == eMouseDown) {
-      gMenuJustOpenedOrClosedTime = TimeStamp();
-    } else if (event->mMessage == eMouseUp) {
-      return;
-    }
-  }
-
-  if (event->mMessage == eKeyPress && !IsDisabled()) {
-    WidgetKeyboardEvent* keyEvent = event->AsKeyboardEvent();
-    uint32_t keyCode = keyEvent->mKeyCode;
-#ifdef XP_MACOSX
-    // On mac, open menulist on either up/down arrow or space (w/o Cmd pressed)
-    if (!IsMenuPopupOpen() &&
-        ((keyEvent->mCharCode == ' ' && !keyEvent->IsMeta()) ||
-         (keyCode == NS_VK_UP || keyCode == NS_VK_DOWN))) {
-      // When pressing space, don't open the menu if performing an incremental
-      // search.
-      if (keyEvent->mCharCode != ' ' ||
-          !nsMenuPopupFrame::IsWithinIncrementalTime(keyEvent->mTimeStamp)) {
-        aVisitor.mEventStatus = nsEventStatus_eConsumeNoDefault;
-        OpenMenuPopup(false);
-      }
-    }
-#else
-    // On other platforms, toggle menulist on unmodified F4 or Alt arrow
-    if ((keyCode == NS_VK_F4 && !keyEvent->IsAlt()) ||
-        ((keyCode == NS_VK_UP || keyCode == NS_VK_DOWN) && keyEvent->IsAlt())) {
-      aVisitor.mEventStatus = nsEventStatus_eConsumeNoDefault;
-      ToggleMenuState();
-    }
-#endif
-  } else if (event->mMessage == eMouseDown &&
-             event->AsMouseEvent()->mButton == MouseButton::ePrimary &&
-#ifdef XP_MACOSX
-             // On mac, ctrl-click will send a context menu event from the
-             // widget, so we don't want to bring up the menu.
-             !event->AsMouseEvent()->IsControl() &&
-#endif
-             !IsDisabled() && !IsMenuItem()) {
-    // The menu item was selected. Bring up the menu.
-    // We have children.
-    // Don't prevent the default action here, since that will also cancel
-    // potential drag starts.
-    if (!IsOnMenu()) {
-      ToggleMenuState();
-    } else if (!IsMenuPopupOpen()) {
-      OpenMenuPopup(false);
-    }
-  } else if (event->mMessage == eMouseUp && IsMenuItem() && !IsDisabled() &&
-             !event->mFlags.mMultipleActionsPrevented) {
-    // We accept left and middle clicks on all menu items to activate the item.
-    // On context menus we also accept right click to activate the item, because
-    // right-clicking on an item in a context menu cannot open another context
-    // menu.
-    bool isMacCtrlClick = false;
-#ifdef XP_MACOSX
-    isMacCtrlClick = event->AsMouseEvent()->mButton == MouseButton::ePrimary &&
-                     event->AsMouseEvent()->IsControl();
-#endif
-    bool clickMightOpenContextMenu =
-        event->AsMouseEvent()->mButton == MouseButton::eSecondary ||
-        isMacCtrlClick;
-    if (!clickMightOpenContextMenu || IsOnContextMenu()) {
-      aVisitor.mEventStatus = nsEventStatus_eConsumeNoDefault;
-      ExecuteMenu(*event);
-    }
-  } else if (event->mMessage == eContextMenu && IsOnContextMenu() &&
-             !IsMenuItem() && !IsDisabled()) {
-    // Make sure we cancel default processing of the context menu event so
-    // that it doesn't bubble and get seen again by the popuplistener and show
-    // another context menu.
-    aVisitor.mEventStatus = nsEventStatus_eConsumeNoDefault;
-  } else if (event->mMessage == eMouseOut) {
-    KillMenuOpenTimer();
-  } else if (event->mMessage == eMouseMove && (IsOnMenu() || IsOnMenuBar())) {
-    // Use a tolerance to address situations where a user might perform a
-    // "wiggly" click that is accompanied by near-simultaneous mousemove events.
-    const TimeDuration kTolerance = TimeDuration::FromMilliseconds(200);
-    if (!gMenuJustOpenedOrClosedTime.IsNull() &&
-        gMenuJustOpenedOrClosedTime + kTolerance < TimeStamp::Now()) {
-      gMenuJustOpenedOrClosedTime = TimeStamp();
-      return;
-    }
-
-    if (IsDisabled() && IsOnMenuList()) {
-      return;
-    }
-
-    RefPtr<XULMenuParentElement> parent = GetMenuParent();
-    MOZ_ASSERT(parent, "How did IsOnMenu{,Bar} return true then?");
-
-    const bool isOnOpenMenubar =
-        parent->IsMenuBar() && parent->GetActiveMenuChild() &&
-        parent->GetActiveMenuChild()->IsMenuPopupOpen();
-
-    parent->SetActiveMenuChild(this);
-
-    // We need to check if we really became the current menu item or not.
-    if (!IsMenuActive()) {
-      // We didn't (presumably because a context menu was active)
-      return;
-    }
-    if (IsDisabled() || IsMenuItem() || IsMenuPopupOpen() || mMenuOpenTimer) {
-      // Disabled, or already opening or what not.
-      return;
-    }
-
-    if (parent->IsMenuBar() && !isOnOpenMenubar) {
-      // We should only open on hover in the menubar iff the menubar is open
-      // already.
-      return;
-    }
-
-    // A timer is used so that it doesn't open if the user moves the mouse
-    // quickly past the menu. The MenuOpenCloseDelay ensures that only menus
-    // have this behaviour.
-    NS_NewTimerWithFuncCallback(
-        getter_AddRefs(mMenuOpenTimer),
-        [](nsITimer*, void* aClosure) MOZ_CAN_RUN_SCRIPT_BOUNDARY {
-          RefPtr self = static_cast<XULButtonElement*>(aClosure);
-          self->mMenuOpenTimer = nullptr;
-          if (self->IsMenuPopupOpen()) {
-            return;
-          }
-          // make sure we didn't open a context menu in the meantime
-          // (i.e. the user right-clicked while hovering over a submenu).
-          nsXULPopupManager* pm = nsXULPopupManager::GetInstance();
-          if (!pm) {
-            return;
-          }
-          if (pm->HasContextMenu(nullptr) && !self->IsOnContextMenu()) {
-            return;
-          }
-          if (!self->IsMenuActive()) {
-            return;
-          }
-          self->OpenMenuPopup(false);
-        },
-        this, MenuOpenCloseDelay(), nsITimer::TYPE_ONE_SHOT,
-        "XULButtonElement::OpenMenu",
-        OwnerDoc()->EventTargetFor(TaskCategory::Other));
-  }
-}
-
 nsresult XULButtonElement::PostHandleEvent(EventChainPostVisitor& aVisitor) {
   if (aVisitor.mEventStatus == nsEventStatus_eConsumeNoDefault) {
     return nsXULElement::PostHandleEvent(aVisitor);
   }
 
-  if (IsMenu()) {
-    PostHandleEventForMenus(aVisitor);
+  // Menu buttons have their own event handling, don't mess with that.
+  if (GetPrimaryFrame() && GetPrimaryFrame()->IsMenuFrame()) {
     return nsXULElement::PostHandleEvent(aVisitor);
   }
 
@@ -606,7 +131,7 @@ void XULButtonElement::Blurred() {
 
 bool XULButtonElement::MouseClicked(WidgetGUIEvent& aEvent) {
   // Don't execute if we're disabled.
-  if (IsDisabled() || !IsInComposedDoc()) {
+  if (GetXULBoolAttr(nsGkAtoms::disabled) || !IsInComposedDoc()) {
     return false;
   }
 
@@ -633,150 +158,4 @@ bool XULButtonElement::MouseClicked(WidgetGUIEvent& aEvent) {
   return true;
 }
 
-bool XULButtonElement::IsMenu() const {
-  if (mIsAlwaysMenu) {
-    return true;
-  }
-  return IsAnyOfXULElements(nsGkAtoms::button, nsGkAtoms::toolbarbutton) &&
-         AttrValueIs(kNameSpaceID_None, nsGkAtoms::type, nsGkAtoms::menu,
-                     eCaseMatters);
-}
-
-void XULButtonElement::UncheckRadioSiblings() {
-  MOZ_ASSERT(!nsContentUtils::IsSafeToRunScript());
-  MOZ_ASSERT(GetMenuType() == Some(MenuType::Radio));
-  nsAutoString groupName;
-  GetAttr(nsGkAtoms::name, groupName);
-
-  nsIContent* parent = GetParent();
-  if (!parent) {
-    return;
-  }
-
-  auto ShouldUncheck = [&](const nsIContent& aSibling) {
-    const auto* button = XULButtonElement::FromNode(aSibling);
-    if (!button || button->GetMenuType() != Some(MenuType::Radio)) {
-      return false;
-    }
-    if (const auto* attr = button->GetParsedAttr(nsGkAtoms::name)) {
-      if (!attr->Equals(groupName, eCaseMatters)) {
-        return false;
-      }
-    } else if (!groupName.IsEmpty()) {
-      return false;
-    }
-    // we're in the same group, only uncheck if we're checked (for some reason,
-    // some tests rely on that specifically).
-    return button->GetXULBoolAttr(nsGkAtoms::checked);
-  };
-
-  for (nsIContent* child = parent->GetFirstChild(); child;
-       child = child->GetNextSibling()) {
-    if (child == this || !ShouldUncheck(*child)) {
-      continue;
-    }
-    child->AsElement()->UnsetAttr(nsGkAtoms::checked, IgnoreErrors());
-  }
-}
-
-nsresult XULButtonElement::AfterSetAttr(int32_t aNamespaceID, nsAtom* aName,
-                                        const nsAttrValue* aValue,
-                                        const nsAttrValue* aOldValue,
-                                        nsIPrincipal* aSubjectPrincipal,
-                                        bool aNotify) {
-  MOZ_TRY(nsXULElement::AfterSetAttr(aNamespaceID, aName, aValue, aOldValue,
-                                     aSubjectPrincipal, aNotify));
-  if (IsAlwaysMenu() && aNamespaceID == kNameSpaceID_None) {
-    // We need to uncheck radio siblings when we're a checked radio and switch
-    // groups, or become checked.
-    const bool shouldUncheckSiblings = [&] {
-      if (aName == nsGkAtoms::type || aName == nsGkAtoms::name) {
-        return *GetMenuType() == MenuType::Radio &&
-               GetXULBoolAttr(nsGkAtoms::checked);
-      }
-      if (aName == nsGkAtoms::checked && aValue &&
-          aValue->Equals(nsGkAtoms::_true, eCaseMatters)) {
-        return *GetMenuType() == MenuType::Radio;
-      }
-      return false;
-    }();
-    if (shouldUncheckSiblings) {
-      UncheckRadioSiblings();
-    }
-  }
-  return NS_OK;
-}
-
-auto XULButtonElement::GetMenuType() const -> Maybe<MenuType> {
-  if (!IsAlwaysMenu()) {
-    return Nothing();
-  }
-
-  static Element::AttrValuesArray values[] = {nsGkAtoms::checkbox,
-                                              nsGkAtoms::radio, nullptr};
-  switch (FindAttrValueIn(kNameSpaceID_None, nsGkAtoms::type, values,
-                          eCaseMatters)) {
-    case 0:
-      return Some(MenuType::Checkbox);
-    case 1:
-      return Some(MenuType::Radio);
-    default:
-      return Some(MenuType::Normal);
-  }
-}
-
-nsMenuBarFrame* XULButtonElement::GetMenuBar(FlushType aFlushType) {
-  if (!IsMenu()) {
-    return nullptr;
-  }
-  nsIFrame* frame = GetPrimaryFrame(aFlushType);
-  for (; frame; frame = frame->GetParent()) {
-    if (nsMenuBarFrame* menubar = do_QueryFrame(frame)) {
-      return menubar;
-    }
-  }
-  return nullptr;
-}
-
-XULMenuParentElement* XULButtonElement::GetMenuParent() const {
-  return FirstAncestorOfType<XULMenuParentElement>();
-}
-
-XULPopupElement* XULButtonElement::GetMenuPopupContent() const {
-  auto* popup = GetMenuPopupWithoutFlushing();
-  if (!popup) {
-    return nullptr;
-  }
-  return &popup->PopupElement();
-}
-
-nsMenuPopupFrame* XULButtonElement::GetMenuPopupWithoutFlushing() const {
-  return const_cast<XULButtonElement*>(this)->GetMenuPopup(FlushType::None);
-}
-
-nsMenuPopupFrame* XULButtonElement::GetMenuPopup(FlushType aFlushType) {
-  if (!IsMenu()) {
-    return nullptr;
-  }
-  nsIFrame* frame = GetPrimaryFrame(aFlushType);
-  if (!frame) {
-    return nullptr;
-  }
-  for (auto* child : frame->PrincipalChildList()) {
-    if (!child->IsPlaceholderFrame()) {
-      continue;
-    }
-    auto* ph = static_cast<nsPlaceholderFrame*>(child);
-    if (nsMenuPopupFrame* popup = do_QueryFrame(ph->GetOutOfFlowFrame())) {
-      return popup;
-    }
-  }
-  return nullptr;
-}
-
-bool XULButtonElement::OpenedWithKey() {
-  nsMenuBarFrame* menubar = GetMenuBar(FlushType::Frames);
-  return menubar && menubar->IsActiveByKeyboard();
-}
-
 }  // namespace mozilla::dom

dom/xul/XULButtonElement.h

@@ -7,114 +7,25 @@
 #ifndef dom_xul_XULButtonElement_h__
 #define dom_xul_XULButtonElement_h__
 
-#include "mozilla/Attributes.h"
-#include "nsINode.h"
 #include "nsXULElement.h"
 
-class nsMenuBarFrame;
-class nsMenuPopupFrame;
-class nsXULMenuCommandEvent;
-
 namespace mozilla::dom {
 
-class KeyboardEvent;
-class XULPopupElement;
-class XULMenuParentElement;
-
-class XULButtonElement : public nsXULElement {
+class XULButtonElement final : public nsXULElement {
  public:
   explicit XULButtonElement(
-      already_AddRefed<mozilla::dom::NodeInfo>&& aNodeInfo);
-
-  ~XULButtonElement() override;
+      already_AddRefed<mozilla::dom::NodeInfo>&& aNodeInfo)
+      : nsXULElement(std::move(aNodeInfo)) {}
 
   MOZ_CAN_RUN_SCRIPT_BOUNDARY bool MouseClicked(WidgetGUIEvent&);
   MOZ_CAN_RUN_SCRIPT nsresult PostHandleEvent(EventChainPostVisitor&) override;
-  MOZ_CAN_RUN_SCRIPT void PostHandleEventForMenus(EventChainPostVisitor&);
-  MOZ_CAN_RUN_SCRIPT void HandleEnterKeyPress(WidgetEvent&);
-
-  void PopupOpened();
-  MOZ_CAN_RUN_SCRIPT void PopupClosed(bool aDeselectMenu);
-
-  XULPopupElement* GetContainingPopupElement() const;
-  nsMenuPopupFrame* GetContainingPopupWithoutFlushing() const;
-  MOZ_CAN_RUN_SCRIPT void ToggleMenuState();
-  bool IsMenuPopupOpen();
-
-  bool IsMenuItem() const { return NodeInfo()->Equals(nsGkAtoms::menuitem); }
-  bool IsMenuList() const { return NodeInfo()->Equals(nsGkAtoms::menulist); }
-  bool IsMenuActive() const;
-  MOZ_CAN_RUN_SCRIPT void OpenMenuPopup(bool aSelectFirstItem);
-  void CloseMenuPopup(bool aDeselectMenu);
-
-  bool IsOnMenu() const;
-  bool IsOnMenuList() const;
-  bool IsOnMenuBar() const;
-  bool IsOnContextMenu() const;
-
-  XULMenuParentElement* GetMenuParent() const;
-
-  void UnbindFromTree(bool aNullParent) override;
-
-  MOZ_CAN_RUN_SCRIPT bool HandleKeyPress(KeyboardEvent& keyEvent);
-  MOZ_CAN_RUN_SCRIPT bool OpenedWithKey();
-  // Called to execute our command handler.
-  MOZ_CAN_RUN_SCRIPT void ExecuteMenu(WidgetEvent&);
-  MOZ_CAN_RUN_SCRIPT void ExecuteMenu(Modifiers, int16_t aButton,
-                                      bool aIsTrusted);
-
-  // Whether we are a menu/menulist/menuitem element.
-  bool IsAlwaysMenu() const { return mIsAlwaysMenu; }
-  // Whether we should behave like a menu. This is the above plus buttons with
-  // type=menu attribute.
-  bool IsMenu() const;
 
   nsChangeHint GetAttributeChangeHint(const nsAtom* aAttribute,
                                       int32_t aModType) const override;
-  nsresult AfterSetAttr(int32_t aNamespaceID, nsAtom* aName,
-                        const nsAttrValue* aValue, const nsAttrValue* aOldValue,
-                        nsIPrincipal* aSubjectPrincipal, bool aNotify) override;
-
-  NS_IMPL_FROMNODE_HELPER(XULButtonElement,
-                          IsAnyOfXULElements(nsGkAtoms::checkbox,
-                                             nsGkAtoms::radio, nsGkAtoms::thumb,
-                                             nsGkAtoms::button, nsGkAtoms::menu,
-                                             nsGkAtoms::menulist,
-                                             nsGkAtoms::menuitem,
-                                             nsGkAtoms::toolbarbutton,
-                                             nsGkAtoms::toolbarpaletteitem,
-                                             nsGkAtoms::scrollbarbutton))
-
-  nsMenuPopupFrame* GetMenuPopup(FlushType aFlushType);
-  nsMenuPopupFrame* GetMenuPopupWithoutFlushing() const;
-  XULPopupElement* GetMenuPopupContent() const;
-  int32_t MenuOpenCloseDelay() const;
-
-  bool IsDisabled() const { return GetXULBoolAttr(nsGkAtoms::disabled); }
 
  private:
   void Blurred();
-  nsMenuBarFrame* GetMenuBar(FlushType aFlushType);
-  enum class MenuType {
-    Checkbox,
-    Radio,
-    Normal,
-  };
-  Maybe<MenuType> GetMenuType() const;
-
-  void UncheckRadioSiblings();
-  void StopBlinking();
-  MOZ_CAN_RUN_SCRIPT void StartBlinking();
-  void KillMenuOpenTimer();
-  MOZ_CAN_RUN_SCRIPT void PassMenuCommandEventToPopupManager();
-
   bool mIsHandlingKeyEvent = false;
-
-  // Whether this is a XULMenuElement.
-  const bool mIsAlwaysMenu;
-  RefPtr<nsXULMenuCommandEvent> mDelayedMenuCommandEvent;
-  nsCOMPtr<nsITimer> mMenuOpenTimer;
-  nsCOMPtr<nsITimer> mMenuBlinkTimer;
 };
 
 }  // namespace mozilla::dom

dom/xul/XULMenuElement.cpp

@@ -4,16 +4,17 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
-#include "mozilla/dom/XULMenuElement.h"
-#include "mozilla/StaticAnalysisFunctions.h"
-#include "mozilla/dom/XULButtonElement.h"
-#include "mozilla/dom/XULMenuElementBinding.h"
-#include "mozilla/dom/XULPopupElement.h"
 #include "mozilla/dom/KeyboardEvent.h"
 #include "mozilla/dom/KeyboardEventBinding.h"
+#include "mozilla/dom/Element.h"
+#include "nsIFrame.h"
+#include "nsMenuBarFrame.h"
 #include "nsMenuBarListener.h"
-#include "nsXULPopupManager.h"
+#include "nsMenuFrame.h"
 #include "nsMenuPopupFrame.h"
+#include "mozilla/dom/XULMenuElement.h"
+#include "mozilla/dom/XULMenuElementBinding.h"
+#include "nsXULPopupManager.h"
 
 namespace mozilla::dom {
 
@@ -22,31 +23,25 @@ JSObject* XULMenuElement::WrapNode(JSContext* aCx,
   return XULMenuElement_Binding::Wrap(aCx, this, aGivenProto);
 }
 
-Element* XULMenuElement::GetActiveMenuChild() {
-  RefPtr popup = GetMenuPopupContent();
-  return popup ? popup->GetActiveMenuChild() : nullptr;
+already_AddRefed<Element> XULMenuElement::GetActiveChild() {
+  nsMenuFrame* menu = do_QueryFrame(GetPrimaryFrame(FlushType::Frames));
+  if (menu) {
+    RefPtr<Element> el;
+    menu->GetActiveChild(getter_AddRefs(el));
+    return el.forget();
+  }
+  return nullptr;
 }
 
-void XULMenuElement::SetActiveMenuChild(Element* aChild) {
-  RefPtr popup = GetMenuPopupContent();
-  if (NS_WARN_IF(!popup)) {
-    return;
+void XULMenuElement::SetActiveChild(Element* arg) {
+  nsMenuFrame* menu = do_QueryFrame(GetPrimaryFrame(FlushType::Frames));
+  if (menu) {
+    menu->SetActiveChild(arg);
   }
-
-  if (!aChild) {
-    popup->SetActiveMenuChild(nullptr);
-    return;
-  }
-  auto* button = XULButtonElement::FromNode(aChild);
-  if (NS_WARN_IF(!button) || NS_WARN_IF(!button->IsMenu())) {
-    return;
-  }
-  // KnownLive because it's aChild.
-  popup->SetActiveMenuChild(MOZ_KnownLive(button));
 }
 
-bool XULButtonElement::HandleKeyPress(KeyboardEvent& keyEvent) {
-  RefPtr<nsXULPopupManager> pm = nsXULPopupManager::GetInstance();
+bool XULMenuElement::HandleKeyPress(KeyboardEvent& keyEvent) {
+  nsXULPopupManager* pm = nsXULPopupManager::GetInstance();
   if (!pm) {
     return false;
   }
@@ -56,12 +51,15 @@ bool XULButtonElement::HandleKeyPress(KeyboardEvent& keyEvent) {
     return false;
   }
 
-  if (nsMenuBarListener::IsAccessKeyPressed(keyEvent)) {
+  if (nsMenuBarListener::IsAccessKeyPressed(&keyEvent)) return false;
+
+  nsMenuFrame* menu = do_QueryFrame(GetPrimaryFrame(FlushType::Frames));
+  if (!menu) {
     return false;
   }
 
-  nsMenuPopupFrame* popupFrame = GetMenuPopup(FlushType::Frames);
-  if (NS_WARN_IF(!popupFrame)) {
+  nsMenuPopupFrame* popupFrame = menu->GetPopup();
+  if (!popupFrame) {
     return false;
   }
 
@@ -76,8 +74,25 @@ bool XULButtonElement::HandleKeyPress(KeyboardEvent& keyEvent) {
       return pm->HandleKeyboardNavigationInPopup(popupFrame, theDirection);
     }
     default:
-      return pm->HandleShortcutNavigation(keyEvent, popupFrame);
+      return pm->HandleShortcutNavigation(&keyEvent, popupFrame);
   }
 }
 
+bool XULMenuElement::OpenedWithKey() {
+  nsMenuFrame* menuframe = do_QueryFrame(GetPrimaryFrame(FlushType::Frames));
+  if (!menuframe) {
+    return false;
+  }
+
+  nsIFrame* frame = menuframe->GetParent();
+  while (frame) {
+    nsMenuBarFrame* menubar = do_QueryFrame(frame);
+    if (menubar) {
+      return menubar->IsActiveByKeyboard();
+    }
+    frame = frame->GetParent();
+  }
+  return false;
+}
+
 }  // namespace mozilla::dom

dom/xul/XULMenuElement.h

@@ -7,29 +7,27 @@
 #ifndef mozilla_dom_XULMenuElement_h
 #define mozilla_dom_XULMenuElement_h
 
-#include "XULButtonElement.h"
-#include "nsINode.h"
 #include "nsXULElement.h"
 
 namespace mozilla::dom {
 
 class KeyboardEvent;
 
-class XULMenuElement final : public XULButtonElement {
+class XULMenuElement final : public nsXULElement {
  public:
   explicit XULMenuElement(already_AddRefed<mozilla::dom::NodeInfo>&& aNodeInfo)
-      : XULButtonElement(std::move(aNodeInfo)) {}
+      : nsXULElement(std::move(aNodeInfo)) {}
 
-  MOZ_CAN_RUN_SCRIPT void SetActiveMenuChild(Element*);
-  Element* GetActiveMenuChild();
-
-  NS_IMPL_FROMNODE_HELPER(XULMenuElement,
-                          IsAnyOfXULElements(nsGkAtoms::menu,
-                                             nsGkAtoms::menulist));
+  MOZ_CAN_RUN_SCRIPT already_AddRefed<Element> GetActiveChild();
+  MOZ_CAN_RUN_SCRIPT void SetActiveChild(Element* arg);
+  MOZ_CAN_RUN_SCRIPT bool HandleKeyPress(KeyboardEvent& keyEvent);
+  MOZ_CAN_RUN_SCRIPT bool OpenedWithKey();
 
  private:
   virtual ~XULMenuElement() = default;
   JSObject* WrapNode(JSContext* aCx, JS::Handle<JSObject*> aGivenProto) final;
+
+  nsIFrame* GetFrame();
 };
 
 }  // namespace mozilla::dom

dom/xul/XULMenuParentElement.cpp

@@ -1,369 +0,0 @@
-/* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
-/* This Source Code Form is subject to the terms of the Mozilla Public
- * License, v. 2.0. If a copy of the MPL was not distributed with this
- * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
-
-#include "XULMenuParentElement.h"
-#include "XULButtonElement.h"
-#include "XULPopupElement.h"
-#include "mozilla/LookAndFeel.h"
-#include "mozilla/TextEvents.h"
-#include "mozilla/dom/DocumentInlines.h"
-#include "mozilla/dom/KeyboardEvent.h"
-#include "mozilla/EventDispatcher.h"
-#include "nsDebug.h"
-#include "nsMenuBarFrame.h"
-#include "nsMenuPopupFrame.h"
-#include "nsString.h"
-#include "nsStringFwd.h"
-#include "nsUTF8Utils.h"
-#include "nsXULElement.h"
-#include "nsMenuBarListener.h"
-#include "nsXULPopupManager.h"
-
-namespace mozilla::dom {
-
-NS_IMPL_ISUPPORTS_CYCLE_COLLECTION_INHERITED_0(XULMenuParentElement,
-                                               nsXULElement)
-NS_IMPL_CYCLE_COLLECTION_INHERITED(XULMenuParentElement, nsXULElement,
-                                   mActiveItem)
-
-XULMenuParentElement::XULMenuParentElement(
-    already_AddRefed<mozilla::dom::NodeInfo>&& aNodeInfo)
-    : nsXULElement(std::move(aNodeInfo)) {}
-
-XULMenuParentElement::~XULMenuParentElement() = default;
-
-class MenuActivateEvent final : public Runnable {
- public:
-  MenuActivateEvent(Element* aMenu, bool aIsActivate)
-      : Runnable("MenuActivateEvent"), mMenu(aMenu), mIsActivate(aIsActivate) {}
-
-  // TODO: Convert this to MOZ_CAN_RUN_SCRIPT (bug 1415230, bug 1535398)
-  MOZ_CAN_RUN_SCRIPT_BOUNDARY NS_IMETHOD Run() override {
-    nsAutoString domEventToFire;
-    if (mIsActivate) {
-      // Highlight the menu.
-      mMenu->SetAttr(kNameSpaceID_None, nsGkAtoms::menuactive, u"true"_ns,
-                     true);
-      // The menuactivated event is used by accessibility to track the user's
-      // movements through menus
-      domEventToFire.AssignLiteral("DOMMenuItemActive");
-    } else {
-      // Unhighlight the menu.
-      mMenu->UnsetAttr(kNameSpaceID_None, nsGkAtoms::menuactive, true);
-      domEventToFire.AssignLiteral("DOMMenuItemInactive");
-    }
-
-    RefPtr<nsPresContext> pc = mMenu->OwnerDoc()->GetPresContext();
-    RefPtr<dom::Event> event = NS_NewDOMEvent(mMenu, pc, nullptr);
-    event->InitEvent(domEventToFire, true, true);
-
-    event->SetTrusted(true);
-
-    EventDispatcher::DispatchDOMEvent(mMenu, nullptr, event, pc, nullptr);
-    return NS_OK;
-  }
-
- private:
-  const RefPtr<Element> mMenu;
-  bool mIsActivate;
-};
-
-static void ActivateOrDeactivate(XULButtonElement& aButton, bool aActivate) {
-  if (!aButton.IsMenu()) {
-    return;
-  }
-
-  if (nsXULPopupManager* pm = nsXULPopupManager::GetInstance()) {
-    if (aActivate) {
-      // Cancel the close timer if selecting a menu within the popup to be
-      // closed
-      pm->CancelMenuTimer(aButton.GetContainingPopupWithoutFlushing());
-    } else if (auto* popup = aButton.GetMenuPopupWithoutFlushing()) {
-      // Set up the close timer if deselecting selecting a sub-menu.
-      pm->HidePopupAfterDelay(popup, aButton.MenuOpenCloseDelay());
-    }
-  }
-
-  nsCOMPtr<nsIRunnable> event = new MenuActivateEvent(&aButton, aActivate);
-  aButton.OwnerDoc()->Dispatch(TaskCategory::Other, event.forget());
-}
-
-void XULMenuParentElement::LockMenuUntilClosed(bool aLock) {
-  auto* popup = XULPopupElement::FromNode(*this);
-  if (!popup) {
-    return;
-  }
-  mLocked = aLock;
-  // Lock/Unlock the parent menu too.
-  if (XULButtonElement* menu = popup->GetContainingMenu()) {
-    if (XULMenuParentElement* parent = menu->GetMenuParent()) {
-      parent->LockMenuUntilClosed(aLock);
-    }
-  }
-}
-
-void XULMenuParentElement::SetActiveMenuChild(XULButtonElement* aChild,
-                                              ByKey aByKey) {
-  if (aChild == mActiveItem) {
-    return;
-  }
-
-  if (mActiveItem) {
-    ActivateOrDeactivate(*mActiveItem, false);
-  }
-  mActiveItem = nullptr;
-
-  if (nsMenuBarFrame* f = do_QueryFrame(GetPrimaryFrame())) {
-    f->SetActive(!!aChild);
-  }
-
-  if (!aChild) {
-    return;
-  }
-
-  mActiveItem = aChild;
-  ActivateOrDeactivate(*mActiveItem, true);
-
-  if (IsInMenuList()) {
-    if (nsMenuPopupFrame* f = do_QueryFrame(GetPrimaryFrame())) {
-      f->EnsureActiveMenuListItemIsVisible();
-#ifdef XP_WIN
-      // On Windows, a menulist should update its value whenever navigation was
-      // done by the keyboard.
-      //
-      // NOTE(emilio): This is a rather odd per-platform behavior difference,
-      // but other browsers also do this.
-      if (aByKey == ByKey::Yes && f->IsOpen()) {
-        // Fire a command event as the new item, but we don't want to close the
-        // menu, blink it, or update any other state of the menuitem. The
-        // command event will cause the item to be selected.
-        RefPtr<mozilla::PresShell> presShell = OwnerDoc()->GetPresShell();
-        nsContentUtils::DispatchXULCommand(aChild, /* aTrusted = */ true,
-                                           nullptr, presShell, false, false,
-                                           false, false);
-      }
-#endif
-    }
-  }
-}
-
-static bool IsValidMenuItem(const XULMenuParentElement& aMenuParent,
-                            const nsIContent& aContent) {
-  const auto* button = XULButtonElement::FromNode(aContent);
-  if (!button || !button->IsMenu()) {
-    return false;
-  }
-  if (!button->GetPrimaryFrame()) {
-    // Hidden buttons are not focusable/activatable.
-    return false;
-  }
-  if (!button->IsDisabled()) {
-    return true;
-  }
-  // In the menubar or a menulist disabled items are always skipped.
-  const bool skipDisabled =
-      LookAndFeel::GetInt(LookAndFeel::IntID::SkipNavigatingDisabledMenuItem) ||
-      aMenuParent.IsMenuBar() || aMenuParent.IsInMenuList();
-  return !skipDisabled;
-}
-
-enum class StartKind { Parent, Item };
-
-template <bool aForward>
-static XULButtonElement* DoGetNextMenuItem(
-    const XULMenuParentElement& aMenuParent, const nsIContent& aStart,
-    StartKind aStartKind) {
-  nsIContent* start =
-      aStartKind == StartKind::Item
-          ? (aForward ? aStart.GetNextSibling() : aStart.GetPreviousSibling())
-          : (aForward ? aStart.GetFirstChild() : aStart.GetLastChild());
-  for (nsIContent* node = start; node;
-       node = aForward ? node->GetNextSibling() : node->GetPreviousSibling()) {
-    if (IsValidMenuItem(aMenuParent, *node)) {
-      return static_cast<XULButtonElement*>(node);
-    }
-    if (node->IsXULElement(nsGkAtoms::menugroup)) {
-      if (XULButtonElement* child = DoGetNextMenuItem<aForward>(
-              aMenuParent, *node, StartKind::Parent)) {
-        return child;
-      }
-    }
-  }
-  if (aStartKind == StartKind::Item && aStart.GetParent() &&
-      aStart.GetParent()->IsXULElement(nsGkAtoms::menugroup)) {
-    // We haven't found anything in aStart's sibling list, but if we're in a
-    // group we need to keep looking.
-    return DoGetNextMenuItem<aForward>(aMenuParent, *aStart.GetParent(),
-                                       StartKind::Item);
-  }
-  return nullptr;
-}
-
-XULButtonElement* XULMenuParentElement::GetFirstMenuItem() const {
-  return DoGetNextMenuItem<true>(*this, *this, StartKind::Parent);
-}
-
-XULButtonElement* XULMenuParentElement::GetLastMenuItem() const {
-  return DoGetNextMenuItem<false>(*this, *this, StartKind::Parent);
-}
-
-XULButtonElement* XULMenuParentElement::GetNextMenuItemFrom(
-    const XULButtonElement& aStartingItem) const {
-  return DoGetNextMenuItem<true>(*this, aStartingItem, StartKind::Item);
-}
-
-XULButtonElement* XULMenuParentElement::GetPrevMenuItemFrom(
-    const XULButtonElement& aStartingItem) const {
-  return DoGetNextMenuItem<false>(*this, aStartingItem, StartKind::Item);
-}
-
-XULButtonElement* XULMenuParentElement::GetNextMenuItem(Wrap aWrap) const {
-  if (mActiveItem) {
-    if (auto* next = GetNextMenuItemFrom(*mActiveItem)) {
-      return next;
-    }
-    if (aWrap == Wrap::No) {
-      return nullptr;
-    }
-  }
-  return GetFirstMenuItem();
-}
-
-XULButtonElement* XULMenuParentElement::GetPrevMenuItem(Wrap aWrap) const {
-  if (mActiveItem) {
-    if (auto* prev = GetPrevMenuItemFrom(*mActiveItem)) {
-      return prev;
-    }
-    if (aWrap == Wrap::No) {
-      return nullptr;
-    }
-  }
-  return GetLastMenuItem();
-}
-
-void XULMenuParentElement::SelectFirstItem() {
-  if (RefPtr firstItem = GetFirstMenuItem()) {
-    SetActiveMenuChild(firstItem);
-  }
-}
-
-XULButtonElement* XULMenuParentElement::FindMenuWithShortcut(
-    KeyboardEvent& aKeyEvent) const {
-  using AccessKeyArray = AutoTArray<uint32_t, 10>;
-  AccessKeyArray accessKeys;
-  WidgetKeyboardEvent* nativeKeyEvent =
-      aKeyEvent.WidgetEventPtr()->AsKeyboardEvent();
-  if (nativeKeyEvent) {
-    nativeKeyEvent->GetAccessKeyCandidates(accessKeys);
-  }
-  const uint32_t charCode = aKeyEvent.CharCode();
-  if (accessKeys.IsEmpty() && charCode) {
-    accessKeys.AppendElement(charCode);
-  }
-  if (accessKeys.IsEmpty()) {
-    return nullptr;  // no character was pressed so just return
-  }
-  XULButtonElement* foundMenu = nullptr;
-  size_t foundIndex = AccessKeyArray::NoIndex;
-  for (auto* item = GetFirstMenuItem(); item;
-       item = GetNextMenuItemFrom(*item)) {
-    nsAutoString shortcutKey;
-    item->GetAttr(nsGkAtoms::accesskey, shortcutKey);
-    if (shortcutKey.IsEmpty()) {
-      continue;
-    }
-
-    ToLowerCase(shortcutKey);
-    const char16_t* start = shortcutKey.BeginReading();
-    const char16_t* end = shortcutKey.EndReading();
-    uint32_t ch = UTF16CharEnumerator::NextChar(&start, end);
-    size_t index = accessKeys.IndexOf(ch);
-    if (index == AccessKeyArray::NoIndex) {
-      continue;
-    }
-    if (foundIndex == AccessKeyArray::NoIndex || index < foundIndex) {
-      foundMenu = item;
-      foundIndex = index;
-    }
-  }
-  return foundMenu;
-}
-
-XULButtonElement* XULMenuParentElement::FindMenuWithShortcut(
-    const nsAString& aString, bool& aDoAction) const {
-  aDoAction = false;
-  uint32_t accessKeyMatchCount = 0;
-  uint32_t matchCount = 0;
-
-  XULButtonElement* foundAccessKeyMenu = nullptr;
-  XULButtonElement* foundMenuBeforeCurrent = nullptr;
-  XULButtonElement* foundMenuAfterCurrent = nullptr;
-
-  bool foundActive = false;
-  for (auto* item = GetFirstMenuItem(); item;
-       item = GetNextMenuItemFrom(*item)) {
-    nsAutoString textKey;
-    // Get the shortcut attribute.
-    item->GetAttr(nsGkAtoms::accesskey, textKey);
-    const bool isAccessKey = !textKey.IsEmpty();
-    if (textKey.IsEmpty()) {  // No shortcut, try first letter
-      item->GetAttr(nsGkAtoms::label, textKey);
-      if (textKey.IsEmpty()) {  // No label, try another attribute (value)
-        item->GetAttr(nsGkAtoms::value, textKey);
-      }
-    }
-
-    const bool isActive = item == GetActiveMenuChild();
-    foundActive |= isActive;
-
-    if (!StringBeginsWith(
-            nsContentUtils::TrimWhitespace<
-                nsContentUtils::IsHTMLWhitespaceOrNBSP>(textKey, false),
-            aString, nsCaseInsensitiveStringComparator)) {
-      continue;
-    }
-    // There is one match
-    matchCount++;
-    if (isAccessKey) {
-      // There is one shortcut-key match
-      accessKeyMatchCount++;
-      // Record the matched item. If there is only one matched shortcut
-      // item, do it
-      foundAccessKeyMenu = item;
-    }
-    // Get the active status
-    if (isActive && aString.Length() > 1 && !foundMenuBeforeCurrent) {
-      // If there is more than one char typed and the current item matches, the
-      // current item has highest priority, otherwise the item next to current
-      // has highest priority.
-      return item;
-    }
-    if (!foundActive || isActive) {
-      // It's a first candidate item located before/on the current item
-      if (!foundMenuBeforeCurrent) {
-        foundMenuBeforeCurrent = item;
-      }
-    } else {
-      if (!foundMenuAfterCurrent) {
-        foundMenuAfterCurrent = item;
-      }
-    }
-  }
-
-  aDoAction = !IsInMenuList() && (matchCount == 1 || accessKeyMatchCount == 1);
-
-  if (accessKeyMatchCount == 1) {
-    // We have one matched accesskey item
-    return foundAccessKeyMenu;
-  }
-  // If we have matched an item after the current, use it.
-  if (foundMenuAfterCurrent) {
-    return foundMenuAfterCurrent;
-  }
-  // If we haven't, use the item before the current, if any.
-  return foundMenuBeforeCurrent;
-}
-
-}  // namespace mozilla::dom

dom/xul/XULMenuParentElement.h

@@ -1,77 +0,0 @@
-/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
-/* vim: set ts=8 sts=2 et sw=2 tw=80: */
-/* This Source Code Form is subject to the terms of the Mozilla Public
- * License, v. 2.0. If a copy of the MPL was not distributed with this
- * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
-
-#ifndef XULMenuParentElement_h__
-#define XULMenuParentElement_h__
-
-#include "mozilla/Attributes.h"
-#include "nsISupports.h"
-#include "nsXULElement.h"
-
-namespace mozilla::dom {
-
-class KeyboardEvent;
-class XULButtonElement;
-
-nsXULElement* NS_NewXULMenuParentElement(
-    already_AddRefed<mozilla::dom::NodeInfo>&& aNodeInfo);
-
-class XULMenuParentElement : public nsXULElement {
- public:
-  NS_DECL_ISUPPORTS_INHERITED
-  NS_DECL_CYCLE_COLLECTION_CLASS_INHERITED(XULMenuParentElement, nsXULElement)
-
-  explicit XULMenuParentElement(
-      already_AddRefed<mozilla::dom::NodeInfo>&& aNodeInfo);
-
-  bool IsMenuBar() const { return NodeInfo()->Equals(nsGkAtoms::menubar); }
-  bool IsMenu() const { return !IsMenuBar(); }
-
-  bool IsLocked() const { return mLocked; }
-
-  void LockMenuUntilClosed(bool aLock);
-
-  bool IsInMenuList() const {
-    return GetParent() && GetParent()->IsXULElement(nsGkAtoms::menulist);
-  }
-
-  XULButtonElement* FindMenuWithShortcut(KeyboardEvent&) const;
-  XULButtonElement* FindMenuWithShortcut(const nsAString& aString,
-                                         bool& aDoAction) const;
-
-  NS_IMPL_FROMNODE_HELPER(XULMenuParentElement,
-                          IsAnyOfXULElements(nsGkAtoms::menupopup,
-                                             nsGkAtoms::popup, nsGkAtoms::panel,
-                                             nsGkAtoms::tooltip,
-                                             nsGkAtoms::menubar));
-
-  XULButtonElement* GetActiveMenuChild() const { return mActiveItem.get(); }
-  enum class ByKey : bool { No, Yes };
-  MOZ_CAN_RUN_SCRIPT void SetActiveMenuChild(XULButtonElement*,
-                                             ByKey = ByKey::No);
-
-  XULButtonElement* GetFirstMenuItem() const;
-  XULButtonElement* GetLastMenuItem() const;
-
-  XULButtonElement* GetNextMenuItemFrom(const XULButtonElement&) const;
-  XULButtonElement* GetPrevMenuItemFrom(const XULButtonElement&) const;
-
-  enum class Wrap : bool { No, Yes };
-  XULButtonElement* GetNextMenuItem(Wrap = Wrap::Yes) const;
-  XULButtonElement* GetPrevMenuItem(Wrap = Wrap::Yes) const;
-
-  MOZ_CAN_RUN_SCRIPT void SelectFirstItem();
-
- protected:
-  RefPtr<XULButtonElement> mActiveItem;
-  bool mLocked = false;
-
-  ~XULMenuParentElement() override;
-};
-
-}  // namespace mozilla::dom
-
-#endif  // XULMenuParentElement_h

dom/xul/XULPopupElement.cpp

@@ -4,23 +4,18 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
-#include "XULMenuParentElement.h"
 #include "nsCOMPtr.h"
 #include "nsIContent.h"
-#include "nsMenuBarListener.h"
 #include "nsNameSpaceManager.h"
 #include "nsGkAtoms.h"
 #include "nsMenuPopupFrame.h"
 #include "nsView.h"
 #include "mozilla/AppUnits.h"
-#include "mozilla/AsyncEventDispatcher.h"
 #include "mozilla/dom/DOMRect.h"
 #include "mozilla/dom/Document.h"
 #include "mozilla/dom/Element.h"
 #include "mozilla/dom/Event.h"
 #include "mozilla/dom/XULPopupElement.h"
-#include "mozilla/dom/XULButtonElement.h"
-#include "mozilla/dom/XULMenuElement.h"
 #include "mozilla/dom/XULPopupElementBinding.h"
 #ifdef MOZ_WAYLAND
 #  include "mozilla/WidgetUtilsGtk.h"
@@ -40,12 +35,6 @@ JSObject* XULPopupElement::WrapNode(JSContext* aCx,
   return XULPopupElement_Binding::Wrap(aCx, this, aGivenProto);
 }
 
-nsMenuPopupFrame* XULPopupElement::GetFrame(FlushType aFlushType) {
-  nsIFrame* f = GetPrimaryFrame(aFlushType);
-  MOZ_ASSERT(!f || f->IsMenuPopupFrame());
-  return static_cast<nsMenuPopupFrame*>(f);
-}
-
 void XULPopupElement::OpenPopup(Element* aAnchorElement,
                                 const StringOrOpenPopupOptions& aOptions,
                                 int32_t aXPos, int32_t aYPos,
@@ -70,8 +59,9 @@ void XULPopupElement::OpenPopup(Element* aAnchorElement,
     // 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);
+      nsMenuFrame* menu = do_QueryFrame(GetPrimaryFrame()->GetParent());
+      if (menu) {
+        pm->ShowMenu(menu->GetContent(), false);
         return;
       }
     }
@@ -128,36 +118,6 @@ static Modifiers ConvertModifiers(const ActivateMenuItemOptions& aModifiers) {
   return modifiers;
 }
 
-XULButtonElement* XULPopupElement::GetContainingMenu() const {
-  auto* button = XULButtonElement::FromNodeOrNull(GetParent());
-  if (!button || !button->IsMenu()) {
-    return nullptr;
-  }
-  return button;
-}
-
-void XULPopupElement::PopupOpened(bool aSelectFirstItem) {
-  if (aSelectFirstItem) {
-    SelectFirstItem();
-  }
-  if (RefPtr button = GetContainingMenu()) {
-    if (RefPtr parent = button->GetMenuParent()) {
-      parent->SetActiveMenuChild(button);
-    }
-  }
-}
-
-void XULPopupElement::PopupClosed(bool aDeselectMenu) {
-  LockMenuUntilClosed(false);
-  SetActiveMenuChild(nullptr);
-  auto dispatcher = MakeRefPtr<AsyncEventDispatcher>(
-      this, u"DOMMenuInactive"_ns, CanBubble::eYes, ChromeOnlyDispatch::eNo);
-  dispatcher->PostDOMEvent();
-  if (RefPtr button = GetContainingMenu()) {
-    button->PopupClosed(aDeselectMenu);
-  }
-}
-
 void XULPopupElement::ActivateItem(Element& aItemElement,
                                    const ActivateMenuItemOptions& aOptions,
                                    ErrorResult& aRv) {
@@ -175,29 +135,19 @@ void XULPopupElement::ActivateItem(Element& aItemElement,
     }
   }
 
-  auto* item = XULButtonElement::FromNode(aItemElement);
-  if (!item || !item->IsMenu()) {
-    return aRv.ThrowInvalidStateError("Not a menu item");
+  // Used only to flush frames.
+  GetPrimaryFrame(FlushType::Frames);
+
+  nsMenuFrame* itemFrame = do_QueryFrame(aItemElement.GetPrimaryFrame());
+  if (!itemFrame) {
+    return aRv.ThrowInvalidStateError("Menu item is not visible");
   }
 
-  if (!item->GetPrimaryFrame(FlushType::Frames)) {
-    return aRv.ThrowInvalidStateError("Menu item is hidden");
+  if (!itemFrame->GetMenuParent() || !itemFrame->GetMenuParent()->IsOpen()) {
+    return aRv.ThrowInvalidStateError("Menu is closed");
   }
 
-  auto* popup = item->GetContainingPopupElement();
-  if (!popup) {
-    return aRv.ThrowInvalidStateError("No popup");
-  }
-
-  nsMenuPopupFrame* frame = popup->GetFrame(FlushType::None);
-  if (!frame || !frame->IsOpen()) {
-    return aRv.ThrowInvalidStateError("Popup is not open");
-  }
-
-  // This is a chrome-only API, so we're trusted.
-  const bool trusted = true;
-  // KnownLive because item is aItemElement.
-  MOZ_KnownLive(item)->ExecuteMenu(modifiers, aOptions.mButton, trusted);
+  itemFrame->ActivateItem(modifiers, aOptions.mButton);
 }
 
 void XULPopupElement::MoveTo(int32_t aLeft, int32_t aTop) {
@@ -210,7 +160,7 @@ void XULPopupElement::MoveTo(int32_t aLeft, int32_t aTop) {
 void XULPopupElement::MoveToAnchor(Element* aAnchorElement,
                                    const nsAString& aPosition, int32_t aXPos,
                                    int32_t aYPos, bool aAttributesOverride) {
-  nsMenuPopupFrame* menuPopupFrame = GetFrame(FlushType::None);
+  nsMenuPopupFrame* menuPopupFrame = do_QueryFrame(GetPrimaryFrame());
   if (menuPopupFrame && menuPopupFrame->IsVisibleOrShowing()) {
     menuPopupFrame->MoveToAnchor(aAnchorElement, aPosition, aXPos, aYPos,
                                  aAttributesOverride);

dom/xul/XULPopupElement.h

@@ -7,15 +7,12 @@
 #ifndef XULPopupElement_h__
 #define XULPopupElement_h__
 
-#include "XULMenuParentElement.h"
 #include "mozilla/Attributes.h"
 #include "nsCycleCollectionParticipant.h"
-#include "nsINode.h"
 #include "nsWrapperCache.h"
 #include "nsString.h"
 #include "nsXULElement.h"
 
-class nsMenuPopupFrame;
 struct JSContext;
 
 namespace mozilla {
@@ -32,13 +29,13 @@ struct ActivateMenuItemOptions;
 nsXULElement* NS_NewXULPopupElement(
     already_AddRefed<mozilla::dom::NodeInfo>&& aNodeInfo);
 
-class XULPopupElement : public XULMenuParentElement {
+class XULPopupElement : public nsXULElement {
  private:
-  nsMenuPopupFrame* GetFrame(FlushType);
+  nsIFrame* GetFrame(bool aFlushLayout);
 
  public:
   explicit XULPopupElement(already_AddRefed<mozilla::dom::NodeInfo>&& aNodeInfo)
-      : XULMenuParentElement(std::move(aNodeInfo)) {}
+      : nsXULElement(std::move(aNodeInfo)) {}
 
   void GetLabel(DOMString& aValue) const {
     GetXULAttr(nsGkAtoms::label, aValue);
@@ -69,17 +66,11 @@ class XULPopupElement : public XULMenuParentElement {
 
   void HidePopup(bool aCancel);
 
-  MOZ_CAN_RUN_SCRIPT void ActivateItem(Element& aItemElement,
-                                       const ActivateMenuItemOptions& aOptions,
-                                       ErrorResult& aRv);
+  void ActivateItem(Element& aItemElement,
+                    const ActivateMenuItemOptions& aOptions, ErrorResult& aRv);
 
   void GetState(nsString& aState);
 
-  MOZ_CAN_RUN_SCRIPT void PopupOpened(bool aSelectFirstItem);
-  MOZ_CAN_RUN_SCRIPT void PopupClosed(bool aDeselectMenu);
-
-  XULButtonElement* GetContainingMenu() const;
-
   nsINode* GetTriggerNode() const;
 
   Element* GetAnchorNode() const;
@@ -98,11 +89,6 @@ class XULPopupElement : public XULMenuParentElement {
   bool IsWaylandDragSource() const;
   bool IsWaylandPopup() const;
 
-  NS_IMPL_FROMNODE_HELPER(XULPopupElement,
-                          IsAnyOfXULElements(nsGkAtoms::menupopup,
-                                             nsGkAtoms::popup, nsGkAtoms::panel,
-                                             nsGkAtoms::tooltip));
-
  protected:
   virtual ~XULPopupElement() = default;
 

dom/xul/XULTooltipElement.h

@@ -21,10 +21,12 @@ class XULTooltipElement final : public XULPopupElement {
       : XULPopupElement(std::move(aNodeInfo)) {}
   nsresult Init();
 
-  nsresult AfterSetAttr(int32_t aNameSpaceID, nsAtom* aName,
-                        const nsAttrValue* aValue, const nsAttrValue* aOldValue,
-                        nsIPrincipal* aSubjectPrincipal, bool aNotify) override;
-  nsresult PostHandleEvent(EventChainPostVisitor& aVisitor) override;
+  virtual nsresult AfterSetAttr(int32_t aNameSpaceID, nsAtom* aName,
+                                const nsAttrValue* aValue,
+                                const nsAttrValue* aOldValue,
+                                nsIPrincipal* aSubjectPrincipal,
+                                bool aNotify) override;
+  virtual nsresult PostHandleEvent(EventChainPostVisitor& aVisitor) override;
 
  protected:
   virtual ~XULTooltipElement() = default;

dom/xul/moz.build

@@ -25,7 +25,6 @@ EXPORTS.mozilla.dom += [
     "XULButtonElement.h",
     "XULFrameElement.h",
     "XULMenuElement.h",
-    "XULMenuParentElement.h",
     "XULPersist.h",
     "XULPopupElement.h",
     "XULResizerElement.h",
@@ -47,7 +46,6 @@ UNIFIED_SOURCES += [
     "XULButtonElement.cpp",
     "XULFrameElement.cpp",
     "XULMenuElement.cpp",
-    "XULMenuParentElement.cpp",
     "XULPersist.cpp",
     "XULPopupElement.cpp",
     "XULResizerElement.cpp",

dom/xul/nsXULElement.cpp

@@ -99,6 +99,7 @@
 #include "nsISupportsUtils.h"
 #include "nsIURI.h"
 #include "nsIXPConnect.h"
+#include "nsMenuFrame.h"
 #include "nsMenuPopupFrame.h"
 #include "nsNodeInfoManager.h"
 #include "nsPIDOMWindow.h"
@@ -183,11 +184,6 @@ nsXULElement* nsXULElement::Construct(
     return new (nim) XULFrameElement(nodeInfo.forget());
   }
 
-  if (nodeInfo->Equals(nsGkAtoms::menubar)) {
-    auto* nim = nodeInfo->NodeInfoManager();
-    return new (nim) XULMenuParentElement(nodeInfo.forget());
-  }
-
   if (nodeInfo->Equals(nsGkAtoms::menu) ||
       nodeInfo->Equals(nsGkAtoms::menulist)) {
     auto* nim = nodeInfo->NodeInfoManager();
@@ -203,7 +199,6 @@ nsXULElement* nsXULElement::Construct(
       nodeInfo->Equals(nsGkAtoms::radio) ||
       nodeInfo->Equals(nsGkAtoms::thumb) ||
       nodeInfo->Equals(nsGkAtoms::button) ||
-      nodeInfo->Equals(nsGkAtoms::menuitem) ||
       nodeInfo->Equals(nsGkAtoms::toolbarbutton) ||
       nodeInfo->Equals(nsGkAtoms::toolbarpaletteitem) ||
       nodeInfo->Equals(nsGkAtoms::scrollbarbutton)) {
@@ -456,10 +451,8 @@ bool nsXULElement::IsFocusableInternal(int32_t* aTabIndex, bool aWithMouse) {
 }
 
 bool nsXULElement::HasMenu() {
-  if (auto* button = XULButtonElement::FromNode(this)) {
-    return button->IsMenu();
-  }
-  return false;
+  nsMenuFrame* menu = do_QueryFrame(GetPrimaryFrame(FlushType::Frames));
+  return !!menu;
 }
 
 void nsXULElement::OpenMenu(bool aOpenFlag) {
@@ -469,16 +462,14 @@ void nsXULElement::OpenMenu(bool aOpenFlag) {
   }
 
   nsXULPopupManager* pm = nsXULPopupManager::GetInstance();
-  if (!pm) {
-    return;
-  }
-
-  if (aOpenFlag) {
-    // Nothing will happen if this element isn't a menu.
-    pm->ShowMenu(this, false);
-  } else {
-    // Nothing will happen if this element isn't a menu.
-    pm->HideMenu(this);
+  if (pm) {
+    if (aOpenFlag) {
+      // Nothing will happen if this element isn't a menu.
+      pm->ShowMenu(this, false);
+    } else {
+      // Nothing will happen if this element isn't a menu.
+      pm->HideMenu(this);
+    }
   }
 }
 
@@ -1048,9 +1039,6 @@ void nsXULElement::ClickWithInputSource(uint16_t aInputSource,
                                  WidgetMouseEvent::eReal);
       WidgetMouseEvent eventUp(aIsTrustedEvent, eMouseUp, nullptr,
                                WidgetMouseEvent::eReal);
-      // This helps to avoid commands being dispatched from
-      // XULButtonElement::PostHandleEventForMenu.
-      eventUp.mFlags.mMultipleActionsPrevented = true;
       WidgetMouseEvent eventClick(aIsTrustedEvent, eMouseClick, nullptr,
                                   WidgetMouseEvent::eReal);
       eventDown.mInputSource = eventUp.mInputSource = eventClick.mInputSource =

dom/xul/nsXULElement.h

@@ -340,11 +340,6 @@ class nsXULElement : public nsStyledElement {
   NS_DECL_ISUPPORTS_INHERITED
   NS_DECL_CYCLE_COLLECTION_CLASS_INHERITED(nsXULElement, nsStyledElement)
 
-  // This doesn't work on XUL elements! You probably want
-  // GetXULBoolAttr(nsGkAtoms::disabled) or so.
-  // TODO(emilio): Maybe we should unify HTML and XUL here.
-  bool IsDisabled() const = delete;
-
   // nsINode
   void GetEventTargetParent(mozilla::EventChainPreVisitor& aVisitor) override;
   MOZ_CAN_RUN_SCRIPT_BOUNDARY

dom/xul/nsXULPopupListener.cpp

@@ -9,7 +9,6 @@
  */
 
 #include "nsXULPopupListener.h"
-#include "XULButtonElement.h"
 #include "nsCOMPtr.h"
 #include "nsGkAtoms.h"
 #include "nsContentCID.h"
@@ -38,6 +37,7 @@
 #include "nsPIDOMWindow.h"
 #include "nsViewManager.h"
 #include "nsError.h"
+#include "nsMenuFrame.h"
 
 using namespace mozilla;
 using namespace mozilla::dom;
@@ -248,16 +248,14 @@ nsresult nsXULPopupListener::LaunchPopup(MouseEvent* aEvent) {
   }
 
   // return if no popup was found or the popup is the element itself.
-  if (!popup || popup == mElement) {
-    return NS_OK;
-  }
+  if (!popup || popup == mElement) return NS_OK;
 
   // Submenus can't be used as context menus or popups, bug 288763.
   // Similar code also in nsXULTooltipListener::GetTooltipFor.
-  if (auto* button = XULButtonElement::FromNodeOrNull(popup->GetParent())) {
-    if (button->IsMenu()) {
-      return NS_OK;
-    }
+  nsIContent* parent = popup->GetParent();
+  if (parent) {
+    nsMenuFrame* menu = do_QueryFrame(parent->GetPrimaryFrame());
+    if (menu) return NS_OK;
   }
 
   nsXULPopupManager* pm = nsXULPopupManager::GetInstance();

layout/base/PresShell.cpp

@@ -150,6 +150,7 @@
 // For style data reconstruction
 #include "nsStyleChangeList.h"
 #include "nsCSSFrameConstructor.h"
+#include "nsMenuFrame.h"
 #include "nsTreeBodyFrame.h"
 #include "XULTreeElement.h"
 #include "nsMenuPopupFrame.h"
@@ -8850,11 +8851,12 @@ bool PresShell::EventHandler::AdjustContextMenuKeyEvent(
     WidgetMouseEvent* aMouseEvent) {
   // if a menu is open, open the context menu relative to the active item on the
   // menu.
-  if (nsXULPopupManager* pm = nsXULPopupManager::GetInstance()) {
+  nsXULPopupManager* pm = nsXULPopupManager::GetInstance();
+  if (pm) {
     nsIFrame* popupFrame = pm->GetTopPopup(ePopupTypeMenu);
     if (popupFrame) {
-      nsIFrame* itemFrame = (static_cast<nsMenuPopupFrame*>(popupFrame))
-                                ->GetCurrentMenuItemFrame();
+      nsIFrame* itemFrame =
+          (static_cast<nsMenuPopupFrame*>(popupFrame))->GetCurrentMenuItem();
       if (!itemFrame) itemFrame = popupFrame;
 
       nsCOMPtr<nsIWidget> widget = popupFrame->GetNearestWidget();

layout/base/nsCSSFrameConstructor.cpp

@@ -202,6 +202,7 @@ static FrameCtorDebugFlags gFlags[] = {
 #  define NUM_DEBUG_FLAGS (sizeof(gFlags) / sizeof(gFlags[0]))
 #endif
 
+#include "nsMenuFrame.h"
 #include "nsTreeColFrame.h"
 
 //------------------------------------------------------------------
@@ -4151,10 +4152,15 @@ nsCSSFrameConstructor::FindXULTagData(const Element& aElement,
       SIMPLE_XUL_CREATE(image, NS_NewImageBoxFrame),
       SIMPLE_XUL_CREATE(treechildren, NS_NewTreeBodyFrame),
       SIMPLE_XUL_CREATE(treecol, NS_NewTreeColFrame),
+      SIMPLE_TAG_CHAIN(button, nsCSSFrameConstructor::FindXULButtonData),
+      SIMPLE_TAG_CHAIN(toolbarbutton, nsCSSFrameConstructor::FindXULButtonData),
       SIMPLE_TAG_CHAIN(label,
                        nsCSSFrameConstructor::FindXULLabelOrDescriptionData),
       SIMPLE_TAG_CHAIN(description,
                        nsCSSFrameConstructor::FindXULLabelOrDescriptionData),
+      SIMPLE_XUL_CREATE(menu, NS_NewMenuFrame),
+      SIMPLE_XUL_CREATE(menulist, NS_NewMenuFrame),
+      SIMPLE_XUL_CREATE(menuitem, NS_NewMenuItemFrame),
 #ifdef XP_MACOSX
       SIMPLE_TAG_CHAIN(menubar, nsCSSFrameConstructor::FindXULMenubarData),
 #else
@@ -4175,6 +4181,19 @@ nsCSSFrameConstructor::FindXULTagData(const Element& aElement,
   return FindDataByTag(aElement, aStyle, sXULTagData, ArrayLength(sXULTagData));
 }
 
+/* static */
+const nsCSSFrameConstructor::FrameConstructionData*
+nsCSSFrameConstructor::FindXULButtonData(const Element& aElement,
+                                         ComputedStyle&) {
+  static constexpr FrameConstructionData sXULMenuData =
+      SIMPLE_XUL_FCDATA(NS_NewMenuFrame);
+  if (aElement.AttrValueIs(kNameSpaceID_None, nsGkAtoms::type, nsGkAtoms::menu,
+                           eCaseMatters)) {
+    return &sXULMenuData;
+  }
+  return nullptr;
+}
+
 /* static */
 const nsCSSFrameConstructor::FrameConstructionData*
 nsCSSFrameConstructor::FindXULLabelOrDescriptionData(const Element& aElement,
@@ -5419,7 +5438,9 @@ void nsCSSFrameConstructor::AddFrameConstructionItemsInternal(
     return;
   }
 
-  const bool isPopup = data->mBits & FCDATA_IS_POPUP;
+  const bool isPopup =
+      (data->mBits & FCDATA_IS_POPUP) && (!aParentFrame ||  // Parent is inline
+                                          !aParentFrame->IsMenuFrame());
 
   const uint32_t bits = data->mBits;
 
@@ -5911,13 +5932,17 @@ void nsCSSFrameConstructor::AppendFramesToParent(
 bool nsCSSFrameConstructor::IsValidSibling(nsIFrame* aSibling,
                                            nsIContent* aContent,
                                            Maybe<StyleDisplay>& aDisplay) {
+  nsIFrame* parentFrame = aSibling->GetParent();
+  LayoutFrameType parentType = parentFrame->Type();
+
   StyleDisplay siblingDisplay = aSibling->GetDisplay();
   if (StyleDisplay::TableColumnGroup == siblingDisplay ||
       StyleDisplay::TableColumn == siblingDisplay ||
       StyleDisplay::TableCaption == siblingDisplay ||
       StyleDisplay::TableHeaderGroup == siblingDisplay ||
       StyleDisplay::TableRowGroup == siblingDisplay ||
-      StyleDisplay::TableFooterGroup == siblingDisplay) {
+      StyleDisplay::TableFooterGroup == siblingDisplay ||
+      LayoutFrameType::Menu == parentType) {
     // if we haven't already, resolve a style to find the display type of
     // aContent.
     if (aDisplay.isNothing()) {

layout/base/nsLayoutUtils.cpp

@@ -822,7 +822,18 @@ FrameChildListID nsLayoutUtils::GetChildListNameFor(nsIFrame* aChildFrame) {
     }
   } else {
     LayoutFrameType childType = aChildFrame->Type();
-    if (LayoutFrameType::TableColGroup == childType) {
+    if (LayoutFrameType::MenuPopup == childType) {
+      nsIFrame* parent = aChildFrame->GetParent();
+      MOZ_ASSERT(parent, "nsMenuPopupFrame can't be the root frame");
+      MOZ_ASSERT(parent->IsMenuFrame(),
+                 "nsMenuPopupFrame should be out of flow if not under a menu");
+      nsIFrame* firstPopup =
+          parent->GetChildList(FrameChildListID::Popup).FirstChild();
+      MOZ_ASSERT(!firstPopup || !firstPopup->GetNextSibling(),
+                 "We assume popupList only has one child, but it has more.");
+      id = firstPopup == aChildFrame ? FrameChildListID::Popup
+                                     : FrameChildListID::Principal;
+    } else if (LayoutFrameType::TableColGroup == childType) {
       id = FrameChildListID::ColGroup;
     } else if (aChildFrame->IsTableCaption()) {
       id = FrameChildListID::Caption;

layout/base/tests/chrome/test_bug420499.xhtml

@@ -9,7 +9,7 @@ https://bugzilla.mozilla.org/show_bug.cgi?id=420499
   <script src="chrome://mochikit/content/tests/SimpleTest/SimpleTest.js"/>
   <script src="chrome://mochikit/content/tests/SimpleTest/EventUtils.js"></script>
 
-
+  
 
     <menu id="menu" label="Menu">
       <menupopup id="file-popup">
@@ -37,10 +37,10 @@ https://bugzilla.mozilla.org/show_bug.cgi?id=420499
         <description value="This is a tooltip"/>
       </tooltip>
     </popupset>
-
+  
   <!-- test results are displayed in the html:body -->
   <body xmlns="http://www.w3.org/1999/xhtml" bgcolor="white">
-
+  
   <p id="par1">Paragraph 1</p>
   <p id="par2">Paragraph 2</p>
   <p id="par3">Paragraph 3</p>
@@ -55,7 +55,7 @@ https://bugzilla.mozilla.org/show_bug.cgi?id=420499
 
   <!-- test code goes here -->
   <script type="application/javascript"><![CDATA[
-
+    
     /** Test for Bug 420499 **/
     SimpleTest.waitForExplicitFinish();
 
@@ -73,7 +73,7 @@ https://bugzilla.mozilla.org/show_bug.cgi?id=420499
         .QueryInterface(Ci.nsISelectionController);
       return selCon.caretVisible;
     }
-
+    
     function focusInput() {
       ok(!isCaretVisible(), "Caret shouldn't be visible");
       $("text-input").focus();
@@ -81,14 +81,14 @@ https://bugzilla.mozilla.org/show_bug.cgi?id=420499
       window.addEventListener("popupshown", popupMenuShownHandler);
       $("menu").open = true;
     }
-
+    
     function popupMenuShownHandler() {
       window.removeEventListener("popupshown", popupMenuShownHandler);
       ok(!isCaretVisible(), "Caret shouldn't be visible when menu open");
       window.addEventListener("popuphidden", ensureParagraphFocused);
       $("menu").open = false;
     }
-
+    
     function ensureParagraphFocused() {
       window.removeEventListener("popuphidden", ensureParagraphFocused);
       ok(isCaretVisible(), "Caret should have returned to previous focus");

layout/generic/FrameClasses.py

@@ -64,6 +64,7 @@ FRAME_CLASSES = [
     Frame("nsMathMLsemanticsFrame", "None", NOT_LEAF),
     Frame("nsMathMLTokenFrame", "None", NOT_LEAF),
     Frame("nsMenuBarFrame", "Box", NOT_LEAF),
+    Frame("nsMenuFrame", "Menu", NOT_LEAF),
     Frame("nsMenuPopupFrame", "MenuPopup", NOT_LEAF),
     Frame("nsMeterFrame", "Meter", LEAF),
     Frame("nsNumberControlFrame", "TextInput", LEAF),

layout/generic/test/test_bug632379.xhtml

@@ -17,7 +17,7 @@ https://bugzilla.mozilla.org/show_bug.cgi?id=632379
 <toolbox flex="1">
   <menubar>
     <menu label="MENU" accesskey="m" id="mainMenu">
-      <menupopup maxheight="100" onpopupshown="openSubmenu(this, event)">
+      <menupopup maxheight="100" onpopupshown="openSubmenu()">
         <menu label="menu1" accesskey="1" id="menu1">
           <menupopup onpopupshown="snapshot(this)">
             <menuitem label="item"/>
@@ -178,7 +178,7 @@ var count=0;
 
 function snapshot(elem)
 {
-    info(`snapshot(${elem.parentNode.id}) called`);
+    info("snapshot() called");
     pos[count] = elem.getBoundingClientRect().top;
     info(elem.querySelector("menuitem").getBoundingClientRect().height);
     ++count;
@@ -204,12 +204,11 @@ function doTest() {
     $("mainMenu").open = true;
 }
 
-function openSubmenu(mainMenu, e)
+function openSubmenu()
 {
-    if (e.originalTarget != mainMenu) {
-        return;
+    if (!gFinished) {
+        info("openSubmenu() called");
     }
-    info("openSubmenu() called");
     // open a submenu in the middle
     sendString("5");
 }

layout/xul/moz.build

@@ -27,6 +27,7 @@ UNIFIED_SOURCES += [
     "nsLeafBoxFrame.cpp",
     "nsMenuBarFrame.cpp",
     "nsMenuBarListener.cpp",
+    "nsMenuFrame.cpp",
     "nsMenuPopupFrame.cpp",
     "nsRepeatService.cpp",
     "nsScrollbarButtonFrame.cpp",

layout/xul/nsMenuBarFrame.cpp

@@ -5,13 +5,13 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "nsMenuBarFrame.h"
-#include "mozilla/BasicEvents.h"
 #include "nsIContent.h"
 #include "nsAtom.h"
 #include "nsPresContext.h"
 #include "nsCSSRendering.h"
 #include "nsNameSpaceManager.h"
 #include "nsGkAtoms.h"
+#include "nsMenuFrame.h"
 #include "nsMenuPopupFrame.h"
 #include "nsUnicharUtils.h"
 #include "nsPIDOMWindow.h"
@@ -24,11 +24,13 @@
 #include "nsUTF8Utils.h"
 #include "mozilla/ComputedStyle.h"
 #include "mozilla/PresShell.h"
+#include "mozilla/TextEvents.h"
 #include "mozilla/dom/Document.h"
-#include "mozilla/dom/XULMenuParentElement.h"
-#include "mozilla/dom/XULButtonElement.h"
+#include "mozilla/dom/Event.h"
+#include "mozilla/dom/KeyboardEvent.h"
 
 using namespace mozilla;
+using mozilla::dom::KeyboardEvent;
 
 //
 // NS_NewMenuBarFrame
@@ -50,7 +52,11 @@ NS_QUERYFRAME_TAIL_INHERITING(nsBoxFrame)
 //
 nsMenuBarFrame::nsMenuBarFrame(ComputedStyle* aStyle,
                                nsPresContext* aPresContext)
-    : nsBoxFrame(aStyle, aPresContext, kClassID) {}
+    : nsBoxFrame(aStyle, aPresContext, kClassID),
+      mStayActive(false),
+      mIsActive(false),
+      mActiveByKeyboard(false),
+      mCurrentMenu(nullptr) {}  // cntr
 
 void nsMenuBarFrame::Init(nsIContent* aContent, nsContainerFrame* aParent,
                           nsIFrame* aPrevInFlow) {
@@ -60,27 +66,20 @@ void nsMenuBarFrame::Init(nsIContent* aContent, nsContainerFrame* aParent,
   mMenuBarListener = new nsMenuBarListener(this, aContent);
 }
 
-dom::XULMenuParentElement& nsMenuBarFrame::MenubarElement() const {
-  auto* content = dom::XULMenuParentElement::FromNode(GetContent());
-  MOZ_DIAGNOSTIC_ASSERT(content);
-  return *content;
-}
-
-MOZ_CAN_RUN_SCRIPT void nsMenuBarFrame::SetActive(bool aActiveFlag) {
+NS_IMETHODIMP
+nsMenuBarFrame::SetActive(bool aActiveFlag) {
   // If the activity is not changed, there is nothing to do.
-  if (mIsActive == aActiveFlag) {
-    return;
-  }
+  if (mIsActive == aActiveFlag) return NS_OK;
 
   if (!aActiveFlag) {
-    // If there is a request to deactivate the menu bar, check to see whether
+    // Don't deactivate when switching between menus on the menubar.
+    if (mStayActive) return NS_OK;
+
+    // if there is a request to deactivate the menu bar, check to see whether
     // there is a menu popup open for the menu bar. In this case, don't
     // deactivate the menu bar.
-    if (auto* activeChild = MenubarElement().GetActiveMenuChild()) {
-      if (activeChild->IsMenuPopupOpen()) {
-        return;
-      }
-    }
+    nsXULPopupManager* pm = nsXULPopupManager::GetInstance();
+    if (pm && pm->IsPopupOpenForMenuParent(this)) return NS_OK;
   }
 
   mIsActive = aActiveFlag;
@@ -91,36 +90,257 @@ MOZ_CAN_RUN_SCRIPT void nsMenuBarFrame::SetActive(bool aActiveFlag) {
     RemoveKeyboardNavigator();
   }
 
-  RefPtr menubar = &MenubarElement();
-  if (!aActiveFlag) {
-    menubar->SetActiveMenuChild(nullptr);
-  }
-
   constexpr auto active = u"DOMMenuBarActive"_ns;
   constexpr auto inactive = u"DOMMenuBarInactive"_ns;
-  FireDOMEvent(aActiveFlag ? active : inactive, menubar);
+
+  FireDOMEvent(mIsActive ? active : inactive, mContent);
+
+  return NS_OK;
+}
+
+nsMenuFrame* nsMenuBarFrame::ToggleMenuActiveState() {
+  if (mIsActive) {
+    // Deactivate the menu bar
+    SetActive(false);
+    if (mCurrentMenu) {
+      nsMenuFrame* closeframe = mCurrentMenu;
+      closeframe->SelectMenu(false);
+      mCurrentMenu = nullptr;
+      return closeframe;
+    }
+  } else {
+    // if the menu bar is already selected (eg. mouseover), deselect it
+    if (mCurrentMenu) mCurrentMenu->SelectMenu(false);
+
+    // Set the active menu to be the top left item (e.g., the File menu).
+    // We use an attribute called "menuactive" to track the current
+    // active menu.
+    nsMenuFrame* firstFrame =
+        nsXULPopupManager::GetNextMenuItem(this, nullptr, false, false);
+    if (firstFrame) {
+      // Activate the menu bar
+      SetActive(true);
+      firstFrame->SelectMenu(true);
+
+      // Track this item for keyboard navigation.
+      mCurrentMenu = firstFrame;
+    }
+  }
+
+  return nullptr;
+}
+
+nsMenuFrame* nsMenuBarFrame::FindMenuWithShortcut(KeyboardEvent* aKeyEvent,
+                                                  bool aPeek) {
+  uint32_t charCode = aKeyEvent->CharCode();
+
+  AutoTArray<uint32_t, 10> accessKeys;
+  WidgetKeyboardEvent* nativeKeyEvent =
+      aKeyEvent->WidgetEventPtr()->AsKeyboardEvent();
+  if (nativeKeyEvent) {
+    nativeKeyEvent->GetAccessKeyCandidates(accessKeys);
+  }
+  if (accessKeys.IsEmpty() && charCode) accessKeys.AppendElement(charCode);
+
+  if (accessKeys.IsEmpty())
+    return nullptr;  // no character was pressed so just return
+
+  // Enumerate over our list of frames.
+  nsContainerFrame* immediateParent =
+      nsXULPopupManager::ImmediateParentFrame(this);
+
+  // Find a most preferred accesskey which should be returned.
+  nsIFrame* foundMenu = nullptr;
+  size_t foundIndex = accessKeys.NoIndex;
+  nsIFrame* currFrame = immediateParent->PrincipalChildList().FirstChild();
+
+  while (currFrame) {
+    nsIContent* current = currFrame->GetContent();
+
+    // See if it's a menu item.
+    if (nsXULPopupManager::IsValidMenuItem(current, false)) {
+      // Get the shortcut attribute.
+      nsAutoString shortcutKey;
+      if (current->IsElement()) {
+        current->AsElement()->GetAttr(kNameSpaceID_None, nsGkAtoms::accesskey,
+                                      shortcutKey);
+      }
+      if (!shortcutKey.IsEmpty()) {
+        ToLowerCase(shortcutKey);
+        const char16_t* start = shortcutKey.BeginReading();
+        const char16_t* end = shortcutKey.EndReading();
+        uint32_t ch = UTF16CharEnumerator::NextChar(&start, end);
+        size_t index = accessKeys.IndexOf(ch);
+        if (index != accessKeys.NoIndex &&
+            (foundIndex == accessKeys.NoIndex || index < foundIndex)) {
+          foundMenu = currFrame;
+          foundIndex = index;
+        }
+      }
+    }
+    currFrame = currFrame->GetNextSibling();
+  }
+  if (foundMenu) {
+    return do_QueryFrame(foundMenu);
+  }
+
+  // didn't find a matching menu item
+#ifdef XP_WIN
+  if (!aPeek) {
+    // behavior on Windows - this item is on the menu bar, beep and deactivate
+    // the menu bar
+    if (mIsActive) {
+      nsCOMPtr<nsISound> soundInterface = do_GetService("@mozilla.org/sound;1");
+      if (soundInterface) soundInterface->Beep();
+    }
+
+    nsXULPopupManager* pm = nsXULPopupManager::GetInstance();
+    if (pm) {
+      nsIFrame* popup = pm->GetTopPopup(ePopupTypeMenu);
+      if (popup) pm->HidePopup(popup->GetContent(), true, true, true, false);
+    }
+
+    SetCurrentMenuItem(nullptr);
+    SetActive(false);
+  }
+
+#endif  // #ifdef XP_WIN
+
+  return nullptr;
+}
+
+/* virtual */
+nsMenuFrame* nsMenuBarFrame::GetCurrentMenuItem() { return mCurrentMenu; }
+
+NS_IMETHODIMP
+nsMenuBarFrame::SetCurrentMenuItem(nsMenuFrame* aMenuItem) {
+  if (mCurrentMenu == aMenuItem) return NS_OK;
+
+  if (mCurrentMenu) mCurrentMenu->SelectMenu(false);
+
+  if (aMenuItem) aMenuItem->SelectMenu(true);
+
+  mCurrentMenu = aMenuItem;
+
+  return NS_OK;
+}
+
+void nsMenuBarFrame::CurrentMenuIsBeingDestroyed() {
+  mCurrentMenu->SelectMenu(false);
+  mCurrentMenu = nullptr;
+}
+
+class nsMenuBarSwitchMenu : public Runnable {
+ public:
+  nsMenuBarSwitchMenu(nsIContent* aMenuBar, nsIContent* aOldMenu,
+                      nsIContent* aNewMenu, bool aSelectFirstItem)
+      : mozilla::Runnable("nsMenuBarSwitchMenu"),
+        mMenuBar(aMenuBar),
+        mOldMenu(aOldMenu),
+        mNewMenu(aNewMenu),
+        mSelectFirstItem(aSelectFirstItem) {}
+
+  NS_IMETHOD Run() override {
+    nsXULPopupManager* pm = nsXULPopupManager::GetInstance();
+    if (!pm) return NS_ERROR_UNEXPECTED;
+
+    // if switching from one menu to another, set a flag so that the call to
+    // HidePopup doesn't deactivate the menubar when the first menu closes.
+    nsMenuBarFrame* menubar = nullptr;
+    if (mOldMenu && mNewMenu) {
+      menubar = do_QueryFrame(mMenuBar->GetPrimaryFrame());
+      if (menubar) menubar->SetStayActive(true);
+    }
+
+    if (mOldMenu) {
+      AutoWeakFrame weakMenuBar(menubar);
+      pm->HidePopup(mOldMenu, false, false, false, false);
+      // clear the flag again
+      if (mNewMenu && weakMenuBar.IsAlive()) menubar->SetStayActive(false);
+    }
+
+    if (mNewMenu) {
+      pm->ShowMenu(mNewMenu, mSelectFirstItem);
+    }
+
+    return NS_OK;
+  }
+
+ private:
+  nsCOMPtr<nsIContent> mMenuBar;
+  nsCOMPtr<nsIContent> mOldMenu;
+  nsCOMPtr<nsIContent> mNewMenu;
+  bool mSelectFirstItem;
+};
+
+NS_IMETHODIMP
+nsMenuBarFrame::ChangeMenuItem(nsMenuFrame* aMenuItem, bool aSelectFirstItem,
+                               bool aFromKey) {
+  if (mCurrentMenu == aMenuItem) return NS_OK;
+
+  // check if there's an open context menu, we ignore this
+  nsXULPopupManager* pm = nsXULPopupManager::GetInstance();
+  if (pm && pm->HasContextMenu(nullptr)) return NS_OK;
+
+  nsIContent* aOldMenu = nullptr;
+  nsIContent* aNewMenu = nullptr;
+
+  // Unset the current child.
+  bool wasOpen = false;
+  if (mCurrentMenu) {
+    wasOpen = mCurrentMenu->IsOpen();
+    mCurrentMenu->SelectMenu(false);
+    if (wasOpen) {
+      nsMenuPopupFrame* popupFrame = mCurrentMenu->GetPopup();
+      if (popupFrame) aOldMenu = popupFrame->GetContent();
+    }
+  }
+
+  // set to null first in case the IsAlive check below returns false
+  mCurrentMenu = nullptr;
+
+  // Set the new child.
+  if (aMenuItem) {
+    nsCOMPtr<nsIContent> content = aMenuItem->GetContent();
+    aMenuItem->SelectMenu(true);
+    mCurrentMenu = aMenuItem;
+    if (wasOpen && !aMenuItem->IsDisabled()) aNewMenu = content;
+  }
+
+  // use an event so that hiding and showing can be done synchronously, which
+  // avoids flickering
+  nsCOMPtr<nsIRunnable> event = new nsMenuBarSwitchMenu(
+      GetContent(), aOldMenu, aNewMenu, aSelectFirstItem);
+  return mContent->OwnerDoc()->Dispatch(TaskCategory::Other, event.forget());
+}
+
+nsMenuFrame* nsMenuBarFrame::Enter(WidgetGUIEvent* aEvent) {
+  if (!mCurrentMenu) return nullptr;
+
+  if (mCurrentMenu->IsOpen()) return mCurrentMenu->Enter(aEvent);
+
+  return mCurrentMenu;
+}
+
+bool nsMenuBarFrame::MenuClosed() {
+  SetActive(false);
+  if (!mIsActive && mCurrentMenu) {
+    mCurrentMenu->SelectMenu(false);
+    mCurrentMenu = nullptr;
+    return true;
+  }
+  return false;
 }
 
 void nsMenuBarFrame::InstallKeyboardNavigator() {
-  if (nsXULPopupManager* pm = nsXULPopupManager::GetInstance()) {
-    pm->SetActiveMenuBar(this, true);
-  }
-}
-
-void nsMenuBarFrame::MenuClosed() { SetActive(false); }
-
-void nsMenuBarFrame::HandleEnterKeyPress(WidgetEvent& aEvent) {
-  if (RefPtr<dom::XULButtonElement> activeChild =
-          MenubarElement().GetActiveMenuChild()) {
-    activeChild->HandleEnterKeyPress(aEvent);
-  }
+  nsXULPopupManager* pm = nsXULPopupManager::GetInstance();
+  if (pm) pm->SetActiveMenuBar(this, true);
 }
 
 void nsMenuBarFrame::RemoveKeyboardNavigator() {
   if (!mIsActive) {
-    if (nsXULPopupManager* pm = nsXULPopupManager::GetInstance()) {
-      pm->SetActiveMenuBar(this, false);
-    }
+    nsXULPopupManager* pm = nsXULPopupManager::GetInstance();
+    if (pm) pm->SetActiveMenuBar(this, false);
   }
 }
 

layout/xul/nsMenuBarFrame.h

@@ -11,10 +11,13 @@
 #ifndef nsMenuBarFrame_h__
 #define nsMenuBarFrame_h__
 
+#include "mozilla/Attributes.h"
 #include "nsAtom.h"
 #include "nsCOMPtr.h"
 #include "nsBoxFrame.h"
+#include "nsMenuFrame.h"
 #include "nsMenuBarListener.h"
+#include "nsMenuParent.h"
 
 class nsIContent;
 
@@ -22,42 +25,79 @@ namespace mozilla {
 class PresShell;
 namespace dom {
 class KeyboardEvent;
-class XULMenuParentElement;
 }  // namespace dom
 }  // namespace mozilla
 
 nsIFrame* NS_NewMenuBarFrame(mozilla::PresShell* aPresShell,
                              mozilla::ComputedStyle* aStyle);
 
-class nsMenuBarFrame final : public nsBoxFrame {
+class nsMenuBarFrame final : public nsBoxFrame, public nsMenuParent {
  public:
   NS_DECL_QUERYFRAME
   NS_DECL_FRAMEARENA_HELPERS(nsMenuBarFrame)
 
   explicit nsMenuBarFrame(ComputedStyle* aStyle, nsPresContext* aPresContext);
 
+  // nsMenuParent interface
+  virtual nsMenuFrame* GetCurrentMenuItem() override;
+  NS_IMETHOD SetCurrentMenuItem(nsMenuFrame* aMenuItem) override;
+  virtual void CurrentMenuIsBeingDestroyed() override;
+  NS_IMETHOD ChangeMenuItem(nsMenuFrame* aMenuItem, bool aSelectFirstItem,
+                            bool aFromKey) override;
+
+  NS_IMETHOD SetActive(bool aActiveFlag) override;
+
+  virtual bool IsMenuBar() override { return true; }
+  virtual bool IsContextMenu() override { return false; }
+  virtual bool IsActive() override { return mIsActive; }
+  virtual bool IsMenu() override { return false; }
+  virtual bool IsOpen() override {
+    // menubars are considered always open
+    return true;
+  }
+
+  bool IsMenuOpen() { return mCurrentMenu && mCurrentMenu->IsOpen(); }
+
   void InstallKeyboardNavigator();
   void RemoveKeyboardNavigator();
-  MOZ_CAN_RUN_SCRIPT void MenuClosed();
 
-  void Init(nsIContent* aContent, nsContainerFrame* aParent,
-            nsIFrame* aPrevInFlow) override;
+  virtual void Init(nsIContent* aContent, nsContainerFrame* aParent,
+                    nsIFrame* aPrevInFlow) override;
 
-  void DestroyFrom(nsIFrame* aDestructRoot,
-                   PostDestroyData& aPostDestroyData) override;
+  virtual void DestroyFrom(nsIFrame* aDestructRoot,
+                           PostDestroyData& aPostDestroyData) override;
+
+  virtual void LockMenuUntilClosed(bool aLock) override {}
+  virtual bool IsMenuLocked() override { return false; }
+
+  // Non-interface helpers
+
+  // The 'stay active' flag is set when navigating from one top-level menu
+  // to another, to prevent the menubar from deactivating and submenus from
+  // firing extra DOMMenuItemActive events.
+  bool GetStayActive() { return mStayActive; }
+  void SetStayActive(bool aStayActive) { mStayActive = aStayActive; }
+
+  // Called when a menu on the menu bar is clicked on. Returns a menu if one
+  // needs to be closed.
+  nsMenuFrame* ToggleMenuActiveState();
 
   bool IsActiveByKeyboard() { return mActiveByKeyboard; }
   void SetActiveByKeyboard() { mActiveByKeyboard = true; }
-  MOZ_CAN_RUN_SCRIPT void SetActive(bool aActive);
-  bool IsActive() const { return mIsActive; }
 
-  mozilla::dom::XULMenuParentElement& MenubarElement() const;
+  // indicate that a menu on the menubar was closed. Returns true if the caller
+  // may deselect the menuitem.
+  virtual bool MenuClosed() override;
 
   // Called when Enter is pressed while the menubar is focused. If the current
   // menu is open, let the child handle the key.
-  MOZ_CAN_RUN_SCRIPT void HandleEnterKeyPress(mozilla::WidgetEvent&);
+  nsMenuFrame* Enter(mozilla::WidgetGUIEvent* aEvent);
 
-  bool IsFrameOfType(uint32_t aFlags) const override {
+  // Used to handle ALT+key combos
+  nsMenuFrame* FindMenuWithShortcut(mozilla::dom::KeyboardEvent* aKeyEvent,
+                                    bool aPeek);
+
+  virtual bool IsFrameOfType(uint32_t aFlags) const override {
     // Override bogus IsFrameOfType in nsBoxFrame.
     if (aFlags & (nsIFrame::eReplacedContainsBlock | nsIFrame::eReplaced))
       return false;
@@ -65,7 +105,7 @@ class nsMenuBarFrame final : public nsBoxFrame {
   }
 
 #ifdef DEBUG_FRAME_DUMP
-  nsresult GetFrameName(nsAString& aResult) const override {
+  virtual nsresult GetFrameName(nsAString& aResult) const override {
     return MakeFrameName(u"MenuBar"_ns, aResult);
   }
 #endif
@@ -74,10 +114,19 @@ class nsMenuBarFrame final : public nsBoxFrame {
   RefPtr<nsMenuBarListener> mMenuBarListener;  // The listener that tells us
                                                // about key and mouse events.
 
-  bool mIsActive = false;  // Whether or not the menu bar is active (a menu item
-                           // is highlighted or shown).
-  // Whether the menubar was made active via the keyboard.
-  bool mActiveByKeyboard = false;
+  // flag that is temporarily set when switching from one menu on the menubar to
+  // another to indicate that the menubar should not be deactivated.
+  bool mStayActive;
+
+  bool mIsActive;  // Whether or not the menu bar is active (a menu item is
+                   // highlighted or shown).
+
+  // whether the menubar was made active via the keyboard.
+  bool mActiveByKeyboard;
+
+  // The current menu that is active (highlighted), which may not be open. This
+  // will be null if no menu is active.
+  nsMenuFrame* mCurrentMenu;
 };  // class nsMenuBarFrame
 
 #endif

layout/xul/nsMenuBarListener.cpp

@@ -5,13 +5,9 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "nsMenuBarListener.h"
-#include "XULButtonElement.h"
-#include "mozilla/Attributes.h"
-#include "mozilla/dom/XULButtonElement.h"
 #include "nsMenuBarFrame.h"
 #include "nsMenuPopupFrame.h"
 #include "nsPIWindowRoot.h"
-#include "nsISound.h"
 
 // Drag & Drop, Clipboard
 #include "nsWidgetsCID.h"
@@ -28,8 +24,6 @@
 #include "mozilla/dom/EventBinding.h"
 #include "mozilla/dom/KeyboardEvent.h"
 #include "mozilla/dom/KeyboardEventBinding.h"
-#include "mozilla/dom/XULMenuParentElement.h"
-#include "nsXULPopupManager.h"
 
 using namespace mozilla;
 using mozilla::dom::Event;
@@ -49,13 +43,12 @@ Modifiers nsMenuBarListener::mAccessKeyMask = 0;
 nsMenuBarListener::nsMenuBarListener(nsMenuBarFrame* aMenuBarFrame,
                                      nsIContent* aMenuBarContent)
     : mMenuBarFrame(aMenuBarFrame),
-      mContent(dom::XULMenuParentElement::FromNode(aMenuBarContent)),
-      mEventTarget(aMenuBarContent->GetComposedDoc()),
+      mEventTarget(aMenuBarContent ? aMenuBarContent->GetComposedDoc()
+                                   : nullptr),
       mTopWindowEventTarget(nullptr),
       mAccessKeyDown(false),
       mAccessKeyDownCanceled(false) {
   MOZ_ASSERT(mEventTarget);
-  MOZ_ASSERT(mContent);
 
   // Hook up the menubar as a key listener on the whole document.  This will
   // see every keypress that occurs, but after everyone else does.
@@ -159,12 +152,12 @@ void nsMenuBarListener::InitAccessKey() {
 }
 
 void nsMenuBarListener::ToggleMenuActiveState() {
-  if (mMenuBarFrame->IsActive()) {
-    mMenuBarFrame->SetActive(false);
-  } else {
-    RefPtr content = mContent;
-    mMenuBarFrame->SetActive(true);
-    content->SelectFirstItem();
+  nsMenuFrame* closemenu = mMenuBarFrame->ToggleMenuActiveState();
+  nsXULPopupManager* pm = nsXULPopupManager::GetInstance();
+  if (pm && closemenu) {
+    nsMenuPopupFrame* popupFrame = closemenu->GetPopup();
+    if (popupFrame)
+      pm->HidePopup(popupFrame->GetContent(), false, false, true, false);
   }
 }
 
@@ -206,7 +199,8 @@ nsresult nsMenuBarListener::KeyUp(Event* aKeyEvent) {
       // First, close all existing popups because other popups shouldn't
       // handle key events when menubar is active and IME should be
       // disabled.
-      if (nsXULPopupManager* pm = nsXULPopupManager::GetInstance()) {
+      nsXULPopupManager* pm = nsXULPopupManager::GetInstance();
+      if (pm) {
         pm->Rollup(0, false, nullptr, nullptr);
       }
       // If menubar active state is changed or the menubar is destroyed
@@ -266,7 +260,7 @@ nsresult nsMenuBarListener::KeyPress(Event* aKeyEvent) {
 #ifndef XP_MACOSX
     // Need to handle F10 specially on Non-Mac platform.
     if (nativeKeyEvent->mMessage == eKeyPress && keyCode == NS_VK_F10) {
-      if ((GetModifiersForAccessKey(*keyEvent) & ~MODIFIER_CONTROL) == 0) {
+      if ((GetModifiersForAccessKey(keyEvent) & ~MODIFIER_CONTROL) == 0) {
         // If the keyboard event should activate the menubar and will be
         // sent to a remote process, it should be executed with reply
         // event from the focused remote process.  Note that if the menubar
@@ -285,9 +279,8 @@ nsresult nsMenuBarListener::KeyPress(Event* aKeyEvent) {
 
         if (mMenuBarFrame->IsActive()) {
 #  ifdef MOZ_WIDGET_GTK
-          RefPtr child = mContent->GetActiveMenuChild();
           // In GTK, this also opens the first menu.
-          child->OpenMenuPopup(false);
+          mMenuBarFrame->GetCurrentMenuItem()->OpenMenu(false);
 #  endif
           aKeyEvent->StopPropagation();
           aKeyEvent->PreventDefault();
@@ -298,20 +291,8 @@ nsresult nsMenuBarListener::KeyPress(Event* aKeyEvent) {
     }
 #endif  // !XP_MACOSX
 
-    RefPtr menuForKey = GetMenuForKeyEvent(*keyEvent);
-    if (!menuForKey) {
-#ifdef XP_WIN
-      // Behavior on Windows - this item is on the menu bar, beep and deactivate
-      // the menu bar.
-      // TODO(emilio): This is rather odd, and I cannot get the beep to work,
-      // but this matches what old code was doing...
-      if (mMenuBarFrame->IsActive()) {
-        if (nsCOMPtr<nsISound> sound = do_GetService("@mozilla.org/sound;1")) {
-          sound->Beep();
-        }
-        mMenuBarFrame->SetActive(false);
-      }
-#endif
+    nsMenuFrame* menuFrameForKey = GetMenuForKeyEvent(keyEvent, false);
+    if (!menuFrameForKey) {
       return NS_OK;
     }
 
@@ -329,7 +310,7 @@ nsresult nsMenuBarListener::KeyPress(Event* aKeyEvent) {
 
     mMenuBarFrame->SetActiveByKeyboard();
     mMenuBarFrame->SetActive(true);
-    menuForKey->OpenMenuPopup(true);
+    menuFrameForKey->OpenMenu(true);
 
     // The opened menu will listen next keyup event.
     // Therefore, we should clear the keydown flags here.
@@ -342,7 +323,7 @@ nsresult nsMenuBarListener::KeyPress(Event* aKeyEvent) {
   return NS_OK;
 }
 
-bool nsMenuBarListener::IsAccessKeyPressed(KeyboardEvent& aKeyEvent) {
+bool nsMenuBarListener::IsAccessKeyPressed(KeyboardEvent* aKeyEvent) {
   InitAccessKey();
   // No other modifiers are allowed to be down except for Shift.
   uint32_t modifiers = GetModifiersForAccessKey(aKeyEvent);
@@ -352,39 +333,41 @@ bool nsMenuBarListener::IsAccessKeyPressed(KeyboardEvent& aKeyEvent) {
 }
 
 Modifiers nsMenuBarListener::GetModifiersForAccessKey(
-    KeyboardEvent& aKeyEvent) {
-  WidgetInputEvent* inputEvent = aKeyEvent.WidgetEventPtr()->AsInputEvent();
+    KeyboardEvent* aKeyEvent) {
+  WidgetInputEvent* inputEvent = aKeyEvent->WidgetEventPtr()->AsInputEvent();
   MOZ_ASSERT(inputEvent);
 
   static const Modifiers kPossibleModifiersForAccessKey =
       (MODIFIER_SHIFT | MODIFIER_CONTROL | MODIFIER_ALT | MODIFIER_META |
        MODIFIER_OS);
-  return inputEvent->mModifiers & kPossibleModifiersForAccessKey;
+  return (inputEvent->mModifiers & kPossibleModifiersForAccessKey);
 }
 
-dom::XULButtonElement* nsMenuBarListener::GetMenuForKeyEvent(
-    KeyboardEvent& aKeyEvent) {
+nsMenuFrame* nsMenuBarListener::GetMenuForKeyEvent(KeyboardEvent* aKeyEvent,
+                                                   bool aPeek) {
   if (!IsAccessKeyPressed(aKeyEvent)) {
     return nullptr;
   }
 
-  uint32_t charCode = aKeyEvent.CharCode();
+  uint32_t charCode = aKeyEvent->CharCode();
   bool hasAccessKeyCandidates = charCode != 0;
   if (!hasAccessKeyCandidates) {
     WidgetKeyboardEvent* nativeKeyEvent =
-        aKeyEvent.WidgetEventPtr()->AsKeyboardEvent();
+        aKeyEvent->WidgetEventPtr()->AsKeyboardEvent();
+
     AutoTArray<uint32_t, 10> keys;
     nativeKeyEvent->GetAccessKeyCandidates(keys);
     hasAccessKeyCandidates = !keys.IsEmpty();
   }
 
-  if (!hasAccessKeyCandidates) {
-    return nullptr;
+  if (hasAccessKeyCandidates) {
+    // Do shortcut navigation.
+    // A letter was pressed. We want to see if a shortcut gets matched. If
+    // so, we'll know the menu got activated.
+    return mMenuBarFrame->FindMenuWithShortcut(aKeyEvent, aPeek);
   }
-  // Do shortcut navigation.
-  // A letter was pressed. We want to see if a shortcut gets matched. If
-  // so, we'll know the menu got activated.
-  return mMenuBarFrame->MenubarElement().FindMenuWithShortcut(aKeyEvent);
+
+  return nullptr;
 }
 
 void nsMenuBarListener::ReserveKeyIfNeeded(Event* aKeyEvent) {
@@ -416,7 +399,7 @@ nsresult nsMenuBarListener::KeyDown(Event* aKeyEvent) {
 
 #ifndef XP_MACOSX
   if (capturing && !mAccessKeyDown && theChar == NS_VK_F10 &&
-      (GetModifiersForAccessKey(*keyEvent) & ~MODIFIER_CONTROL) == 0) {
+      (GetModifiersForAccessKey(keyEvent) & ~MODIFIER_CONTROL) == 0) {
     ReserveKeyIfNeeded(aKeyEvent);
   }
 #endif
@@ -429,7 +412,7 @@ nsresult nsMenuBarListener::KeyDown(Event* aKeyEvent) {
     // enhanced 102-key keyboards if we don't check this.
     bool isAccessKeyDownEvent =
         ((theChar == (uint32_t)mAccessKey) &&
-         (GetModifiersForAccessKey(*keyEvent) & ~mAccessKeyMask) == 0);
+         (GetModifiersForAccessKey(keyEvent) & ~mAccessKeyMask) == 0);
 
     if (!capturing && !mAccessKeyDown) {
       // If accesskey isn't being pressed and the key isn't the accesskey,
@@ -457,7 +440,8 @@ nsresult nsMenuBarListener::KeyDown(Event* aKeyEvent) {
   }
 
   if (capturing && mAccessKey) {
-    if (GetMenuForKeyEvent(*keyEvent)) {
+    nsMenuFrame* menuFrameForKey = GetMenuForKeyEvent(keyEvent, true);
+    if (menuFrameForKey) {
       ReserveKeyIfNeeded(aKeyEvent);
     }
   }
@@ -468,7 +452,7 @@ nsresult nsMenuBarListener::KeyDown(Event* aKeyEvent) {
 ////////////////////////////////////////////////////////////////////////
 
 nsresult nsMenuBarListener::Blur(Event* aEvent) {
-  if (!IsMenuOpen() && mMenuBarFrame->IsActive()) {
+  if (!mMenuBarFrame->IsMenuOpen() && mMenuBarFrame->IsActive()) {
     ToggleMenuActiveState();
     mAccessKeyDown = false;
     mAccessKeyDownCanceled = false;
@@ -486,11 +470,6 @@ nsresult nsMenuBarListener::OnWindowDeactivated(Event* aEvent) {
   return NS_OK;  // means I am NOT consuming event
 }
 
-bool nsMenuBarListener::IsMenuOpen() const {
-  auto* activeChild = mContent->GetActiveMenuChild();
-  return activeChild && activeChild->IsMenuPopupOpen();
-}
-
 ////////////////////////////////////////////////////////////////////////
 nsresult nsMenuBarListener::MouseDown(Event* aMouseEvent) {
   // NOTE: MouseDown method listens all phases
@@ -507,9 +486,8 @@ nsresult nsMenuBarListener::MouseDown(Event* aMouseEvent) {
     return NS_OK;
   }
 
-  if (!IsMenuOpen() && mMenuBarFrame->IsActive()) {
+  if (!mMenuBarFrame->IsMenuOpen() && mMenuBarFrame->IsActive())
     ToggleMenuActiveState();
-  }
 
   return NS_OK;  // means I am NOT consuming event
 }
@@ -524,8 +502,7 @@ nsresult nsMenuBarListener::Fullscreen(Event* aEvent) {
 }
 
 ////////////////////////////////////////////////////////////////////////
-MOZ_CAN_RUN_SCRIPT_BOUNDARY nsresult
-nsMenuBarListener::HandleEvent(Event* aEvent) {
+nsresult nsMenuBarListener::HandleEvent(Event* aEvent) {
   // If the menu bar is collapsed, don't do anything.
   if (!mMenuBarFrame->StyleVisibility()->IsVisible()) {
     return NS_OK;

layout/xul/nsMenuBarListener.h

@@ -23,8 +23,6 @@ namespace mozilla {
 namespace dom {
 class EventTarget;
 class KeyboardEvent;
-class XULMenuParentElement;
-class XULButtonElement;
 }  // namespace dom
 }  // namespace mozilla
 
@@ -55,36 +53,34 @@ class nsMenuBarListener final : public nsIDOMEventListener {
   static int32_t GetMenuAccessKey();
 
   /**
-   * IsAccessKeyPressed() returns true if the modifier state of the event
-   * matches the modifier state of access key.
+   * IsAccessKeyPressed() returns true if the modifier state of aEvent matches
+   * the modifier state of access key.
    */
-  static bool IsAccessKeyPressed(mozilla::dom::KeyboardEvent&);
+  static bool IsAccessKeyPressed(mozilla::dom::KeyboardEvent* aEvent);
 
  protected:
   virtual ~nsMenuBarListener();
 
-  bool IsMenuOpen() const;
-
-  MOZ_CAN_RUN_SCRIPT nsresult KeyUp(mozilla::dom::Event* aMouseEvent);
-  MOZ_CAN_RUN_SCRIPT nsresult KeyDown(mozilla::dom::Event* aMouseEvent);
-  MOZ_CAN_RUN_SCRIPT nsresult KeyPress(mozilla::dom::Event* aMouseEvent);
-  MOZ_CAN_RUN_SCRIPT nsresult Blur(mozilla::dom::Event* aEvent);
-  MOZ_CAN_RUN_SCRIPT nsresult OnWindowDeactivated(mozilla::dom::Event* aEvent);
-  MOZ_CAN_RUN_SCRIPT nsresult MouseDown(mozilla::dom::Event* aMouseEvent);
-  MOZ_CAN_RUN_SCRIPT nsresult Fullscreen(mozilla::dom::Event* aEvent);
+  nsresult KeyUp(mozilla::dom::Event* aMouseEvent);
+  nsresult KeyDown(mozilla::dom::Event* aMouseEvent);
+  nsresult KeyPress(mozilla::dom::Event* aMouseEvent);
+  nsresult Blur(mozilla::dom::Event* aEvent);
+  nsresult OnWindowDeactivated(mozilla::dom::Event* aEvent);
+  nsresult MouseDown(mozilla::dom::Event* aMouseEvent);
+  nsresult Fullscreen(mozilla::dom::Event* aEvent);
 
   static void InitAccessKey();
 
   static mozilla::Modifiers GetModifiersForAccessKey(
-      mozilla::dom::KeyboardEvent& event);
+      mozilla::dom::KeyboardEvent* event);
 
   /**
    * Given a key event for an Alt+shortcut combination,
    * return the menu, if any, that would be opened. If aPeek
    * is false, then play a beep and deactivate the menubar on Windows.
    */
-  mozilla::dom::XULButtonElement* GetMenuForKeyEvent(
-      mozilla::dom::KeyboardEvent& aKeyEvent);
+  nsMenuFrame* GetMenuForKeyEvent(mozilla::dom::KeyboardEvent* aKeyEvent,
+                                  bool aPeek);
 
   /**
    * Call MarkAsReservedByChrome if the user's preferences indicate that
@@ -94,13 +90,12 @@ class nsMenuBarListener final : public nsIDOMEventListener {
 
   // This should only be called by the nsMenuBarListener during event dispatch,
   // thus ensuring that this doesn't get destroyed during the process.
-  MOZ_CAN_RUN_SCRIPT void ToggleMenuActiveState();
+  void ToggleMenuActiveState();
 
   bool Destroyed() const { return !mMenuBarFrame; }
 
   // The menu bar object.
   nsMenuBarFrame* mMenuBarFrame;
-  mozilla::dom::XULMenuParentElement* mContent;
   // The event target to listen to the events.
   // XXX Should this store this as strong reference?  However,
   //     OnDestroyMenuBarFrame() should be called at destroying mMenuBarFrame.

layout/xul/nsMenuFrame.h

@@ -0,0 +1,266 @@
+/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
+/* vim: set ts=8 sts=2 et sw=2 tw=80: */
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+//
+// nsMenuFrame
+//
+
+#ifndef nsMenuFrame_h__
+#define nsMenuFrame_h__
+
+#include "nsAtom.h"
+#include "nsCOMPtr.h"
+
+#include "nsBoxFrame.h"
+#include "nsFrameList.h"
+#include "nsGkAtoms.h"
+#include "nsMenuParent.h"
+#include "nsXULPopupManager.h"
+#include "nsINamed.h"
+#include "nsIReflowCallback.h"
+#include "nsITimer.h"
+#include "mozilla/Attributes.h"
+
+namespace mozilla {
+class PresShell;
+}  // namespace mozilla
+
+nsIFrame* NS_NewMenuFrame(mozilla::PresShell* aPresShell,
+                          mozilla::ComputedStyle*);
+nsIFrame* NS_NewMenuItemFrame(mozilla::PresShell* aPresShell,
+                              mozilla::ComputedStyle*);
+
+class nsIContent;
+
+namespace mozilla {
+namespace dom {
+class Element;
+}  // namespace dom
+}  // namespace mozilla
+
+// the type of menuitem
+enum nsMenuType {
+  // a normal menuitem where a command is carried out when activated
+  eMenuType_Normal = 0,
+  // a menuitem with a checkmark that toggles when activated
+  eMenuType_Checkbox = 1,
+  // a radio menuitem where only one of it and its siblings with the same
+  // name attribute can be checked at a time
+  eMenuType_Radio = 2
+};
+
+class nsMenuFrame;
+
+/**
+ * nsMenuTimerMediator is a wrapper around an nsMenuFrame which can be safely
+ * passed to timers. The class is reference counted unlike the underlying
+ * nsMenuFrame, so that it will exist as long as the timer holds a reference
+ * to it. The callback is delegated to the contained nsMenuFrame as long as
+ * the contained nsMenuFrame has not been destroyed.
+ */
+class nsMenuTimerMediator final : public nsITimerCallback, public nsINamed {
+ public:
+  explicit nsMenuTimerMediator(nsMenuFrame* aFrame);
+
+  NS_DECL_ISUPPORTS
+  NS_DECL_NSITIMERCALLBACK
+  NS_DECL_NSINAMED
+
+  void ClearFrame();
+
+ private:
+  ~nsMenuTimerMediator();
+
+  // Pointer to the wrapped frame.
+  nsMenuFrame* mFrame;
+};
+
+class nsMenuFrame final : public nsBoxFrame, public nsIReflowCallback {
+ public:
+  explicit nsMenuFrame(ComputedStyle* aStyle, nsPresContext* aPresContext);
+
+  NS_DECL_QUERYFRAME
+  NS_DECL_FRAMEARENA_HELPERS(nsMenuFrame)
+
+  NS_IMETHOD DoXULLayout(nsBoxLayoutState& aBoxLayoutState) override;
+
+  virtual void Init(nsIContent* aContent, nsContainerFrame* aParent,
+                    nsIFrame* aPrevInFlow) override;
+
+  // The following methods are all overridden so that the menupopup
+  // can be stored in a separate list, so that it doesn't impact reflow of the
+  // actual menu item at all.
+  virtual const nsFrameList& GetChildList(ChildListID aList) const override;
+  virtual void GetChildLists(nsTArray<ChildList>* aLists) const override;
+  virtual void DestroyFrom(nsIFrame* aDestructRoot,
+                           PostDestroyData& aPostDestroyData) override;
+
+  // Overridden to prevent events from going to children of the menu.
+  virtual void BuildDisplayListForChildren(
+      nsDisplayListBuilder* aBuilder, const nsDisplayListSet& aLists) override;
+
+  // this method can destroy the frame
+  virtual nsresult HandleEvent(nsPresContext* aPresContext,
+                               mozilla::WidgetGUIEvent* aEvent,
+                               nsEventStatus* aEventStatus) override;
+
+  void SetInitialChildList(ChildListID aListID,
+                           nsFrameList&& aChildList) override;
+  void AppendFrames(ChildListID aListID, nsFrameList&& aFrameList) override;
+  void InsertFrames(ChildListID aListID, nsIFrame* aPrevFrame,
+                    const nsLineList::iterator* aPrevFrameLine,
+                    nsFrameList&& aFrameList) override;
+  virtual void RemoveFrame(ChildListID aListID, nsIFrame* aOldFrame) override;
+
+  NS_IMETHOD SelectMenu(bool aActivateFlag);
+
+  virtual nsIScrollableFrame* GetScrollTargetFrame() const override;
+
+  /**
+   * NOTE: OpenMenu will open the menu asynchronously.
+   */
+  void OpenMenu(bool aSelectFirstItem);
+  // CloseMenu closes the menu asynchronously
+  void CloseMenu(bool aDeselectMenu);
+
+  bool IsChecked() { return mChecked; }
+
+  NS_IMETHOD GetActiveChild(mozilla::dom::Element** aResult);
+  NS_IMETHOD SetActiveChild(mozilla::dom::Element* aChild);
+
+  // called when the Enter key is pressed while the menuitem is the current
+  // one in its parent popup. This will carry out the command attached to
+  // the menuitem. If the menu should be opened, this frame will be returned,
+  // otherwise null will be returned.
+  nsMenuFrame* Enter(mozilla::WidgetGUIEvent* aEvent);
+
+  // Return the nearest menu bar or menupopup ancestor frame.
+  nsMenuParent* GetMenuParent() const;
+
+  const nsAString& GetRadioGroupName() { return mGroupName; }
+  nsMenuType GetMenuType() { return mType; }
+  nsMenuPopupFrame* GetPopup() const;
+
+  /**
+   * @return true if this frame has a popup child frame.
+   */
+  bool HasPopup() const {
+    return HasAnyStateBits(NS_STATE_MENU_HAS_POPUP_LIST);
+  }
+
+  // nsMenuFrame methods
+
+  bool IsOnMenuBar() const {
+    nsMenuParent* menuParent = GetMenuParent();
+    return menuParent && menuParent->IsMenuBar();
+  }
+  bool IsOnActiveMenuBar() const {
+    nsMenuParent* menuParent = GetMenuParent();
+    return menuParent && menuParent->IsMenuBar() && menuParent->IsActive();
+  }
+  virtual bool IsOpen();
+  virtual bool IsMenu();
+  bool IsParentMenuList();
+  bool IsDisabled();
+  void ToggleMenuState();
+
+  // Activate this menu item.
+  void ActivateItem(mozilla::Modifiers aModifiers, int16_t aButton);
+
+  // indiciate that the menu's popup has just been opened, so that the menu
+  // can update its open state. This method modifies the open attribute on
+  // the menu, so the frames could be gone after this call.
+  void PopupOpened();
+  // indiciate that the menu's popup has just been closed, so that the menu
+  // can update its open state. The menu should be unhighlighted if
+  // aDeselectedMenu is true. This method modifies the open attribute on
+  // the menu, so the frames could be gone after this call.
+  void PopupClosed(bool aDeselectMenu);
+
+  // returns true if this is a menu on another menu popup. A menu is a submenu
+  // if it has a parent popup or menupopup.
+  bool IsOnMenu() const {
+    nsMenuParent* menuParent = GetMenuParent();
+    return menuParent && menuParent->IsMenu();
+  }
+  void SetIsMenu(bool aIsMenu) { mIsMenu = aIsMenu; }
+
+#ifdef DEBUG_FRAME_DUMP
+  virtual nsresult GetFrameName(nsAString& aResult) const override {
+    return MakeFrameName(u"Menu"_ns, aResult);
+  }
+#endif
+
+  // nsIReflowCallback
+  virtual bool ReflowFinished() override;
+  virtual void ReflowCallbackCanceled() override;
+
+ protected:
+  friend class nsMenuTimerMediator;
+  friend class nsASyncMenuInitialization;
+  friend class nsMenuAttributeChangedEvent;
+
+  /**
+   * Initialize the popup list to the first popup frame within
+   * aChildList. Removes the popup, if any, from aChildList.
+   */
+  void SetPopupFrame(nsFrameList& aChildList);
+
+  /**
+   * Get the popup frame list from the frame property.
+   * @return the property value if it exists, nullptr otherwise.
+   */
+  nsFrameList* GetPopupList() const;
+
+  /**
+   * Destroy the popup list property.  The list must exist and be empty.
+   */
+  void DestroyPopupList();
+
+  // Update the menu's type (normal, checkbox, radio).
+  // This method can destroy the frame.
+  void UpdateMenuType();
+  // Update the checked state of the menu, and for radios, clear any other
+  // checked items. This method can destroy the frame.
+  void UpdateMenuSpecialState();
+
+  // Called to execute our command handler. This method can destroy the frame.
+  void Execute(mozilla::WidgetGUIEvent* aEvent);
+
+  // This method can destroy the frame
+  nsresult AttributeChanged(int32_t aNameSpaceID, nsAtom* aAttribute,
+                            int32_t aModType) override;
+  virtual ~nsMenuFrame() = default;
+
+  bool ShouldBlink();
+  void StartBlinking();
+  void StopBlinking();
+  void CreateMenuCommandEvent(bool aIsTrusted, mozilla::Modifiers aModifiers,
+                              int16_t aButton);
+  void PassMenuCommandEventToPopupManager();
+
+ protected:
+  nsresult Notify(nsITimer* aTimer);
+
+  bool mIsMenu;   // Whether or not we can even have children or not.
+  bool mChecked;  // are we checked?
+  bool mReflowCallbackPosted;
+  nsMenuType mType;
+
+  // Reference to the mediator which wraps this frame.
+  RefPtr<nsMenuTimerMediator> mTimerMediator;
+
+  nsCOMPtr<nsITimer> mOpenTimer;
+  nsCOMPtr<nsITimer> mBlinkTimer;
+
+  uint8_t mBlinkState;  // 0: not blinking, 1: off, 2: on
+  RefPtr<nsXULMenuCommandEvent> mDelayedMenuCommandEvent;
+
+  nsString mGroupName;
+
+};  // class nsMenuFrame
+
+#endif

layout/xul/nsMenuParent.h

@@ -0,0 +1,69 @@
+/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
+/* vim: set ts=8 sts=2 et sw=2 tw=80: */
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+#ifndef nsMenuParent_h___
+#define nsMenuParent_h___
+
+class nsMenuFrame;
+
+/*
+ * nsMenuParent is an interface implemented by nsMenuBarFrame and
+ * nsMenuPopupFrame as both serve as parent frames to nsMenuFrame.
+ *
+ * Don't implement this interface on other classes unless you also fix up
+ * references, as this interface is directly cast to and from nsMenuBarFrame and
+ * nsMenuPopupFrame.
+ */
+
+class nsMenuParent {
+ public:
+  // returns the menu frame of the currently active item within the menu
+  virtual nsMenuFrame* GetCurrentMenuItem() = 0;
+  // sets the currently active menu frame.
+  NS_IMETHOD SetCurrentMenuItem(nsMenuFrame* aMenuItem) = 0;
+  // indicate that the current menu frame is being destroyed, so clear the
+  // current menu item
+  virtual void CurrentMenuIsBeingDestroyed() = 0;
+  // deselects the current item and closes its popup if any, then selects the
+  // new item aMenuItem. For a menubar, if another menu is already open, the
+  // new menu aMenuItem is opened. In this case, if aSelectFirstItem is true,
+  // select the first item in it. For menupopups, the menu is not opened and
+  // the aSelectFirstItem argument is not used. The aFromKey argument indicates
+  // that the keyboard was used to navigate to the new menu item.
+  NS_IMETHOD ChangeMenuItem(nsMenuFrame* aMenuItem, bool aSelectFirstItem,
+                            bool aFromKey) = 0;
+
+  // returns true if the menupopup is open. For menubars, returns false.
+  virtual bool IsOpen() = 0;
+  // returns true if the menubar is currently active. For menupopups, returns
+  // false.
+  virtual bool IsActive() = 0;
+  // returns true if this is a menubar. If false, it is a popup
+  virtual bool IsMenuBar() = 0;
+  // returns true if this is a menu, which has a tag of menupopup or popup.
+  // Otherwise, this returns false
+  virtual bool IsMenu() = 0;
+  // returns true if this is a context menu
+  virtual bool IsContextMenu() = 0;
+
+  // indicate that the menubar should become active or inactive
+  NS_IMETHOD SetActive(bool aActiveFlag) = 0;
+
+  // notify that the menu has been closed and any active state should be
+  // cleared. This should return true if the menu should be deselected
+  // by the caller.
+  virtual bool MenuClosed() = 0;
+
+  // Lock this menu and its parents until they're closed or unlocked.
+  // A menu being "locked" means that all events inside it that would change the
+  // selected menu item should be ignored.
+  // This is used when closing the popup is delayed because of a blink or fade
+  // animation.
+  virtual void LockMenuUntilClosed(bool aLock) = 0;
+  virtual bool IsMenuLocked() = 0;
+};
+
+#endif

layout/xul/nsMenuPopupFrame.cpp

@@ -5,12 +5,8 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "nsMenuPopupFrame.h"
-#include "XULButtonElement.h"
-#include "XULPopupElement.h"
-#include "mozilla/dom/XULPopupElement.h"
 #include "nsGkAtoms.h"
 #include "nsIContent.h"
-#include "nsIFrameInlines.h"
 #include "nsAtom.h"
 #include "nsPresContext.h"
 #include "mozilla/ComputedStyle.h"
@@ -19,6 +15,7 @@
 #include "nsIFrameInlines.h"
 #include "nsViewManager.h"
 #include "nsWidgetsCID.h"
+#include "nsMenuFrame.h"
 #include "nsMenuBarFrame.h"
 #include "nsPIDOMWindow.h"
 #include "nsFrameManager.h"
@@ -67,7 +64,7 @@ using namespace mozilla;
 using mozilla::dom::Document;
 using mozilla::dom::Element;
 using mozilla::dom::Event;
-using mozilla::dom::XULButtonElement;
+using mozilla::dom::KeyboardEvent;
 
 int8_t nsMenuPopupFrame::sDefaultLevelIsTop = -1;
 
@@ -105,6 +102,7 @@ NS_QUERYFRAME_TAIL_INHERITING(nsBoxFrame)
 nsMenuPopupFrame::nsMenuPopupFrame(ComputedStyle* aStyle,
                                    nsPresContext* aPresContext)
     : nsBoxFrame(aStyle, aPresContext, kClassID),
+      mCurrentMenu(nullptr),
       mView(nullptr),
       mPrefSize(-1, -1),
       mXPos(0),
@@ -120,6 +118,7 @@ nsMenuPopupFrame::nsMenuPopupFrame(ComputedStyle* aStyle,
       mIsOpenChanged(false),
       mMenuCanOverlapOSBar(false),
       mInContentShell(true),
+      mIsMenuLocked(false),
       mIsOffset(false),
       mHFlip(false),
       mVFlip(false),
@@ -541,20 +540,6 @@ void nsMenuPopupFrame::Reflow(nsPresContext* aPresContext,
   FinishAndStoreOverflow(&aDesiredSize, aReflowInput.mStyleDisplay);
 }
 
-void nsMenuPopupFrame::EnsureActiveMenuListItemIsVisible() {
-  if (!IsMenuList() || !IsOpen()) {
-    return;
-  }
-  nsIFrame* frame = GetCurrentMenuItemFrame();
-  if (!frame) {
-    return;
-  }
-  RefPtr<mozilla::PresShell> presShell = PresShell();
-  presShell->ScrollFrameIntoView(
-      frame, Nothing(), ScrollAxis(), ScrollAxis(),
-      ScrollFlags::ScrollOverflowHidden | ScrollFlags::ScrollFirstAncestorOnly);
-}
-
 void nsMenuPopupFrame::LayoutPopup(nsBoxLayoutState& aState) {
   if (IsNativeMenu()) {
     return;
@@ -722,7 +707,9 @@ void nsMenuPopupFrame::LayoutPopup(nsBoxLayoutState& aState) {
     mIsOpenChanged = false;
 
     // Make sure the current selection in a menulist is visible.
-    EnsureActiveMenuListItemIsVisible();
+    if (IsMenuList() && mCurrentMenu) {
+      EnsureMenuItemIsVisible(mCurrentMenu);
+    }
 
     // If the animate attribute is set to open, check for a transition and wait
     // for it to finish before firing the popupshown event.
@@ -758,7 +745,8 @@ bool nsMenuPopupFrame::ReflowFinished() {
 void nsMenuPopupFrame::ReflowCallbackCanceled() { mReflowCallbackData.Clear(); }
 
 bool nsMenuPopupFrame::IsMenuList() const {
-  return PopupElement().IsInMenuList();
+  return mContent->GetParent() &&
+         mContent->GetParent()->IsXULElement(nsGkAtoms::menulist);
 }
 
 bool nsMenuPopupFrame::ShouldExpandToInflowParentOrAnchor() const {
@@ -774,19 +762,14 @@ nsIContent* nsMenuPopupFrame::GetTriggerContent(
       return aMenuPopupFrame->mTriggerContent;
     }
 
-    auto* button = XULButtonElement::FromNodeOrNull(
-        aMenuPopupFrame->GetContent()->GetParent());
-    if (!button || !button->IsMenu()) {
-      break;
-    }
-
-    auto* popup = button->GetContainingPopupElement();
-    if (!popup) {
-      break;
-    }
-
     // check up the menu hierarchy until a popup with a trigger node is found
-    aMenuPopupFrame = do_QueryFrame(popup->GetPrimaryFrame());
+    nsMenuFrame* menuFrame = do_QueryFrame(aMenuPopupFrame->GetParent());
+    if (!menuFrame) break;
+
+    nsMenuParent* parentPopup = menuFrame->GetMenuParent();
+    if (!parentPopup || !parentPopup->IsMenu()) break;
+
+    aMenuPopupFrame = static_cast<nsMenuPopupFrame*>(parentPopup);
   }
 
   return nullptr;
@@ -1047,8 +1030,11 @@ void nsMenuPopupFrame::ShowPopup(bool aIsContextMenu) {
       PresShell::ReleaseCapturingContent();
     }
 
-    if (RefPtr menu = PopupElement().GetContainingMenu()) {
-      menu->PopupOpened();
+    nsMenuFrame* menuFrame = do_QueryFrame(GetParent());
+    if (menuFrame) {
+      AutoWeakFrame weakFrame(this);
+      menuFrame->PopupOpened();
+      if (!weakFrame.IsAlive()) return;
     }
 
     // do we need an actual reflow here?
@@ -1082,8 +1068,7 @@ void nsMenuPopupFrame::ClearTriggerContentIncludingDocument() {
   mTriggerContent = nullptr;
 }
 
-void nsMenuPopupFrame::HidePopup(bool aDeselectMenu, nsPopupState aNewState,
-                                 bool aFromFrameDestruction) {
+void nsMenuPopupFrame::HidePopup(bool aDeselectMenu, nsPopupState aNewState) {
   NS_ASSERTION(aNewState == ePopupClosed || aNewState == ePopupInvisible,
                "popup being set to unexpected state");
 
@@ -1091,9 +1076,8 @@ void nsMenuPopupFrame::HidePopup(bool aDeselectMenu, nsPopupState aNewState,
 
   // don't hide the popup when it isn't open
   if (mPopupState == ePopupClosed || mPopupState == ePopupShowing ||
-      mPopupState == ePopupPositioning) {
+      mPopupState == ePopupPositioning)
     return;
-  }
 
   if (aNewState == ePopupClosed) {
     // clear the trigger content if the popup is being closed. But don't clear
@@ -1106,17 +1090,20 @@ void nsMenuPopupFrame::HidePopup(bool aDeselectMenu, nsPopupState aNewState,
   // when invisible and about to be closed, HidePopup has already been called,
   // so just set the new state to closed and return
   if (mPopupState == ePopupInvisible) {
-    if (aNewState == ePopupClosed) {
-      mPopupState = ePopupClosed;
-    }
+    if (aNewState == ePopupClosed) mPopupState = ePopupClosed;
     return;
   }
 
   mPopupState = aNewState;
 
+  if (IsMenu()) SetCurrentMenuItem(nullptr);
+
   mIncrementalString.Truncate();
 
+  LockMenuUntilClosed(false);
+
   mIsOpenChanged = false;
+  mCurrentMenu = nullptr;  // make sure no current menu is set
   mHFlip = mVFlip = false;
 
   if (auto* widget = GetWidget()) {
@@ -1130,17 +1117,22 @@ void nsMenuPopupFrame::HidePopup(bool aDeselectMenu, nsPopupState aNewState,
   nsViewManager* viewManager = view->GetViewManager();
   viewManager->SetViewVisibility(view, nsViewVisibility_kHide);
 
-  RefPtr popup = &PopupElement();
+  FireDOMEvent(u"DOMMenuInactive"_ns, mContent);
+
   // XXX, bug 137033, In Windows, if mouse is outside the window when the
   // menupopup closes, no mouse_enter/mouse_exit event will be fired to clear
   // current hover state, we should clear it manually. This code may not the
   // best solution, but we can leave it here until we find the better approach.
-  if (!aFromFrameDestruction &&
-      popup->State().HasState(dom::ElementState::HOVER)) {
+  NS_ASSERTION(mContent->IsElement(), "How do we have a non-element?");
+  if (mContent->AsElement()->State().HasState(dom::ElementState::HOVER)) {
     EventStateManager* esm = PresContext()->EventStateManager();
     esm->SetContentState(nullptr, dom::ElementState::HOVER);
   }
-  popup->PopupClosed(aDeselectMenu);
+
+  nsMenuFrame* menuFrame = do_QueryFrame(GetParent());
+  if (menuFrame) {
+    menuFrame->PopupClosed(aDeselectMenu);
+  }
 }
 
 nsIFrame::ReflowChildFlags nsMenuPopupFrame::GetXULLayoutFlags() {
@@ -1794,6 +1786,9 @@ void nsMenuPopupFrame::WidgetPositionOrSizeDidChange() {
   }
 }
 
+/* virtual */
+nsMenuFrame* nsMenuPopupFrame::GetCurrentMenuItem() { return mCurrentMenu; }
+
 LayoutDeviceIntRect nsMenuPopupFrame::GetConstraintRect(
     const LayoutDeviceIntRect& aAnchorRect,
     const LayoutDeviceIntRect& aRootScreenRect, nsPopupLevel aPopupLevel) {
@@ -1952,102 +1947,189 @@ nsIScrollableFrame* nsMenuPopupFrame::GetScrollFrame(nsIFrame* aStart) {
   return nullptr;
 }
 
+void nsMenuPopupFrame::EnsureMenuItemIsVisible(nsMenuFrame* aMenuItem) {
+  if (aMenuItem) {
+    RefPtr<mozilla::PresShell> presShell = aMenuItem->PresShell();
+    presShell->ScrollFrameIntoView(aMenuItem, Nothing(), ScrollAxis(),
+                                   ScrollAxis(),
+                                   ScrollFlags::ScrollOverflowHidden |
+                                       ScrollFlags::ScrollFirstAncestorOnly);
+  }
+}
+
 void nsMenuPopupFrame::ChangeByPage(bool aIsUp) {
   // Only scroll by page within menulists.
   if (!IsMenuList()) {
     return;
   }
 
-  nsIScrollableFrame* scrollframe = GetScrollFrame(this);
-
-  RefPtr popup = &PopupElement();
-  XULButtonElement* currentMenu = popup->GetActiveMenuChild();
-  XULButtonElement* newMenu = nullptr;
+  nsMenuFrame* newMenu = nullptr;
+  nsIFrame* currentMenu = mCurrentMenu;
   if (!currentMenu) {
     // If there is no current menu item, get the first item. When moving up,
-    // just use this as the newMenu and leave currentMenu null so that no check
-    // for a later element is performed. When moving down, set currentMenu so
-    // that we look for one page down from the first item.
-    newMenu = popup->GetFirstMenuItem();
+    // just use this as the newMenu and leave currentMenu null so that no
+    // check for a later element is performed. When moving down, set currentMenu
+    // so that we look for one page down from the first item.
+    newMenu = nsXULPopupManager::GetNextMenuItem(this, nullptr, true, false);
     if (!aIsUp) {
       currentMenu = newMenu;
     }
   }
 
-  if (currentMenu && currentMenu->GetPrimaryFrame()) {
-    const nscoord scrollHeight =
-        scrollframe ? scrollframe->GetScrollPortRect().height : mRect.height;
-    const nsRect currentRect = currentMenu->GetPrimaryFrame()->GetRect();
-    const XULButtonElement* startMenu = currentMenu;
+  if (currentMenu) {
+    nscoord scrollHeight = mRect.height;
+    nsIScrollableFrame* scrollframe = GetScrollFrame(this);
+    if (scrollframe) {
+      scrollHeight = scrollframe->GetScrollPortRect().height;
+    }
 
     // Get the position of the current item and add or subtract one popup's
     // height to or from it.
-    const nscoord targetPos = aIsUp ? currentRect.YMost() - scrollHeight
-                                    : currentRect.y + scrollHeight;
+    nscoord targetPosition = aIsUp
+                                 ? currentMenu->GetRect().YMost() - scrollHeight
+                                 : currentMenu->GetRect().y + scrollHeight;
+
+    // Indicates that the last visible child was a valid menuitem.
+    bool lastWasValid = false;
+
     // Look for the next child which is just past the target position. This
     // child will need to be selected.
-    for (; currentMenu;
-         currentMenu = aIsUp ? popup->GetPrevMenuItemFrom(*currentMenu)
-                             : popup->GetNextMenuItemFrom(*currentMenu)) {
-      if (!currentMenu->GetPrimaryFrame()) {
-        continue;
-      }
-      const nsRect curRect = currentMenu->GetPrimaryFrame()->GetRect();
-      const nscoord curPos = aIsUp ? curRect.y : curRect.YMost();
-      // If the right position was found, break out. Otherwise, look for another
-      // item.
-      if (aIsUp ? (curPos < targetPos) : (curPos > targetPos)) {
-        if (!newMenu || newMenu == startMenu) {
-          newMenu = currentMenu;
+    while (currentMenu) {
+      // Only consider menu frames.
+      nsMenuFrame* menuFrame = do_QueryFrame(currentMenu);
+      if (menuFrame &&
+          nsXULPopupManager::IsValidMenuItem(menuFrame->GetContent(), true)) {
+        // If the right position was found, break out. Otherwise, look for
+        // another item.
+        if ((!aIsUp && currentMenu->GetRect().YMost() > targetPosition) ||
+            (aIsUp && currentMenu->GetRect().y < targetPosition)) {
+          // If the last visible child was not a valid menuitem or was disabled,
+          // use this as the menu to select, skipping over any non-valid items
+          // at the edge of the page.
+          if (!lastWasValid) {
+            newMenu = menuFrame;
+          }
+
+          break;
         }
-        break;
+
+        // Assign this item to newMenu. This item will be selected in case we
+        // don't find any more.
+        lastWasValid = true;
+        newMenu = menuFrame;
+      } else {
+        lastWasValid = false;
       }
 
-      // Assign this item to newMenu. This item will be selected in case we
-      // don't find any more.
-      newMenu = currentMenu;
+      currentMenu =
+          aIsUp ? currentMenu->GetPrevSibling() : currentMenu->GetNextSibling();
     }
   }
 
   // Select the new menuitem.
-  if (RefPtr newMenuRef = newMenu) {
-    popup->SetActiveMenuChild(newMenuRef);
+  if (newMenu) {
+    ChangeMenuItem(newMenu, false, true);
   }
 }
 
-dom::XULPopupElement& nsMenuPopupFrame::PopupElement() const {
-  auto* popup = dom::XULPopupElement::FromNode(GetContent());
-  MOZ_DIAGNOSTIC_ASSERT(popup);
-  return *popup;
+NS_IMETHODIMP nsMenuPopupFrame::SetCurrentMenuItem(nsMenuFrame* aMenuItem) {
+  if (mCurrentMenu == aMenuItem) return NS_OK;
+
+  if (mCurrentMenu) {
+    mCurrentMenu->SelectMenu(false);
+  }
+
+  if (aMenuItem) {
+    EnsureMenuItemIsVisible(aMenuItem);
+    aMenuItem->SelectMenu(true);
+  }
+
+  mCurrentMenu = aMenuItem;
+
+  return NS_OK;
 }
 
-XULButtonElement* nsMenuPopupFrame::GetCurrentMenuItem() const {
-  return PopupElement().GetActiveMenuChild();
+void nsMenuPopupFrame::CurrentMenuIsBeingDestroyed() { mCurrentMenu = nullptr; }
+
+NS_IMETHODIMP
+nsMenuPopupFrame::ChangeMenuItem(nsMenuFrame* aMenuItem, bool aSelectFirstItem,
+                                 bool aFromKey) {
+  if (mCurrentMenu == aMenuItem) return NS_OK;
+
+  // When a context menu is open, the current menu is locked, and no change
+  // to the menu is allowed.
+  nsXULPopupManager* pm = nsXULPopupManager::GetInstance();
+  if (!mIsContextMenu && pm && pm->HasContextMenu(this)) return NS_OK;
+
+  // Unset the current child.
+  if (mCurrentMenu) {
+    mCurrentMenu->SelectMenu(false);
+    nsMenuPopupFrame* popup = mCurrentMenu->GetPopup();
+    if (popup) {
+      if (mCurrentMenu->IsOpen()) {
+        if (pm) pm->HidePopupAfterDelay(popup);
+      }
+    }
+  }
+
+  // Set the new child.
+  if (aMenuItem) {
+    EnsureMenuItemIsVisible(aMenuItem);
+    aMenuItem->SelectMenu(true);
+
+    // On Windows, a menulist should update its value whenever navigation was
+    // done by the keyboard.
+#ifdef XP_WIN
+    if (aFromKey && IsOpen() && IsMenuList()) {
+      // Fire a command event as the new item, but we don't want to close
+      // the menu, blink it, or update any other state of the menuitem. The
+      // command event will cause the item to be selected.
+      nsCOMPtr<nsIContent> menuItemContent = aMenuItem->GetContent();
+      RefPtr<mozilla::PresShell> presShell = PresShell();
+      nsContentUtils::DispatchXULCommand(menuItemContent, /* aTrusted = */ true,
+                                         nullptr, presShell, false, false,
+                                         false, false);
+    }
+#endif
+  }
+
+  mCurrentMenu = aMenuItem;
+
+  return NS_OK;
 }
 
-nsIFrame* nsMenuPopupFrame::GetCurrentMenuItemFrame() const {
-  auto* child = GetCurrentMenuItem();
-  return child ? child->GetPrimaryFrame() : nullptr;
-}
-
-void nsMenuPopupFrame::HandleEnterKeyPress(WidgetEvent& aEvent) {
+nsMenuFrame* nsMenuPopupFrame::Enter(WidgetGUIEvent* aEvent) {
   mIncrementalString.Truncate();
-  if (RefPtr menu = GetCurrentMenuItem()) {
-    // Give it to the child.
-    menu->HandleEnterKeyPress(aEvent);
-  }
+
+  // Give it to the child.
+  if (mCurrentMenu) return mCurrentMenu->Enter(aEvent);
+
+  return nullptr;
 }
 
-XULButtonElement* nsMenuPopupFrame::FindMenuWithShortcut(
-    KeyboardEvent& aKeyEvent, bool& aDoAction) {
-  uint32_t charCode = aKeyEvent.CharCode();
-  uint32_t keyCode = aKeyEvent.KeyCode();
+nsMenuFrame* nsMenuPopupFrame::FindMenuWithShortcut(KeyboardEvent* aKeyEvent,
+                                                    bool& doAction) {
+  uint32_t charCode = aKeyEvent->CharCode();
+  uint32_t keyCode = aKeyEvent->KeyCode();
 
-  aDoAction = false;
+  doAction = false;
 
   // Enumerate over our list of frames.
-  const bool isMenu = !IsMenuList();
-  TimeStamp keyTime = aKeyEvent.WidgetEventPtr()->mTimeStamp;
+  nsContainerFrame* immediateParent =
+      nsXULPopupManager::ImmediateParentFrame(this);
+  uint32_t matchCount = 0, matchShortcutCount = 0;
+  bool foundActive = false;
+  nsMenuFrame* frameBefore = nullptr;
+  nsMenuFrame* frameAfter = nullptr;
+  nsMenuFrame* frameShortcut = nullptr;
+
+  nsIContent* parentContent = mContent->GetParent();
+
+  bool isMenu = parentContent && !parentContent->NodeInfo()->Equals(
+                                     nsGkAtoms::menulist, kNameSpaceID_XUL);
+
+  TimeStamp keyTime = aKeyEvent->WidgetEventPtr()->mTimeStamp;
+
   if (charCode == 0) {
     if (keyCode == dom::KeyboardEvent_Binding::DOM_VK_BACK_SPACE) {
       if (!isMenu && !mIncrementalString.IsEmpty()) {
@@ -2055,9 +2137,8 @@ XULButtonElement* nsMenuPopupFrame::FindMenuWithShortcut(
         return nullptr;
       }
 #ifdef XP_WIN
-      if (nsCOMPtr<nsISound> sound = do_GetService("@mozilla.org/sound;1")) {
-        sound->Beep();
-      }
+      nsCOMPtr<nsISound> soundInterface = do_GetService("@mozilla.org/sound;1");
+      if (soundInterface) soundInterface->Beep();
 #endif  // #ifdef XP_WIN
     }
     return nullptr;
@@ -2088,12 +2169,94 @@ XULButtonElement* nsMenuPopupFrame::FindMenuWithShortcut(
 
   sLastKeyTime = keyTime;
 
-  auto* item =
-      PopupElement().FindMenuWithShortcut(incrementalString, aDoAction);
-  if (item) {
-    return item;
+  // NOTE: If you crashed here due to a bogus |immediateParent| it is
+  //       possible that the menu whose shortcut is being looked up has
+  //       been destroyed already.  One strategy would be to
+  //       setTimeout(<func>,0) as detailed in:
+  //       <http://bugzilla.mozilla.org/show_bug.cgi?id=126675#c32>
+  nsIFrame* firstMenuItem =
+      nsXULPopupManager::GetNextMenuItem(immediateParent, nullptr, true, false);
+  nsIFrame* currFrame = firstMenuItem;
+
+  int32_t menuAccessKey = nsMenuBarListener::GetMenuAccessKey();
+
+  // We start searching from first child. This process is divided into two parts
+  //   -- before current and after current -- by the current item
+  while (currFrame) {
+    nsIContent* current = currFrame->GetContent();
+    nsAutoString textKey;
+    bool isShortcut = false;
+    if (current->IsElement()) {
+      if (menuAccessKey >= 0) {
+        // Get the shortcut attribute.
+        current->AsElement()->GetAttr(kNameSpaceID_None, nsGkAtoms::accesskey,
+                                      textKey);
+      }
+      isShortcut = !textKey.IsEmpty();
+      if (textKey.IsEmpty()) {  // No shortcut, try first letter
+        current->AsElement()->GetAttr(kNameSpaceID_None, nsGkAtoms::label,
+                                      textKey);
+        if (textKey.IsEmpty())  // No label, try another attribute (value)
+          current->AsElement()->GetAttr(kNameSpaceID_None, nsGkAtoms::value,
+                                        textKey);
+      }
+    }
+
+    if (StringBeginsWith(
+            nsContentUtils::TrimWhitespace<
+                nsContentUtils::IsHTMLWhitespaceOrNBSP>(textKey, false),
+            incrementalString, nsCaseInsensitiveStringComparator)) {
+      // mIncrementalString is a prefix of textKey
+      nsMenuFrame* menu = do_QueryFrame(currFrame);
+      if (menu) {
+        // There is one match
+        matchCount++;
+        if (isShortcut) {
+          // There is one shortcut-key match
+          matchShortcutCount++;
+          // Record the matched item. If there is only one matched shortcut
+          // item, do it
+          frameShortcut = menu;
+        }
+        if (!foundActive) {
+          // It's a first candidate item located before/on the current item
+          if (!frameBefore) frameBefore = menu;
+        } else {
+          // It's a first candidate item located after the current item
+          if (!frameAfter) frameAfter = menu;
+        }
+      } else
+        return nullptr;
+    }
+
+    // Get the active status
+    if (current->IsElement() && current->AsElement()->AttrValueIs(
+                                    kNameSpaceID_None, nsGkAtoms::menuactive,
+                                    nsGkAtoms::_true, eCaseMatters)) {
+      foundActive = true;
+      if (stringLength > 1) {
+        // If there is more than one char typed, the current item has highest
+        // priority,
+        //   otherwise the item next to current has highest priority
+        if (currFrame == frameBefore) return frameBefore;
+      }
+    }
+
+    nsMenuFrame* menu = do_QueryFrame(currFrame);
+    currFrame =
+        nsXULPopupManager::GetNextMenuItem(immediateParent, menu, true, true);
+    if (currFrame == firstMenuItem) break;
   }
 
+  doAction = (isMenu && (matchCount == 1 || matchShortcutCount == 1));
+
+  if (matchShortcutCount == 1)  // We have one matched shortcut item
+    return frameShortcut;
+  if (frameAfter)  // If we have matched item after the current, use it
+    return frameAfter;
+  else if (frameBefore)  // If we haven't, use the item before the current
+    return frameBefore;
+
   // If we don't match anything, rollback the last typing
   mIncrementalString.SetLength(mIncrementalString.Length() - 1);
 
@@ -2102,15 +2265,27 @@ XULButtonElement* nsMenuPopupFrame::FindMenuWithShortcut(
   // behavior on Windows - this item is in a menu popup off of the
   // menu bar, so beep and do nothing else
   if (isMenu) {
-    if (nsCOMPtr<nsISound> sound = do_GetService("@mozilla.org/sound;1")) {
-      sound->Beep();
-    }
+    nsCOMPtr<nsISound> soundInterface = do_GetService("@mozilla.org/sound;1");
+    if (soundInterface) soundInterface->Beep();
   }
 #endif  // #ifdef XP_WIN
 
   return nullptr;
 }
 
+void nsMenuPopupFrame::LockMenuUntilClosed(bool aLock) {
+  mIsMenuLocked = aLock;
+
+  // Lock / unlock the parent, too.
+  nsMenuFrame* menu = do_QueryFrame(GetParent());
+  if (menu) {
+    nsMenuParent* parentParent = menu->GetMenuParent();
+    if (parentParent) {
+      parentParent->LockMenuUntilClosed(aLock);
+    }
+  }
+}
+
 nsIWidget* nsMenuPopupFrame::GetWidget() const {
   return mView ? mView->GetWidget() : nullptr;
 }
@@ -2194,12 +2369,16 @@ void nsMenuPopupFrame::DestroyFrom(nsIFrame* aDestructRoot,
     mReflowCallbackData.Clear();
   }
 
-  // XXX: Currently we don't fire popuphidden for these popups, that seems wrong
-  // but alas, also pre-existing.
-  HidePopup(/* aDeselectMenu = */ false, ePopupClosed,
-            /* aFromFrameDestruction = */ true);
+  nsMenuFrame* menu = do_QueryFrame(GetParent());
+  if (menu) {
+    // clear the open attribute on the parent menu
+    nsContentUtils::AddScriptRunner(new nsUnsetAttrRunnable(
+        menu->GetContent()->AsElement(), nsGkAtoms::open));
+  }
 
-  if (RefPtr<nsXULPopupManager> pm = nsXULPopupManager::GetInstance()) {
+  ClearPopupShownDispatcher();
+
+  if (nsXULPopupManager* pm = nsXULPopupManager::GetInstance()) {
     pm->PopupDestroyed(this);
   }
 
@@ -2477,3 +2656,14 @@ void nsMenuPopupFrame::CheckForAnchorChange(nsRect& aRect) {
     SetPopupPosition(true);
   }
 }
+
+nsIWidget* nsMenuPopupFrame::GetParentMenuWidget() {
+  nsMenuFrame* menuFrame = do_QueryFrame(GetParent());
+  if (menuFrame) {
+    nsMenuParent* parentPopup = menuFrame->GetMenuParent();
+    if (parentPopup && (parentPopup->IsMenu() || parentPopup->IsMenuBar())) {
+      return static_cast<nsMenuPopupFrame*>(parentPopup)->GetWidget();
+    }
+  }
+  return nullptr;
+}

layout/xul/nsMenuPopupFrame.h

@@ -18,11 +18,10 @@
 #include "nsAtom.h"
 #include "nsGkAtoms.h"
 #include "nsCOMPtr.h"
-#include "nsIDOMEventListener.h"
-#include "nsIReflowCallback.h"
-#include "nsXULPopupManager.h"
+#include "nsMenuFrame.h"
 
 #include "nsBoxFrame.h"
+#include "nsMenuParent.h"
 
 #include "Units.h"
 
@@ -32,8 +31,6 @@ namespace mozilla {
 class PresShell;
 namespace dom {
 class KeyboardEvent;
-class XULButtonElement;
-class XULPopupElement;
 }  // namespace dom
 }  // namespace mozilla
 
@@ -127,7 +124,9 @@ class nsXULPopupShownEvent final : public mozilla::Runnable,
   const RefPtr<nsPresContext> mPresContext;
 };
 
-class nsMenuPopupFrame final : public nsBoxFrame, public nsIReflowCallback {
+class nsMenuPopupFrame final : public nsBoxFrame,
+                               public nsMenuParent,
+                               public nsIReflowCallback {
  public:
   NS_DECL_QUERYFRAME
   NS_DECL_FRAMEARENA_HELPERS(nsMenuPopupFrame)
@@ -135,11 +134,27 @@ class nsMenuPopupFrame final : public nsBoxFrame, public nsIReflowCallback {
   explicit nsMenuPopupFrame(ComputedStyle* aStyle, nsPresContext* aPresContext);
   ~nsMenuPopupFrame();
 
+  // nsMenuParent interface
+  virtual nsMenuFrame* GetCurrentMenuItem() override;
+  MOZ_CAN_RUN_SCRIPT_BOUNDARY
+  NS_IMETHOD SetCurrentMenuItem(nsMenuFrame* aMenuItem) override;
+  virtual void CurrentMenuIsBeingDestroyed() override;
+  MOZ_CAN_RUN_SCRIPT_BOUNDARY
+  NS_IMETHOD ChangeMenuItem(nsMenuFrame* aMenuItem, bool aSelectFirstItem,
+                            bool aFromKey) override;
+
   // as popups are opened asynchronously, the popup pending state is used to
   // prevent multiple requests from attempting to open the same popup twice
   nsPopupState PopupState() { return mPopupState; }
   void SetPopupState(nsPopupState);
 
+  NS_IMETHOD SetActive(bool aActiveFlag) override {
+    // We don't care.
+    return NS_OK;
+  }
+  virtual bool IsActive() override { return false; }
+  virtual bool IsMenuBar() override { return false; }
+
   /*
    * When this popup is open, should clicks outside of it be consumed?
    * Return true if the popup should rollup on an outside click,
@@ -158,12 +173,17 @@ class nsMenuPopupFrame final : public nsBoxFrame, public nsIReflowCallback {
    */
   ConsumeOutsideClicksResult ConsumeOutsideClicks();
 
-  mozilla::dom::XULPopupElement& PopupElement() const;
-
   void Reflow(nsPresContext* aPresContext, ReflowOutput& aDesiredSize,
               const ReflowInput& aReflowInput,
               nsReflowStatus& aStatus) override;
 
+  bool IsContextMenu() override { return mIsContextMenu; }
+
+  bool MenuClosed() override { return true; }
+
+  void LockMenuUntilClosed(bool aLock) override;
+  bool IsMenuLocked() override { return mIsMenuLocked; }
+
   nsIWidget* GetWidget() const;
 
   // Overridden methods
@@ -173,9 +193,8 @@ class nsMenuPopupFrame final : public nsBoxFrame, public nsIReflowCallback {
   virtual nsresult AttributeChanged(int32_t aNameSpaceID, nsAtom* aAttribute,
                                     int32_t aModType) override;
 
-  // FIXME: This shouldn't run script (this can end up calling HidePopup).
-  MOZ_CAN_RUN_SCRIPT_BOUNDARY void DestroyFrom(
-      nsIFrame* aDestructRoot, PostDestroyData& aPostDestroyData) override;
+  virtual void DestroyFrom(nsIFrame* aDestructRoot,
+                           PostDestroyData& aPostDestroyData) override;
 
   bool HasRemoteContent() const;
 
@@ -196,8 +215,6 @@ class nsMenuPopupFrame final : public nsBoxFrame, public nsIReflowCallback {
   // creates a new one, regardless of whether one has already been created.
   void PrepareWidget(bool aRecreate = false);
 
-  MOZ_CAN_RUN_SCRIPT void EnsureActiveMenuListItemIsVisible();
-
   nsresult CreateWidgetForView(nsView* aView);
   mozilla::StyleWindowShadow GetShadowStyle();
 
@@ -213,28 +230,17 @@ class nsMenuPopupFrame final : public nsBoxFrame, public nsIReflowCallback {
   // popup is being moved, and should not be flipped.
   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.
-  // Also, calling Enter will reset the current incremental search string,
-  // calculated in FindMenuWithShortcut.
-  MOZ_CAN_RUN_SCRIPT void HandleEnterKeyPress(mozilla::WidgetEvent&);
-
-  // Locate and return the menu frame that should be activated for the supplied
-  // key event. If aDoAction is set to true by this method, then the menu's
-  // action should be carried out, as if the user had pressed the Enter key. If
-  // aDoAction is false, the menu should just be highlighted.
-  // This method also handles incremental searching in menus so the user can
-  // type the first few letters of an item/s name to select it.
-  mozilla::dom::XULButtonElement* FindMenuWithShortcut(
-      mozilla::dom::KeyboardEvent& aKeyEvent, bool& aDoAction);
-
-  mozilla::dom::XULButtonElement* GetCurrentMenuItem() const;
-  nsIFrame* GetCurrentMenuItemFrame() const;
+  // 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
+  // should be opened as a result, this method should return the frame for
+  // that menu, or null if no menu should be opened. Also, calling Enter will
+  // reset the current incremental search string, calculated in
+  // FindMenuWithShortcut.
+  nsMenuFrame* Enter(mozilla::WidgetGUIEvent* aEvent);
 
   nsPopupType PopupType() const { return mPopupType; }
-  bool IsContextMenu() const { return mIsContextMenu; }
-
-  bool IsOpen() const {
+  bool IsMenu() override { return mPopupType == ePopupTypeMenu; }
+  bool IsOpen() override {
     return mPopupState == ePopupOpening || mPopupState == ePopupVisible ||
            mPopupState == ePopupShown;
   }
@@ -289,8 +295,16 @@ class nsMenuPopupFrame final : public nsBoxFrame, public nsIReflowCallback {
   void ShowPopup(bool aIsContextMenu);
   // indicate that the popup should be hidden. The new state should either be
   // ePopupClosed or ePopupInvisible.
-  MOZ_CAN_RUN_SCRIPT void HidePopup(bool aDeselectMenu, nsPopupState aNewState,
-                                    bool aFromFrameDestruction = false);
+  void HidePopup(bool aDeselectMenu, nsPopupState aNewState);
+
+  // locate and return the menu frame that should be activated for the
+  // supplied key event. If doAction is set to true by this method,
+  // then the menu's action should be carried out, as if the user had pressed
+  // the Enter key. If doAction is false, the menu should just be highlighted.
+  // This method also handles incremental searching in menus so the user can
+  // type the first few letters of an item/s name to select it.
+  nsMenuFrame* FindMenuWithShortcut(mozilla::dom::KeyboardEvent* aKeyEvent,
+                                    bool& doAction);
 
   void ClearIncrementalString() { mIncrementalString.Truncate(); }
   static bool IsWithinIncrementalTime(mozilla::TimeStamp time) {
@@ -305,7 +319,9 @@ class nsMenuPopupFrame final : public nsBoxFrame, public nsIReflowCallback {
   }
 #endif
 
-  MOZ_CAN_RUN_SCRIPT void ChangeByPage(bool aIsUp);
+  MOZ_CAN_RUN_SCRIPT void EnsureMenuItemIsVisible(nsMenuFrame* aMenuFrame);
+
+  void ChangeByPage(bool aIsUp);
 
   // Move the popup to the screen coordinate |aPos| in CSS pixels.
   // If aUpdateAttrs is true, and the popup already has left or top attributes,
@@ -527,6 +543,7 @@ class nsMenuPopupFrame final : public nsBoxFrame, public nsIReflowCallback {
   // was clicked. It will be cleared when the popup is hidden.
   nsCOMPtr<nsIContent> mTriggerContent;
 
+  nsMenuFrame* mCurrentMenu;  // The current menu that is active.
   nsView* mView;
 
   RefPtr<nsXULPopupShownEvent> mPopupShownDispatcher;
@@ -597,6 +614,7 @@ class nsMenuPopupFrame final : public nsBoxFrame, public nsIReflowCallback {
 
   bool mMenuCanOverlapOSBar;  // can we appear over the taskbar/menubar?
   bool mInContentShell;       // True if the popup is in a content shell
+  bool mIsMenuLocked;         // Should events inside this menu be ignored?
 
   // True if this popup has been offset due to moving off / near the edge of the
   // screen. (This is useful for ensuring that a move, which can't offset the

layout/xul/nsXULPopupManager.cpp

@@ -4,15 +4,11 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
-#include "XULButtonElement.h"
-#include "XULMenuParentElement.h"
 #include "mozilla/Assertions.h"
-#include "mozilla/Attributes.h"
-#include "mozilla/FlushType.h"
 #include "mozilla/UniquePtr.h"
 #include "nsGkAtoms.h"
-#include "nsISound.h"
 #include "nsXULPopupManager.h"
+#include "nsMenuFrame.h"
 #include "nsMenuPopupFrame.h"
 #include "nsMenuBarFrame.h"
 #include "nsMenuBarListener.h"
@@ -49,8 +45,6 @@
 #include "mozilla/dom/PopupPositionedEvent.h"
 #include "mozilla/dom/PopupPositionedEventBinding.h"
 #include "mozilla/dom/XULCommandEvent.h"
-#include "mozilla/dom/XULMenuElement.h"
-#include "mozilla/dom/XULPopupElement.h"
 #include "mozilla/EventDispatcher.h"
 #include "mozilla/EventStateManager.h"
 #include "mozilla/LookAndFeel.h"
@@ -229,7 +223,10 @@ void nsMenuChainItem::CheckForAnchorChange() {
 NS_IMPL_ISUPPORTS(nsXULPopupManager, nsIDOMEventListener, nsIObserver)
 
 nsXULPopupManager::nsXULPopupManager()
-    : mActiveMenuBar(nullptr), mPopups(nullptr), mPendingPopup(nullptr) {
+    : mActiveMenuBar(nullptr),
+      mPopups(nullptr),
+      mTimerMenu(nullptr),
+      mPendingPopup(nullptr) {
   nsCOMPtr<nsIObserverService> obs = mozilla::services::GetObserverService();
   if (obs) {
     obs->AddObserver(this, "xpcom-shutdown", false);
@@ -329,7 +326,7 @@ bool nsXULPopupManager::Rollup(uint32_t aCount, bool aFlush,
 
     ConsumeOutsideClicksResult consumeResult =
         item->Frame()->ConsumeOutsideClicks();
-    consume = consumeResult == ConsumeOutsideClicks_True;
+    consume = (consumeResult == ConsumeOutsideClicks_True);
 
     bool rollup = true;
 
@@ -711,21 +708,21 @@ auto nsXULPopupManager::MayShowMenu(nsIContent* aMenu) -> MayShowMenuResult {
     return {true};
   }
 
-  auto* menu = XULButtonElement::FromNode(aMenu);
-  if (!menu) {
+  nsMenuFrame* menuFrame = do_QueryFrame(aMenu->GetPrimaryFrame());
+  if (!menuFrame || !menuFrame->IsMenu()) {
     return {};
   }
 
-  nsMenuPopupFrame* popupFrame = menu->GetMenuPopup(FlushType::None);
+  nsMenuPopupFrame* popupFrame = menuFrame->GetPopup();
   if (!popupFrame || !MayShowPopup(popupFrame)) {
     return {};
   }
-  return {false, menu, popupFrame};
+  return {false, menuFrame, popupFrame};
 }
 
 void nsXULPopupManager::ShowMenu(nsIContent* aMenu, bool aSelectFirstItem) {
   auto mayShowResult = MayShowMenu(aMenu);
-  if (NS_WARN_IF(!mayShowResult)) {
+  if (!mayShowResult) {
     return;
   }
 
@@ -734,12 +731,19 @@ void nsXULPopupManager::ShowMenu(nsIContent* aMenu, bool aSelectFirstItem) {
     return;
   }
 
+  nsMenuFrame* menuFrame = mayShowResult.mMenuFrame;
   nsMenuPopupFrame* popupFrame = mayShowResult.mMenuPopupFrame;
 
   // inherit whether or not we're a context menu from the parent
-  const bool onMenuBar = mayShowResult.mMenuButton->IsOnMenuBar();
-  const bool onmenu = mayShowResult.mMenuButton->IsOnMenu();
-  const bool parentIsContextMenu = mayShowResult.mMenuButton->IsOnContextMenu();
+  bool parentIsContextMenu = false;
+  bool onMenuBar = false;
+  bool onmenu = menuFrame->IsOnMenu();
+
+  nsMenuParent* parent = menuFrame->GetMenuParent();
+  if (parent && onmenu) {
+    parentIsContextMenu = parent->IsContextMenu();
+    onMenuBar = parent->IsMenuBar();
+  }
 
   nsAutoString position;
 
@@ -768,9 +772,7 @@ void nsXULPopupManager::ShowPopup(nsIContent* aPopup,
                                   bool aAttributesOverride,
                                   bool aSelectFirstItem, Event* aTriggerEvent) {
   nsMenuPopupFrame* popupFrame = GetPopupFrameForContent(aPopup, true);
-  if (!popupFrame || !MayShowPopup(popupFrame)) {
-    return;
-  }
+  if (!popupFrame || !MayShowPopup(popupFrame)) return;
 
   PendingPopup pendingPopup(aPopup, aTriggerEvent);
   nsCOMPtr<nsIContent> triggerContent = pendingPopup.GetTriggerContent();
@@ -1065,8 +1067,8 @@ void nsXULPopupManager::ShowPopupCallback(nsIContent* aPopup,
 
   if (isMenu) {
     // if the menu is on a menubar, use the menubar's listener instead
-    if (auto* menu = aPopupFrame->PopupElement().GetContainingMenu()) {
-      item->SetOnMenuBar(menu->IsOnMenuBar());
+    if (nsMenuFrame* menuFrame = do_QueryFrame(aPopupFrame->GetParent())) {
+      item->SetOnMenuBar(menuFrame->IsOnMenuBar());
     }
   }
 
@@ -1090,8 +1092,10 @@ void nsXULPopupManager::ShowPopupCallback(nsIContent* aPopup,
   SetCaptureState(oldmenu);
   NS_ENSURE_TRUE_VOID(weakFrame.IsAlive());
 
-  RefPtr popup = &aPopupFrame->PopupElement();
-  popup->PopupOpened(aSelectFirstItem);
+  if (aSelectFirstItem) {
+    nsMenuFrame* next = GetNextMenuItem(aPopupFrame, nullptr, true, false);
+    aPopupFrame->SetCurrentMenuItem(next);
+  }
 
   if (isMenu) {
     UpdateMenuItems(aPopup);
@@ -1226,15 +1230,17 @@ void nsXULPopupManager::HideMenu(nsIContent* aMenu) {
     return;
   }
 
-  auto* button = XULButtonElement::FromNode(aMenu);
-  if (!button || !button->IsMenu()) {
+  nsMenuFrame* menu = do_QueryFrame(aMenu->GetPrimaryFrame(FlushType::Frames));
+  if (!menu) {
     return;
   }
-  auto* popup = button->GetMenuPopupContent();
-  if (!popup) {
+
+  nsMenuPopupFrame* popupFrame = menu->GetPopup();
+  if (!popupFrame) {
     return;
   }
-  HidePopup(popup, false, true, false, false);
+
+  HidePopup(popupFrame->GetContent(), false, true, false, false);
 }
 
 // This is used to hide the popup after a transition finishes.
@@ -1349,23 +1355,29 @@ void nsXULPopupManager::HidePopupCallback(
   }
 }
 
-void nsXULPopupManager::HidePopupAfterDelay(nsMenuPopupFrame* aPopup,
-                                            int32_t aDelay) {
+void nsXULPopupManager::HidePopupAfterDelay(nsMenuPopupFrame* aPopup) {
   // Don't close up immediately.
   // Kick off a close timer.
   KillMenuTimer();
 
+  int32_t menuDelay =
+      LookAndFeel::GetInt(LookAndFeel::IntID::SubmenuDelay, 300);  // ms
+
   // Kick off the timer.
-  nsIEventTarget* target =
-      aPopup->PopupElement().OwnerDoc()->EventTargetFor(TaskCategory::Other);
+  nsIEventTarget* target = nullptr;
+  if (nsIContent* content = aPopup->GetContent()) {
+    target = content->OwnerDoc()->EventTargetFor(TaskCategory::Other);
+  }
   NS_NewTimerWithFuncCallback(
       getter_AddRefs(mCloseTimer),
       [](nsITimer* aTimer, void* aClosure) {
-        if (nsXULPopupManager* pm = nsXULPopupManager::GetInstance()) {
+        nsXULPopupManager* pm = nsXULPopupManager::GetInstance();
+        if (pm) {
           pm->KillMenuTimer();
         }
       },
-      nullptr, aDelay, nsITimer::TYPE_ONE_SHOT, "KillMenuTimer", target);
+      nullptr, menuDelay, nsITimer::TYPE_ONE_SHOT, "KillMenuTimer", target);
+
   // the popup will call PopupDestroyed if it is destroyed, which checks if it
   // is set to mTimerMenu, so it should be safe to keep a reference to it
   mTimerMenu = aPopup;
@@ -1385,7 +1397,8 @@ void nsXULPopupManager::HidePopupsInList(
   for (f = 0; f < weakPopups.Length(); f++) {
     // check to ensure that the frame is still alive before hiding it.
     if (weakPopups[f].IsAlive()) {
-      auto* frame = static_cast<nsMenuPopupFrame*>(weakPopups[f].GetFrame());
+      nsMenuPopupFrame* frame =
+          static_cast<nsMenuPopupFrame*>(weakPopups[f].GetFrame());
       frame->HidePopup(true, ePopupInvisible);
     }
   }
@@ -1534,7 +1547,7 @@ void nsXULPopupManager::BeginShowingPopup(const PendingPopup& aPendingPopup,
   RefPtr<nsIContent> popup = aPendingPopup.mPopup;
 
   nsMenuPopupFrame* popupFrame = do_QueryFrame(popup->GetPrimaryFrame());
-  if (NS_WARN_IF(!popupFrame)) {
+  if (!popupFrame) {
     return;
   }
 
@@ -1608,8 +1621,8 @@ void nsXULPopupManager::FirePopupHidingEvent(
     bool aIsCancel) {
   nsCOMPtr<nsIContent> popup = aPopup;
   RefPtr<PresShell> presShell = aPresContext->PresShell();
-  Unused << presShell;  // This presShell may be keeping things alive
-                        // on non GTK platforms
+  mozilla::Unused << presShell;  // This presShell may be keeping things alive
+                                 // on non GTK platforms
 
   nsEventStatus status = nsEventStatus_eIgnore;
   WidgetMouseEvent event(true, eXULPopupHiding, nullptr,
@@ -1710,6 +1723,22 @@ bool nsXULPopupManager::IsPopupOpen(nsIContent* aPopup) {
   return false;
 }
 
+bool nsXULPopupManager::IsPopupOpenForMenuParent(nsMenuParent* aMenuParent) {
+  nsMenuChainItem* item = GetTopVisibleMenu();
+  while (item) {
+    nsMenuPopupFrame* popup = item->Frame();
+    if (popup && popup->IsOpen()) {
+      nsMenuFrame* menuFrame = do_QueryFrame(popup->GetParent());
+      if (menuFrame && menuFrame->GetMenuParent() == aMenuParent) {
+        return true;
+      }
+    }
+    item = item->GetParent();
+  }
+
+  return false;
+}
+
 nsIFrame* nsXULPopupManager::GetTopPopup(nsPopupType aType) {
   for (nsMenuChainItem* item = mPopups.get(); item; item = item->GetParent()) {
     if (item->Frame()->IsVisible() &&
@@ -1720,18 +1749,6 @@ nsIFrame* nsXULPopupManager::GetTopPopup(nsPopupType aType) {
   return nullptr;
 }
 
-nsIContent* nsXULPopupManager::GetTopActiveMenuItemContent() {
-  for (nsMenuChainItem* item = mPopups.get(); item; item = item->GetParent()) {
-    if (!item->Frame()->IsVisible()) {
-      continue;
-    }
-    if (auto* content = item->Frame()->PopupElement().GetActiveMenuChild()) {
-      return content;
-    }
-  }
-  return nullptr;
-}
-
 void nsXULPopupManager::GetVisiblePopups(nsTArray<nsIFrame*>& aPopups) {
   aPopups.Clear();
   for (nsMenuChainItem* item = mPopups.get(); item; item = item->GetParent()) {
@@ -1871,13 +1888,10 @@ bool nsXULPopupManager::MayShowPopup(nsMenuPopupFrame* aPopup) {
 #endif
 
   // cannot open a popup that is a submenu of a menupopup that isn't open.
-  if (auto* menu = aPopup->PopupElement().GetContainingMenu()) {
-    if (auto* parent = XULPopupElement::FromNodeOrNull(menu->GetMenuParent())) {
-      nsMenuPopupFrame* f = do_QueryFrame(parent->GetPrimaryFrame());
-      if (f && !f->IsOpen()) {
-        return false;
-      }
-    }
+  nsMenuFrame* menuFrame = do_QueryFrame(aPopup->GetParent());
+  if (menuFrame) {
+    nsMenuParent* parentPopup = menuFrame->GetMenuParent();
+    if (parentPopup && !parentPopup->IsOpen()) return false;
   }
 
   return true;
@@ -1885,7 +1899,13 @@ bool nsXULPopupManager::MayShowPopup(nsMenuPopupFrame* aPopup) {
 
 void nsXULPopupManager::PopupDestroyed(nsMenuPopupFrame* aPopup) {
   // when a popup frame is destroyed, just unhook it from the list of popups
-  CancelMenuTimer(aPopup);
+  if (mTimerMenu == aPopup) {
+    if (mCloseTimer) {
+      mCloseTimer->Cancel();
+      mCloseTimer = nullptr;
+    }
+    mTimerMenu = nullptr;
+  }
 
   nsMenuChainItem* item = FindPopup(aPopup->GetContent());
   if (!item) {
@@ -2111,70 +2131,63 @@ void nsXULPopupManager::KillMenuTimer() {
     mCloseTimer->Cancel();
     mCloseTimer = nullptr;
 
-    if (mTimerMenu->IsOpen()) {
+    if (mTimerMenu->IsOpen())
       HidePopup(mTimerMenu->GetContent(), false, false, true, false);
-    }
   }
 
   mTimerMenu = nullptr;
 }
 
-void nsXULPopupManager::CancelMenuTimer(nsMenuPopupFrame* aMenu) {
-  if (mCloseTimer && mTimerMenu == aMenu) {
+void nsXULPopupManager::CancelMenuTimer(nsMenuParent* aMenuParent) {
+  if (mCloseTimer && mTimerMenu == aMenuParent) {
     mCloseTimer->Cancel();
     mCloseTimer = nullptr;
     mTimerMenu = nullptr;
   }
 }
 
-bool nsXULPopupManager::HandleShortcutNavigation(KeyboardEvent& aKeyEvent,
+bool nsXULPopupManager::HandleShortcutNavigation(KeyboardEvent* aKeyEvent,
                                                  nsMenuPopupFrame* aFrame) {
   // On Windows, don't check shortcuts when the accelerator key is down.
 #ifdef XP_WIN
-  WidgetInputEvent* evt = aKeyEvent.WidgetEventPtr()->AsInputEvent();
+  WidgetInputEvent* evt = aKeyEvent->WidgetEventPtr()->AsInputEvent();
   if (evt && evt->IsAccel()) {
     return false;
   }
 #endif
 
-  if (!aFrame) {
-    if (nsMenuChainItem* item = GetTopVisibleMenu()) {
-      aFrame = item->Frame();
-    }
-  }
+  nsMenuChainItem* item = GetTopVisibleMenu();
+  if (!aFrame && item) aFrame = item->Frame();
 
   if (aFrame) {
-    bool action = false;
-    RefPtr result = aFrame->FindMenuWithShortcut(aKeyEvent, action);
-    if (!result) {
-      return false;
+    bool action;
+    nsMenuFrame* result = aFrame->FindMenuWithShortcut(aKeyEvent, action);
+    if (result) {
+      aFrame->ChangeMenuItem(result, false, true);
+      if (action) {
+        WidgetGUIEvent* evt = aKeyEvent->WidgetEventPtr()->AsGUIEvent();
+        nsMenuFrame* menuToOpen = result->Enter(evt);
+        if (menuToOpen) {
+          nsCOMPtr<nsIContent> content = menuToOpen->GetContent();
+          ShowMenu(content, true);
+        }
+      }
+      return true;
     }
-    RefPtr popup = &aFrame->PopupElement();
-    popup->SetActiveMenuChild(result, XULMenuParentElement::ByKey::Yes);
-    if (action) {
-      WidgetEvent* evt = aKeyEvent.WidgetEventPtr();
-      result->HandleEnterKeyPress(*evt);
-    }
-    return true;
+
+    return false;
   }
 
   if (mActiveMenuBar) {
-    RefPtr menubar = &mActiveMenuBar->MenubarElement();
-    if (RefPtr result = menubar->FindMenuWithShortcut(aKeyEvent)) {
-      result->OpenMenuPopup(true);
+    nsMenuFrame* result =
+        mActiveMenuBar->FindMenuWithShortcut(aKeyEvent, false);
+    if (result) {
+      mActiveMenuBar->SetActive(true);
+      result->OpenMenu(true);
       return true;
     }
-#ifdef XP_WIN
-    // Behavior on Windows - this item is on the menu bar, beep and deactivate
-    // the menu bar.
-    // TODO(emilio): This is rather odd, and I cannot get the beep to work,
-    // but this matches what old code was doing...
-    if (nsCOMPtr<nsISound> sound = do_GetService("@mozilla.org/sound;1")) {
-      sound->Beep();
-    }
-    mActiveMenuBar->SetActive(false);
-#endif
   }
+
   return false;
 }
 
@@ -2192,32 +2205,29 @@ bool nsXULPopupManager::HandleKeyboardNavigation(uint32_t aKeyCode) {
     item = nextitem;
     nextitem = item->GetParent();
 
-    if (!nextitem) {
-      break;
-    }
-    // stop if the parent isn't a menu
-    if (!nextitem->IsMenu()) {
-      break;
-    }
+    if (nextitem) {
+      // stop if the parent isn't a menu
+      if (!nextitem->IsMenu()) break;
 
-    // Check to make sure that the parent is actually the parent menu. It won't
-    // be if the parent is in a different frame hierarchy, for example, for a
-    // context menu opened on another menu.
-    XULPopupElement& expectedParent = nextitem->Frame()->PopupElement();
-    auto* menu = item->Frame()->PopupElement().GetContainingMenu();
-    if (!menu || menu->GetMenuParent() != &expectedParent) {
-      break;
+      // check to make sure that the parent is actually the parent menu. It
+      // won't be if the parent is in a different frame hierarchy, for example,
+      // for a context menu opened on another menu.
+      nsMenuParent* expectedParent =
+          static_cast<nsMenuParent*>(nextitem->Frame());
+      nsMenuFrame* menuFrame = do_QueryFrame(item->Frame()->GetParent());
+      if (!menuFrame || menuFrame->GetMenuParent() != expectedParent) {
+        break;
+      }
     }
   }
 
   nsIFrame* itemFrame;
-  if (item) {
+  if (item)
     itemFrame = item->Frame();
-  } else if (mActiveMenuBar) {
+  else if (mActiveMenuBar)
     itemFrame = mActiveMenuBar;
-  } else {
+  else
     return false;
-  }
 
   nsNavigationDirection theDirection;
   NS_ASSERTION(aKeyCode >= KeyboardEvent_Binding::DOM_VK_END &&
@@ -2227,50 +2237,43 @@ bool nsXULPopupManager::HandleKeyboardNavigation(uint32_t aKeyCode) {
 
   bool selectFirstItem = true;
 #ifdef MOZ_WIDGET_GTK
-  {
-    XULButtonElement* currentItem = nullptr;
-    if (item && mActiveMenuBar && NS_DIRECTION_IS_INLINE(theDirection)) {
-      currentItem = item->Frame()->PopupElement().GetActiveMenuChild();
-      // If nothing is selected in the menu and we have a menubar, let it
-      // handle the movement not to steal focus from it.
-      if (!currentItem) {
-        item = nullptr;
-      }
+  nsMenuFrame* currentItem = nullptr;
+  if (item && mActiveMenuBar && NS_DIRECTION_IS_INLINE(theDirection)) {
+    currentItem = item->Frame()->GetCurrentMenuItem();
+    // If nothing is selected in the menu and we have a menubar, let it
+    // handle the movement not to steal focus from it.
+    if (!currentItem) {
+      item = nullptr;
     }
-    // On menu change, only select first item if an item is already selected.
-    selectFirstItem = !!currentItem;
   }
+  // On menu change, only select first item if an item is already selected.
+  selectFirstItem = currentItem != nullptr;
 #endif
 
   // if a popup is open, first check for navigation within the popup
-  if (item && HandleKeyboardNavigationInPopup(item, theDirection)) {
-    return true;
-  }
+  if (item && HandleKeyboardNavigationInPopup(item, theDirection)) return true;
 
   // no popup handled the key, so check the active menubar, if any
-  if (!mActiveMenuBar) {
-    return false;
-  }
-  RefPtr menubar = XULMenuParentElement::FromNode(mActiveMenuBar->GetContent());
-  if (NS_DIRECTION_IS_INLINE(theDirection)) {
-    RefPtr prevActiveItem = menubar->GetActiveMenuChild();
-    const bool open = prevActiveItem && prevActiveItem->IsMenuPopupOpen();
-    RefPtr nextItem = theDirection == eNavigationDirection_End
-                          ? menubar->GetNextMenuItem()
-                          : menubar->GetPrevMenuItem();
-    menubar->SetActiveMenuChild(nextItem, XULMenuParentElement::ByKey::Yes);
-    if (open && nextItem) {
-      nextItem->OpenMenuPopup(selectFirstItem);
+  if (mActiveMenuBar) {
+    nsMenuFrame* currentMenu = mActiveMenuBar->GetCurrentMenuItem();
+
+    if (NS_DIRECTION_IS_INLINE(theDirection)) {
+      nsMenuFrame* nextItem =
+          (theDirection == eNavigationDirection_End)
+              ? GetNextMenuItem(mActiveMenuBar, currentMenu, false, true)
+              : GetPreviousMenuItem(mActiveMenuBar, currentMenu, false, true);
+      mActiveMenuBar->ChangeMenuItem(nextItem, selectFirstItem, true);
+      return true;
+    } else if (NS_DIRECTION_IS_BLOCK(theDirection)) {
+      // Open the menu and select its first item.
+      if (currentMenu) {
+        nsCOMPtr<nsIContent> content = currentMenu->GetContent();
+        ShowMenu(content, true);
+      }
+      return true;
     }
-    return true;
-  }
-  if (NS_DIRECTION_IS_BLOCK(theDirection)) {
-    // Open the menu and select its first item.
-    if (RefPtr currentMenu = menubar->GetActiveMenuChild()) {
-      ShowMenu(currentMenu, selectFirstItem);
-    }
-    return true;
   }
+
   return false;
 }
 
@@ -2281,79 +2284,81 @@ bool nsXULPopupManager::HandleKeyboardNavigationInPopup(
   NS_ASSERTION(!item || item->Frame() == aFrame,
                "aFrame is expected to be equal to item->Frame()");
 
-  using Wrap = XULMenuParentElement::Wrap;
-  RefPtr<XULPopupElement> menu = &aFrame->PopupElement();
+  nsMenuFrame* currentMenu = aFrame->GetCurrentMenuItem();
 
   aFrame->ClearIncrementalString();
-  RefPtr currentItem = aFrame->GetCurrentMenuItem();
 
   // This method only gets called if we're open.
-  if (!currentItem && NS_DIRECTION_IS_INLINE(aDir)) {
+  if (!currentMenu && NS_DIRECTION_IS_INLINE(aDir)) {
     // We've been opened, but we haven't had anything selected.
     // We can handle End, but our parent handles Start.
     if (aDir == eNavigationDirection_End) {
-      if (RefPtr nextItem = menu->GetNextMenuItem(Wrap::No)) {
-        menu->SetActiveMenuChild(nextItem, XULMenuParentElement::ByKey::Yes);
+      nsMenuFrame* nextItem = GetNextMenuItem(aFrame, nullptr, true, false);
+      if (nextItem) {
+        aFrame->ChangeMenuItem(nextItem, false, true);
         return true;
       }
     }
     return false;
   }
 
-  const bool isContainer = currentItem && !currentItem->IsMenuItem();
-  const bool isOpen = currentItem && currentItem->IsMenuPopupOpen();
-  if (isOpen) {
-    // For an open popup, have the child process the event
-    nsMenuChainItem* child = item ? item->GetChild() : nullptr;
-    if (child && HandleKeyboardNavigationInPopup(child, aDir)) {
+  bool isContainer = false;
+  bool isOpen = false;
+  if (currentMenu) {
+    isOpen = currentMenu->IsOpen();
+    isContainer = currentMenu->IsMenu();
+    if (isOpen) {
+      // for an open popup, have the child process the event
+      nsMenuChainItem* child = item ? item->GetChild() : nullptr;
+      if (child && HandleKeyboardNavigationInPopup(child, aDir)) return true;
+    } else if (aDir == eNavigationDirection_End && isContainer &&
+               !currentMenu->IsDisabled()) {
+      // The menu is not yet open. Open it and select the first item.
+      nsCOMPtr<nsIContent> content = currentMenu->GetContent();
+      ShowMenu(content, true);
       return true;
     }
-  } else if (aDir == eNavigationDirection_End && isContainer &&
-             !currentItem->IsDisabled()) {
-    currentItem->OpenMenuPopup(true);
-    return true;
   }
 
   // For block progression, we can move in either direction
   if (NS_DIRECTION_IS_BLOCK(aDir) || NS_DIRECTION_IS_BLOCK_TO_EDGE(aDir)) {
-    RefPtr<XULButtonElement> nextItem = nullptr;
+    nsMenuFrame* nextItem;
 
     if (aDir == eNavigationDirection_Before ||
         aDir == eNavigationDirection_After) {
       // Cursor navigation does not wrap on Mac or for menulists on Windows.
-      auto wrap =
+      bool wrap =
 #ifdef XP_WIN
-          aFrame->IsMenuList() ? Wrap::No : Wrap::Yes;
+          !aFrame->IsMenuList();
 #elif defined XP_MACOSX
-          Wrap::No;
+          false;
 #else
-          Wrap::Yes;
+          true;
 #endif
 
       if (aDir == eNavigationDirection_Before) {
-        nextItem = menu->GetPrevMenuItem(wrap);
+        nextItem = GetPreviousMenuItem(aFrame, currentMenu, true, wrap);
       } else {
-        nextItem = menu->GetNextMenuItem(wrap);
+        nextItem = GetNextMenuItem(aFrame, currentMenu, true, wrap);
       }
     } else if (aDir == eNavigationDirection_First) {
-      nextItem = menu->GetFirstMenuItem();
+      nextItem = GetNextMenuItem(aFrame, nullptr, true, false);
     } else {
-      nextItem = menu->GetLastMenuItem();
+      nextItem = GetPreviousMenuItem(aFrame, nullptr, true, false);
     }
 
     if (nextItem) {
-      menu->SetActiveMenuChild(nextItem, XULMenuParentElement::ByKey::Yes);
+      aFrame->ChangeMenuItem(nextItem, false, true);
       return true;
     }
-  } else if (currentItem && isOpen && aDir == eNavigationDirection_Start) {
-    // close a submenu when Left is pressed
-    if (nsMenuPopupFrame* popupFrame =
-            currentItem->GetMenuPopup(FlushType::None)) {
-      HidePopup(popupFrame->GetContent(), /* aHideChain = */ false,
-                /* aDeselectMenu = */ false, /* aAsynchronous = */ false,
-                /* aIsCancel = */ false);
+  } else if (currentMenu && isContainer && isOpen) {
+    if (aDir == eNavigationDirection_Start) {
+      // close a submenu when Left is pressed
+      nsMenuPopupFrame* popupFrame = currentMenu->GetPopup();
+      if (popupFrame)
+        HidePopup(popupFrame->GetContent(), false, false, false, false);
+      return true;
     }
-    return true;
   }
 
   return false;
@@ -2421,10 +2426,10 @@ bool nsXULPopupManager::HandleKeyboardEventWithKeyCode(
     case KeyboardEvent_Binding::DOM_VK_F10:
 #endif
       if (aTopVisibleMenuItem &&
-          !aTopVisibleMenuItem->Frame()->PopupElement().AttrValueIs(
+          !aTopVisibleMenuItem->Frame()->GetContent()->AsElement()->AttrValueIs(
               kNameSpaceID_None, nsGkAtoms::activateontab, nsGkAtoms::_true,
               eCaseMatters)) {
-        // Close popups or deactivate menubar when Tab or F10 are pressed
+        // close popups or deactivate menubar when Tab or F10 are pressed
         Rollup(0, false, nullptr, nullptr);
         break;
       } else if (mActiveMenuBar) {
@@ -2438,11 +2443,17 @@ bool nsXULPopupManager::HandleKeyboardEventWithKeyCode(
       // If there is a popup open, check if the current item needs to be opened.
       // Otherwise, tell the active menubar, if any, to activate the menu. The
       // Enter method will return a menu if one needs to be opened as a result.
-      WidgetEvent* event = aKeyEvent->WidgetEventPtr();
+      nsMenuFrame* menuToOpen = nullptr;
+      WidgetGUIEvent* GUIEvent = aKeyEvent->WidgetEventPtr()->AsGUIEvent();
+
       if (aTopVisibleMenuItem) {
-        aTopVisibleMenuItem->Frame()->HandleEnterKeyPress(*event);
+        menuToOpen = aTopVisibleMenuItem->Frame()->Enter(GUIEvent);
       } else if (mActiveMenuBar) {
-        mActiveMenuBar->HandleEnterKeyPress(*event);
+        menuToOpen = mActiveMenuBar->Enter(GUIEvent);
+      }
+      if (menuToOpen) {
+        nsCOMPtr<nsIContent> content = menuToOpen->GetContent();
+        ShowMenu(content, true);
       }
       break;
     }
@@ -2459,6 +2470,173 @@ bool nsXULPopupManager::HandleKeyboardEventWithKeyCode(
   return true;
 }
 
+// TODO(emilio): This should probably just walk the DOM instead and call
+// GetPrimaryFrame() on the items... Do we have anonymous / fallback menu items
+// that could be selectable?
+static nsIContent* FindDefaultInsertionPoint(nsIContent* aParent) {
+  if (ShadowRoot* shadow = aParent->GetShadowRoot()) {
+    if (HTMLSlotElement* slot = shadow->GetDefaultSlot()) {
+      return slot;
+    }
+  }
+  return aParent;
+}
+
+nsContainerFrame* nsXULPopupManager::ImmediateParentFrame(
+    nsContainerFrame* aFrame) {
+  MOZ_ASSERT(aFrame && aFrame->GetContent());
+  nsIContent* insertionPoint = FindDefaultInsertionPoint(aFrame->GetContent());
+
+  nsCSSFrameConstructor* fc = aFrame->PresContext()->FrameConstructor();
+  nsContainerFrame* insertionFrame =
+      insertionPoint ? fc->GetContentInsertionFrameFor(insertionPoint)
+                     : nullptr;
+
+  return insertionFrame ? insertionFrame : aFrame;
+}
+
+nsMenuFrame* nsXULPopupManager::GetNextMenuItem(nsContainerFrame* aParent,
+                                                nsMenuFrame* aStart,
+                                                bool aIsPopup, bool aWrap) {
+  nsContainerFrame* immediateParent = ImmediateParentFrame(aParent);
+  nsIFrame* currFrame = nullptr;
+  if (aStart) {
+    if (aStart->GetNextSibling())
+      currFrame = aStart->GetNextSibling();
+    else if (aStart->GetParent()->GetContent()->IsXULElement(
+                 nsGkAtoms::menugroup))
+      currFrame = aStart->GetParent()->GetNextSibling();
+  } else
+    currFrame = immediateParent->PrincipalChildList().FirstChild();
+
+  while (currFrame) {
+    // See if it's a menu item.
+    nsIContent* currFrameContent = currFrame->GetContent();
+    if (IsValidMenuItem(currFrameContent, aIsPopup)) {
+      return do_QueryFrame(currFrame);
+    }
+    if (currFrameContent->IsXULElement(nsGkAtoms::menugroup) &&
+        currFrameContent->GetChildCount() > 0)
+      currFrame = currFrame->PrincipalChildList().FirstChild();
+    else if (!currFrame->GetNextSibling() &&
+             currFrame->GetParent()->GetContent()->IsXULElement(
+                 nsGkAtoms::menugroup))
+      currFrame = currFrame->GetParent()->GetNextSibling();
+    else
+      currFrame = currFrame->GetNextSibling();
+  }
+
+  if (!aWrap) {
+    return aStart;
+  }
+
+  currFrame = immediateParent->PrincipalChildList().FirstChild();
+
+  // Still don't have anything. Try cycling from the beginning.
+  while (currFrame && currFrame != aStart) {
+    // See if it's a menu item.
+    nsIContent* currFrameContent = currFrame->GetContent();
+    if (IsValidMenuItem(currFrameContent, aIsPopup)) {
+      return do_QueryFrame(currFrame);
+    }
+    if (currFrameContent->IsXULElement(nsGkAtoms::menugroup) &&
+        currFrameContent->GetChildCount() > 0)
+      currFrame = currFrame->PrincipalChildList().FirstChild();
+    else if (!currFrame->GetNextSibling() &&
+             currFrame->GetParent()->GetContent()->IsXULElement(
+                 nsGkAtoms::menugroup))
+      currFrame = currFrame->GetParent()->GetNextSibling();
+    else
+      currFrame = currFrame->GetNextSibling();
+  }
+
+  // No luck. Just return our start value.
+  return aStart;
+}
+
+nsMenuFrame* nsXULPopupManager::GetPreviousMenuItem(nsContainerFrame* aParent,
+                                                    nsMenuFrame* aStart,
+                                                    bool aIsPopup, bool aWrap) {
+  nsContainerFrame* immediateParent = ImmediateParentFrame(aParent);
+  const nsFrameList& frames(immediateParent->PrincipalChildList());
+
+  nsIFrame* currFrame = nullptr;
+  if (aStart) {
+    if (aStart->GetPrevSibling())
+      currFrame = aStart->GetPrevSibling();
+    else if (aStart->GetParent()->GetContent()->IsXULElement(
+                 nsGkAtoms::menugroup))
+      currFrame = aStart->GetParent()->GetPrevSibling();
+  } else
+    currFrame = frames.LastChild();
+
+  while (currFrame) {
+    // See if it's a menu item.
+    nsIContent* currFrameContent = currFrame->GetContent();
+    if (IsValidMenuItem(currFrameContent, aIsPopup)) {
+      return do_QueryFrame(currFrame);
+    }
+    if (currFrameContent->IsXULElement(nsGkAtoms::menugroup) &&
+        currFrameContent->GetChildCount() > 0) {
+      const nsFrameList& menugroupFrames(currFrame->PrincipalChildList());
+      currFrame = menugroupFrames.LastChild();
+    } else if (!currFrame->GetPrevSibling() &&
+               currFrame->GetParent()->GetContent()->IsXULElement(
+                   nsGkAtoms::menugroup))
+      currFrame = currFrame->GetParent()->GetPrevSibling();
+    else
+      currFrame = currFrame->GetPrevSibling();
+  }
+
+  if (!aWrap) {
+    return aStart;
+  }
+
+  currFrame = frames.LastChild();
+
+  // Still don't have anything. Try cycling from the end.
+  while (currFrame && currFrame != aStart) {
+    // See if it's a menu item.
+    nsIContent* currFrameContent = currFrame->GetContent();
+    if (IsValidMenuItem(currFrameContent, aIsPopup)) {
+      return do_QueryFrame(currFrame);
+    }
+    if (currFrameContent->IsXULElement(nsGkAtoms::menugroup) &&
+        currFrameContent->GetChildCount() > 0) {
+      const nsFrameList& menugroupFrames(currFrame->PrincipalChildList());
+      currFrame = menugroupFrames.LastChild();
+    } else if (!currFrame->GetPrevSibling() &&
+               currFrame->GetParent()->GetContent()->IsXULElement(
+                   nsGkAtoms::menugroup))
+      currFrame = currFrame->GetParent()->GetPrevSibling();
+    else
+      currFrame = currFrame->GetPrevSibling();
+  }
+
+  // No luck. Just return our start value.
+  return aStart;
+}
+
+bool nsXULPopupManager::IsValidMenuItem(nsIContent* aContent, bool aOnPopup) {
+  if (!aContent->IsAnyOfXULElements(nsGkAtoms::menu, nsGkAtoms::menuitem)) {
+    return false;
+  }
+
+  nsMenuFrame* menuFrame = do_QueryFrame(aContent->GetPrimaryFrame());
+
+  bool skipNavigatingDisabledMenuItem = true;
+  if (aOnPopup && (!menuFrame || !menuFrame->IsParentMenuList())) {
+    skipNavigatingDisabledMenuItem =
+        LookAndFeel::GetInt(LookAndFeel::IntID::SkipNavigatingDisabledMenuItem,
+                            0) != 0;
+  }
+
+  return !(skipNavigatingDisabledMenuItem && aContent->IsElement() &&
+           aContent->AsElement()->AttrValueIs(kNameSpaceID_None,
+                                              nsGkAtoms::disabled,
+                                              nsGkAtoms::_true, eCaseMatters));
+}
+
 nsresult nsXULPopupManager::HandleEvent(Event* aEvent) {
   RefPtr<KeyboardEvent> keyEvent = aEvent->AsKeyboardEvent();
   NS_ENSURE_TRUE(keyEvent, NS_ERROR_UNEXPECTED);
@@ -2533,9 +2711,7 @@ nsresult nsXULPopupManager::KeyUp(KeyboardEvent* aKeyEvent) {
 
 nsresult nsXULPopupManager::KeyDown(KeyboardEvent* aKeyEvent) {
   nsMenuChainItem* item = GetTopVisibleMenu();
-  if (item && item->Frame()->PopupElement().IsLocked()) {
-    return NS_OK;
-  }
+  if (item && item->Frame()->IsMenuLocked()) return NS_OK;
 
   if (HandleKeyboardEventWithKeyCode(aKeyEvent, item)) {
     return NS_OK;
@@ -2594,8 +2770,8 @@ nsresult nsXULPopupManager::KeyPress(KeyboardEvent* aKeyEvent) {
   // events.
 
   nsMenuChainItem* item = GetTopVisibleMenu();
-  if (item && (item->Frame()->PopupElement().IsLocked() ||
-               item->PopupType() != ePopupTypeMenu)) {
+  if (item &&
+      (item->Frame()->IsMenuLocked() || item->PopupType() != ePopupTypeMenu)) {
     return NS_OK;
   }
 
@@ -2613,7 +2789,7 @@ nsresult nsXULPopupManager::KeyPress(KeyboardEvent* aKeyEvent) {
     consume = false;
   }
 
-  HandleShortcutNavigation(*aKeyEvent, nullptr);
+  HandleShortcutNavigation(aKeyEvent, nullptr);
 
   aKeyEvent->StopCrossProcessForwarding();
   if (consume) {
@@ -2687,9 +2863,8 @@ static void AlignmentPositionToString(nsMenuPopupFrame* aFrame,
 }
 
 NS_IMETHODIMP
-MOZ_CAN_RUN_SCRIPT_BOUNDARY
 nsXULPopupPositionedEvent::Run() {
-  RefPtr<nsXULPopupManager> pm = nsXULPopupManager::GetInstance();
+  nsXULPopupManager* pm = nsXULPopupManager::GetInstance();
   if (!pm) {
     return NS_OK;
   }
@@ -2744,53 +2919,58 @@ nsXULPopupPositionedEvent::Run() {
 NS_IMETHODIMP
 nsXULMenuCommandEvent::Run() {
   nsXULPopupManager* pm = nsXULPopupManager::GetInstance();
-  if (!pm) {
-    return NS_OK;
-  }
-
-  RefPtr menu = XULButtonElement::FromNode(mMenu);
-  MOZ_ASSERT(menu);
-  if (mFlipChecked) {
-    if (menu->GetXULBoolAttr(nsGkAtoms::checked)) {
-      menu->UnsetAttr(kNameSpaceID_None, nsGkAtoms::checked, true);
-    } else {
-      menu->SetAttr(kNameSpaceID_None, nsGkAtoms::checked, u"true"_ns, true);
-    }
-  }
+  if (!pm) return NS_OK;
 
   // The order of the nsViewManager and PresShell COM pointers is
   // important below.  We want the pres shell to get released before the
   // associated view manager on exit from this function.
   // See bug 54233.
   // XXXndeakin is this still needed?
-  RefPtr<nsPresContext> presContext = menu->OwnerDoc()->GetPresContext();
-  RefPtr<PresShell> presShell =
-      presContext ? presContext->PresShell() : nullptr;
-  RefPtr<nsViewManager> kungFuDeathGrip =
-      presShell ? presShell->GetViewManager() : nullptr;
-  Unused << kungFuDeathGrip;  // Not referred to directly within this function
 
-  // Deselect ourselves.
-  if (mCloseMenuMode != CloseMenuMode_None) {
-    if (RefPtr parent = menu->GetMenuParent()) {
-      if (parent->GetActiveMenuChild() == menu) {
-        parent->SetActiveMenuChild(nullptr);
+  nsCOMPtr<nsIContent> popup;
+  nsMenuFrame* menuFrame = do_QueryFrame(mMenu->GetPrimaryFrame());
+  AutoWeakFrame weakFrame(menuFrame);
+  if (menuFrame && mFlipChecked) {
+    if (menuFrame->IsChecked()) {
+      mMenu->UnsetAttr(kNameSpaceID_None, nsGkAtoms::checked, true);
+    } else {
+      mMenu->SetAttr(kNameSpaceID_None, nsGkAtoms::checked, u"true"_ns, true);
+    }
+  }
+
+  if (menuFrame && weakFrame.IsAlive()) {
+    // Find the popup that the menu is inside. Below, this popup will
+    // need to be hidden.
+    nsIFrame* frame = menuFrame->GetParent();
+    while (frame) {
+      nsMenuPopupFrame* popupFrame = do_QueryFrame(frame);
+      if (popupFrame) {
+        popup = popupFrame->GetContent();
+        break;
       }
+      frame = frame->GetParent();
     }
+
+    nsPresContext* presContext = menuFrame->PresContext();
+    RefPtr<PresShell> presShell = presContext->PresShell();
+    RefPtr<nsViewManager> kungFuDeathGrip = presShell->GetViewManager();
+    mozilla::Unused
+        << kungFuDeathGrip;  // Not referred to directly within this function
+
+    // Deselect ourselves.
+    if (mCloseMenuMode != CloseMenuMode_None) menuFrame->SelectMenu(false);
+
+    AutoHandlingUserInputStatePusher userInpStatePusher(mUserInput);
+    RefPtr<Element> menu = mMenu;
+    nsContentUtils::DispatchXULCommand(
+        menu, mIsTrusted, nullptr, presShell, mModifiers & MODIFIER_CONTROL,
+        mModifiers & MODIFIER_ALT, mModifiers & MODIFIER_SHIFT,
+        mModifiers & MODIFIER_META, 0, mButton);
   }
 
-  AutoHandlingUserInputStatePusher userInpStatePusher(mUserInput);
-  nsContentUtils::DispatchXULCommand(
-      menu, mIsTrusted, nullptr, presShell, mModifiers & MODIFIER_CONTROL,
-      mModifiers & MODIFIER_ALT, mModifiers & MODIFIER_SHIFT,
-      mModifiers & MODIFIER_META, 0, mButton);
-
-  if (mCloseMenuMode != CloseMenuMode_None) {
-    if (RefPtr popup = menu->GetContainingPopupElement()) {
-      pm->HidePopup(popup, mCloseMenuMode == CloseMenuMode_Auto, true, false,
-                    false);
-    }
-  }
+  if (popup && mCloseMenuMode != CloseMenuMode_None)
+    pm->HidePopup(popup, mCloseMenuMode == CloseMenuMode_Auto, true, false,
+                  false);
 
   return NS_OK;
 }

layout/xul/nsXULPopupManager.h

@@ -54,8 +54,10 @@
  */
 
 class nsContainerFrame;
+class nsMenuFrame;
 class nsMenuPopupFrame;
 class nsMenuBarFrame;
+class nsMenuParent;
 class nsIDocShellTreeItem;
 class nsPIDOMWindowOuter;
 class nsRefreshDriver;
@@ -66,7 +68,6 @@ namespace dom {
 class Event;
 class KeyboardEvent;
 class UIEvent;
-class XULButtonElement;
 }  // namespace dom
 }  // namespace mozilla
 
@@ -306,7 +307,8 @@ class nsXULPopupPositionedEvent : public mozilla::Runnable {
  public:
   explicit nsXULPopupPositionedEvent(nsIContent* aPopup)
       : mozilla::Runnable("nsXULPopupPositionedEvent"), mPopup(aPopup) {
-    MOZ_ASSERT(aPopup);
+    NS_ASSERTION(
+        aPopup, "null popup supplied to nsXULPopupPositionedEvent constructor");
   }
 
   NS_IMETHOD Run() override;
@@ -316,7 +318,7 @@ class nsXULPopupPositionedEvent : public mozilla::Runnable {
   static bool DispatchIfNeeded(nsIContent* aPopup);
 
  private:
-  const nsCOMPtr<nsIContent> mPopup;
+  nsCOMPtr<nsIContent> mPopup;
 };
 
 // this class is used for dispatching menu command events asynchronously.
@@ -387,7 +389,7 @@ class nsXULPopupManager final : public nsIDOMEventListener,
   void OnNativeSubMenuWillOpen(mozilla::dom::Element* aPopupElement) override;
   void OnNativeSubMenuDidOpen(mozilla::dom::Element* aPopupElement) override;
   void OnNativeSubMenuClosed(mozilla::dom::Element* aPopupElement) override;
-  MOZ_CAN_RUN_SCRIPT_BOUNDARY void OnNativeMenuWillActivateItem(
+  void OnNativeMenuWillActivateItem(
       mozilla::dom::Element* aMenuItemElement) override;
 
   static nsXULPopupManager* sInstance;
@@ -400,11 +402,52 @@ class nsXULPopupManager final : public nsIDOMEventListener,
   // if a popup manager could not be allocated
   static nsXULPopupManager* GetInstance();
 
+  // Returns the immediate parent frame of inserted children of aFrame's
+  // content.
+  //
+  // FIXME(emilio): Or something like that, because this is kind of broken in a
+  // variety of situations like multiple insertion points.
+  static nsContainerFrame* ImmediateParentFrame(nsContainerFrame* aFrame);
+
   // This should be called when a window is moved or resized to adjust the
   // popups accordingly.
   void AdjustPopupsOnWindowChange(nsPIDOMWindowOuter* aWindow);
   void AdjustPopupsOnWindowChange(mozilla::PresShell* aPresShell);
 
+  // given a menu frame, find the prevous or next menu frame. If aPopup is
+  // true then navigate a menupopup, from one item on the menu to the previous
+  // or next one. This is used for cursor navigation between items in a popup
+  // menu. If aIsPopup is false, the navigation is on a menubar, so navigate
+  // between menus on the menubar. This is used for left/right cursor
+  // navigation.
+  //
+  // Items that are not valid, such as non-menu or non-menuitem elements are
+  // skipped, and the next or previous item after that is checked.
+  //
+  // If aStart is null, the first valid item is retrieved by GetNextMenuItem
+  // and the last valid item is retrieved by GetPreviousMenuItem.
+  //
+  // Both methods will loop around the beginning or end if needed.
+  //
+  // aParent - the parent menubar or menupopup
+  // aStart - the menu/menuitem to start navigation from. GetPreviousMenuItem
+  //          returns the item before it, while GetNextMenuItem returns the
+  //          item after it.
+  // aIsPopup - true for menupopups, false for menubars
+  // aWrap - true to wrap around to the beginning and continue searching if not
+  //         found. False to end at the beginning or end of the menu.
+  static nsMenuFrame* GetPreviousMenuItem(nsContainerFrame* aParent,
+                                          nsMenuFrame* aStart, bool aIsPopup,
+                                          bool aWrap);
+  static nsMenuFrame* GetNextMenuItem(nsContainerFrame* aParent,
+                                      nsMenuFrame* aStart, bool aIsPopup,
+                                      bool aWrap);
+
+  // returns true if the menu item aContent is a valid menuitem which may
+  // be navigated to. aIsPopup should be true for items on a popup, or false
+  // for items on a menubar.
+  static bool IsValidMenuItem(nsIContent* aContent, bool aOnPopup);
+
   // inform the popup manager that a menu bar has been activated or deactivated,
   // either because one of its menus has opened or closed, or that the menubar
   // has been focused such that its menus may be navigated with the keyboard.
@@ -416,12 +459,12 @@ class nsXULPopupManager final : public nsIDOMEventListener,
 
   struct MayShowMenuResult {
     const bool mIsNative = false;
-    mozilla::dom::XULButtonElement* const mMenuButton = nullptr;
+    nsMenuFrame* const mMenuFrame = nullptr;
     nsMenuPopupFrame* const mMenuPopupFrame = nullptr;
 
     explicit operator bool() const {
-      MOZ_ASSERT(!!mMenuButton == !!mMenuPopupFrame);
-      return mIsNative || mMenuButton;
+      MOZ_ASSERT(!!mMenuFrame == !!mMenuPopupFrame);
+      return mIsNative || mMenuFrame;
     }
   };
 
@@ -522,13 +565,12 @@ class nsXULPopupManager final : public nsIDOMEventListener,
    * items. This timer is stored in mCloseTimer. The timer may be cancelled and
    * the popup closed by calling KillMenuTimer.
    */
-  void HidePopupAfterDelay(nsMenuPopupFrame* aPopup, int32_t aDelay);
+  void HidePopupAfterDelay(nsMenuPopupFrame* aPopup);
 
   /**
    * Hide all of the popups from a given docshell. This should be called when
    * the document is hidden.
    */
-  MOZ_CAN_RUN_SCRIPT_BOUNDARY
   void HidePopupsInDocShell(nsIDocShellTreeItem* aDocShellToHide);
 
   /**
@@ -551,8 +593,7 @@ class nsXULPopupManager final : public nsIDOMEventListener,
    * aEvent - an nsXULMenuCommandEvent that contains all the info from the mouse
    *          event which triggered the menu to be executed, may not be null
    */
-  MOZ_CAN_RUN_SCRIPT void ExecuteMenu(nsIContent* aMenu,
-                                      nsXULMenuCommandEvent* aEvent);
+  void ExecuteMenu(nsIContent* aMenu, nsXULMenuCommandEvent* aEvent);
 
   /**
    * If a native menu is open, and aItem is an item in the menu's subtree,
@@ -567,6 +608,11 @@ class nsXULPopupManager final : public nsIDOMEventListener,
    */
   bool IsPopupOpen(nsIContent* aPopup);
 
+  /**
+   * Return true if the popup for the supplied menu parent is open.
+   */
+  bool IsPopupOpenForMenuParent(nsMenuParent* aMenuParent);
+
   /**
    * Return the frame for the topmost open popup of a given type, or null if
    * no popup of that type is open. If aType is ePopupTypeAny, a menu of any
@@ -574,11 +620,6 @@ class nsXULPopupManager final : public nsIDOMEventListener,
    */
   nsIFrame* GetTopPopup(nsPopupType aType);
 
-  /**
-   * Returns the topmost active menuitem that's currently visible, if any.
-   */
-  nsIContent* GetTopActiveMenuItemContent();
-
   /**
    * Return an array of all the open and visible popup frames for
    * menus, in order from top to bottom.
@@ -624,7 +665,7 @@ class nsXULPopupManager final : public nsIDOMEventListener,
    * item and later popups from the list. No point going through HidePopup as
    * the frames have gone away.
    */
-  MOZ_CAN_RUN_SCRIPT void PopupDestroyed(nsMenuPopupFrame* aFrame);
+  void PopupDestroyed(nsMenuPopupFrame* aFrame);
 
   /**
    * Returns true if there is a context menu open. If aPopup is specified,
@@ -656,29 +697,29 @@ class nsXULPopupManager final : public nsIDOMEventListener,
    * submenu before the timer fires, we should instead cancel the timer. This
    * ensures that the user can move the mouse diagonally over a menu.
    */
-  void CancelMenuTimer(nsMenuPopupFrame*);
+  void CancelMenuTimer(nsMenuParent* aMenuParent);
 
   /**
    * Handles navigation for menu accelkeys. If aFrame is specified, then the
    * key is handled by that popup, otherwise if aFrame is null, the key is
    * handled by the active popup or menubar.
    */
-  MOZ_CAN_RUN_SCRIPT bool HandleShortcutNavigation(
-      mozilla::dom::KeyboardEvent& aKeyEvent, nsMenuPopupFrame* aFrame);
+  bool HandleShortcutNavigation(mozilla::dom::KeyboardEvent* aKeyEvent,
+                                nsMenuPopupFrame* aFrame);
 
   /**
    * Handles cursor navigation within a menu. Returns true if the key has
    * been handled.
    */
-  MOZ_CAN_RUN_SCRIPT bool HandleKeyboardNavigation(uint32_t aKeyCode);
+  bool HandleKeyboardNavigation(uint32_t aKeyCode);
 
   /**
    * Handle keyboard navigation within a menu popup specified by aFrame.
    * Returns true if the key was handled and other default handling
    * should not occur.
    */
-  MOZ_CAN_RUN_SCRIPT bool HandleKeyboardNavigationInPopup(
-      nsMenuPopupFrame* aFrame, nsNavigationDirection aDir) {
+  bool HandleKeyboardNavigationInPopup(nsMenuPopupFrame* aFrame,
+                                       nsNavigationDirection aDir) {
     return HandleKeyboardNavigationInPopup(nullptr, aFrame, aDir);
   }
 
@@ -686,9 +727,8 @@ class nsXULPopupManager final : public nsIDOMEventListener,
    * Handles the keyboard event with keyCode value. Returns true if the event
    * has been handled.
    */
-  MOZ_CAN_RUN_SCRIPT bool HandleKeyboardEventWithKeyCode(
-      mozilla::dom::KeyboardEvent* aKeyEvent,
-      nsMenuChainItem* aTopVisibleMenuItem);
+  bool HandleKeyboardEventWithKeyCode(mozilla::dom::KeyboardEvent* aKeyEvent,
+                                      nsMenuChainItem* aTopVisibleMenuItem);
 
   // Sets mIgnoreKeys of the Top Visible Menu Item
   nsresult UpdateIgnoreKeys(bool aIgnoreKeys);
@@ -699,9 +739,9 @@ class nsXULPopupManager final : public nsIDOMEventListener,
     return mPendingPopup->mEvent.get();
   }
 
-  MOZ_CAN_RUN_SCRIPT nsresult KeyUp(mozilla::dom::KeyboardEvent* aKeyEvent);
-  MOZ_CAN_RUN_SCRIPT nsresult KeyDown(mozilla::dom::KeyboardEvent* aKeyEvent);
-  MOZ_CAN_RUN_SCRIPT nsresult KeyPress(mozilla::dom::KeyboardEvent* aKeyEvent);
+  nsresult KeyUp(mozilla::dom::KeyboardEvent* aKeyEvent);
+  nsresult KeyDown(mozilla::dom::KeyboardEvent* aKeyEvent);
+  nsresult KeyPress(mozilla::dom::KeyboardEvent* aKeyEvent);
 
  protected:
   nsXULPopupManager();
@@ -720,19 +760,16 @@ class nsXULPopupManager final : public nsIDOMEventListener,
 
   // Hide all of the visible popups from the given list. This function can
   // cause style changes and frame destruction.
-  MOZ_CAN_RUN_SCRIPT void HidePopupsInList(
-      const nsTArray<nsMenuPopupFrame*>& aFrames);
+  void HidePopupsInList(const nsTArray<nsMenuPopupFrame*>& aFrames);
 
   // Hide, but don't close, visible menus. Called before executing a menu item.
   // The caller promises to close the menus properly (with a call to HidePopup)
   // once the item has been executed.
-  MOZ_CAN_RUN_SCRIPT void HideOpenMenusBeforeExecutingMenu(CloseMenuMode aMode);
+  void HideOpenMenusBeforeExecutingMenu(CloseMenuMode aMode);
 
   // callbacks for ShowPopup and HidePopup as events may be done asynchronously
-  MOZ_CAN_RUN_SCRIPT void ShowPopupCallback(nsIContent* aPopup,
-                                            nsMenuPopupFrame* aPopupFrame,
-                                            bool aIsContextMenu,
-                                            bool aSelectFirstItem);
+  void ShowPopupCallback(nsIContent* aPopup, nsMenuPopupFrame* aPopupFrame,
+                         bool aIsContextMenu, bool aSelectFirstItem);
   MOZ_CAN_RUN_SCRIPT void HidePopupCallback(
       nsIContent* aPopup, nsMenuPopupFrame* aPopupFrame, nsIContent* aNextPopup,
       nsIContent* aLastPopup, nsPopupType aPopupType, bool aDeselectMenu);
@@ -779,7 +816,6 @@ class nsXULPopupManager final : public nsIDOMEventListener,
   /**
    * Handle keyboard navigation within a menu popup specified by aItem.
    */
-  MOZ_CAN_RUN_SCRIPT
   bool HandleKeyboardNavigationInPopup(nsMenuChainItem* aItem,
                                        nsNavigationDirection aDir) {
     return HandleKeyboardNavigationInPopup(aItem, aItem->Frame(), aDir);
@@ -793,9 +829,9 @@ class nsXULPopupManager final : public nsIDOMEventListener,
    * an open submenu if one exists. Returns true if the key was
    * handled and other default handling should not occur.
    */
-  MOZ_CAN_RUN_SCRIPT bool HandleKeyboardNavigationInPopup(
-      nsMenuChainItem* aItem, nsMenuPopupFrame* aFrame,
-      nsNavigationDirection aDir);
+  bool HandleKeyboardNavigationInPopup(nsMenuChainItem* aItem,
+                                       nsMenuPopupFrame* aFrame,
+                                       nsNavigationDirection aDir);
 
  protected:
   already_AddRefed<nsINode> GetLastTriggerNode(
@@ -853,7 +889,9 @@ class nsXULPopupManager final : public nsIDOMEventListener,
 
   // timer used for HidePopupAfterDelay
   nsCOMPtr<nsITimer> mCloseTimer;
-  nsMenuPopupFrame* mTimerMenu = nullptr;
+
+  // a popup that is waiting on the timer
+  nsMenuPopupFrame* mTimerMenu;
 
   // Information about the popup that is currently firing a popupshowing event.
   const PendingPopup* mPendingPopup;

layout/xul/nsXULTooltipListener.cpp

@@ -6,7 +6,6 @@
 
 #include "nsXULTooltipListener.h"
 
-#include "XULButtonElement.h"
 #include "nsXULElement.h"
 #include "mozilla/dom/Document.h"
 #include "nsGkAtoms.h"
@@ -608,12 +607,12 @@ nsresult nsXULTooltipListener::GetTooltipFor(nsIContent* aTarget,
   }
 
   // Submenus can't be used as tooltips, see bug 288763.
-  if (nsIContent* parent = tooltip->GetParent()) {
-    if (auto* button = XULButtonElement::FromNode(parent)) {
-      if (button->IsMenu()) {
-        NS_WARNING("Menu cannot be used as a tooltip");
-        return NS_ERROR_FAILURE;
-      }
+  nsIContent* parent = tooltip->GetParent();
+  if (parent) {
+    nsMenuFrame* menu = do_QueryFrame(parent->GetPrimaryFrame());
+    if (menu) {
+      NS_WARNING("Menu cannot be used as a tooltip");
+      return NS_ERROR_FAILURE;
     }
   }
 

layout/xul/test/test_bug1197913.xhtml

@@ -41,7 +41,12 @@ https://bugzilla.mozilla.org/show_bug.cgi?id=1197913
     synthesizeMouse(menuitem, -1, 0, { type: "mousemove" });
 
     setTimeout(() => {
-      ok(menuitem.getAttribute("_moz-menuactive"), "Should be active");
+      if (navigator.platform.toLowerCase().startsWith("win")) {
+        ok(menuitem.getAttribute("_moz-menuactive"));
+      } else {
+        ok(!menuitem.getAttribute("_moz-menuactive"));
+      }
+
       SimpleTest.finish();
     });
   }

toolkit/components/printing/tests/browser_print_page_range.js

@@ -4,7 +4,6 @@
 "use strict";
 
 async function changeRangeTo(helper, destination) {
-  info(`changeRangeTo(${destination})`);
   let rangeSelect = helper.get("range-picker");
   let options = getRangeOptions(helper);
   let numberMove =

toolkit/content/tests/chrome/popup_trigger.js

@@ -604,6 +604,7 @@ var popupTests = [
       "command item3",
       "popuphiding thepopup",
       "popuphidden thepopup",
+      "DOMMenuItemInactive item3",
     ],
     test(testname, step) {
       var item3 = document.getElementById("item3");
@@ -657,6 +658,7 @@ var popupTests = [
       "command amenu",
       "popuphiding thepopup",
       "popuphidden thepopup",
+      "DOMMenuItemInactive amenu",
     ],
     test() {
       sendString("M");
@@ -754,6 +756,7 @@ var popupTests = [
       "popuphidden submenupopup",
       "DOMMenuItemInactive submenuitem",
       "DOMMenuInactive submenupopup",
+      "DOMMenuItemActive submenu",
     ],
     test() {
       synthesizeKey("KEY_ArrowLeft");
@@ -795,6 +798,7 @@ var popupTests = [
       "popuphidden submenupopup",
       "DOMMenuItemInactive submenuitem",
       "DOMMenuInactive submenupopup",
+      "DOMMenuItemActive submenu",
     ],
     test() {
       synthesizeKey("KEY_Escape");
@@ -858,6 +862,7 @@ var popupTests = [
       "command amenu",
       "popuphiding thepopup",
       "popuphidden thepopup",
+      "DOMMenuItemInactive amenu",
     ],
     test() {
       sendString("M");
@@ -922,6 +927,7 @@ var popupTests = [
       "popuphidden thepopup",
       "DOMMenuInactive submenupopup",
       "DOMMenuItemInactive submenu",
+      "DOMMenuItemInactive submenu",
       "DOMMenuInactive thepopup",
     ],
     test() {
@@ -1052,6 +1058,7 @@ var popupTests = [
       "command item1",
       "popuphiding thepopup",
       "popuphidden thepopup",
+      "DOMMenuItemInactive item1",
     ],
     test(testname, step) {
       synthesizeKey("KEY_ArrowDown");
@@ -1134,6 +1141,7 @@ var popupTests = [
     },
   },
   {
+    // openPopup using object as position argument with event
     testname: "openPopup with object argument with event",
     events: ["popupshowing thepopup 1000", "popupshown thepopup"],
     autohide: "thepopup",
@@ -1148,6 +1156,7 @@ var popupTests = [
     },
   },
   {
+    // openPopup with no arguments
     testname: "openPopup with no arguments",
     events: ["popupshowing thepopup", "popupshown thepopup"],
     autohide: "thepopup",

toolkit/content/tests/chrome/test_bug562554.xhtml

@@ -11,8 +11,8 @@
 
   <toolbarbutton type="menu" id="toolbarmenu" height="200" style="-moz-box-pack: start; -moz-box-align: start">
     <menupopup id="menupopup" onpopupshowing="eventReceived('popupshowing'); return false;"/>
-    <stack style="pointer-events: none">
-      <button style="pointer-events: auto; width: 100px; height: 30px; margin-left: 0; margin-top: 0;" allowevents="true"
+    <stack>
+      <button style="width: 100px; height: 30px; margin-left: 0; margin-top: 0;" allowevents="true"
               onclick="eventReceived('clickbutton1'); return false;"/>
       <button style="width: 100px; height: 30px; margin-left: 70px; margin-top: 0;"
               onclick="eventReceived('clickbutton2'); return false;"/>
@@ -43,6 +43,10 @@ let tests = [
   () => synthesizeMouse($("toolbarmenu"), 10, 15, {}, window),
   () => is(eventCount.clickbutton1, 1, "Button 1 clicked"),
 
+  // Click on button2 where it intersects with button1 - should call popupshowing
+  () => synthesizeMouse($("toolbarmenu"), 85, 15, {}, window),
+  () => is(eventCount.popupshowing, 2, "Got second popupshowing event"),
+
   // Click on button2 outside of intersection - should call popupshowing
   () => synthesizeMouse($("toolbarmenu"), 150, 15, {}, window)
 ];
@@ -61,7 +65,7 @@ function nextTest() {
 function finishTest() {
   is(eventCount.clickbutton1, 1, "Correct number of clicks on button 1");
   is(eventCount.clickbutton2, 0, "Correct number of clicks on button 2");
-  is(eventCount.popupshowing, 2, "Correct number of popupshowing events received");
+  is(eventCount.popupshowing, 3, "Correct number of popupshowing events received");
 
   SimpleTest.finish();
 }

toolkit/content/tests/chrome/test_button.xhtml

@@ -6,8 +6,8 @@
   -->
 <window title="Button Test"
         xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul">
-  <script src="chrome://mochikit/content/tests/SimpleTest/SimpleTest.js"></script>
-  <script src="chrome://mochikit/content/tests/SimpleTest/EventUtils.js"></script>
+  <script src="chrome://mochikit/content/tests/SimpleTest/SimpleTest.js"></script>      
+  <script src="chrome://mochikit/content/tests/SimpleTest/EventUtils.js"></script>      
 
 <button id="one" label="One" />
 <button id="two" label="Two"/>

toolkit/content/tests/chrome/test_edit_contextmenu.html

@@ -66,13 +66,13 @@ https://bugzilla.mozilla.org/show_bug.cgi?id=1513343
       ok(contextmenu.querySelector("[command=cmd_delete]").hasAttribute("disabled"), "delete disabled");
       ok(!contextmenu.querySelector("[command=cmd_selectAll]").hasAttribute("disabled"), "select all enabled");
 
-      let popuphidden = new Promise(r => win.addEventListener("popuphidden", r, { once: true }));
-
-      contextmenu.activateItem(contextmenu.querySelector("[command=cmd_undo]"));
-
-      await popuphidden;
-
+      contextmenu.querySelector("[command=cmd_undo]").click();
       is(element.value, "", "undo worked");
+
+      // Close the context menu to avoid affecting next test
+      let popuphidden = new Promise(r => win.addEventListener("popuphidden", r, { once: true }));
+      contextmenu.hidePopup();
+      await popuphidden;
       contextmenu.remove();
     }
   </script>

toolkit/content/tests/chrome/test_menu_hide.xhtml

@@ -61,7 +61,7 @@ function submenuOpened()
 
 function done()
 {
-  ok($("submenu").hasAttribute('_moz-menuactive'), "menu still highlighted");
+  ok(!$("submenu").hasAttribute('_moz-menuactive'), "menu unhighlighted");
   SimpleTest.finish();
 }
 

toolkit/content/tests/chrome/test_menulist.xhtml

@@ -6,9 +6,9 @@
         onload="setTimeout(testtag_menulists, 0);"
         xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul">
 
-  <script src="chrome://mochikit/content/tests/SimpleTest/SimpleTest.js"></script>
-  <script src="chrome://mochikit/content/tests/SimpleTest/EventUtils.js"></script>
-  <script type="application/javascript" src="xul_selectcontrol.js"></script>
+  <script src="chrome://mochikit/content/tests/SimpleTest/SimpleTest.js"></script>      
+  <script src="chrome://mochikit/content/tests/SimpleTest/EventUtils.js"></script>      
+  <script type="application/javascript" src="xul_selectcontrol.js"></script>      
 
 <vbox id="scroller" style="overflow: auto" height="60">
   <menulist id="menulist" onpopupshown="test_menulist_open(this, this.parentNode)"

toolkit/content/tests/chrome/test_menulist_keynav.xhtml

@@ -65,7 +65,6 @@ function runTests()
     keyCheck(list, "KEY_ArrowUp", 1, 1, "cursor up");
 
     // On Windows, wrapping doesn't occur.
-    expectCommandEvent = !iswin;
     keyCheck(list, "KEY_ArrowUp", iswin ? 1 : 4, 1, "cursor up wrap");
 
     list.selectedIndex = 4;
@@ -89,7 +88,8 @@ function runTests()
 
 function pressLetter()
 {
-  // A command event should be fired only if the menulist is closed.
+  // A command event should be fired only if the menulist is closed, or on Windows,
+  // where items are selected immediately.
   expectCommandEvent = !gOpenPhase || iswin;
 
   sendString("G");
@@ -97,6 +97,12 @@ function pressLetter()
 
   keyCheck(list, "T", 2, 1, "letter pressed");
 
+  if (!gOpenPhase) {
+    SpecialPowers.setIntPref("ui.menu.incremental_search.timeout", 0); // prevent to timeout
+    keyCheck(list, "T", 2, 1, "same letter pressed");
+    SpecialPowers.clearUserPref("ui.menu.incremental_search.timeout");
+  }
+
   setTimeout(pressedAgain, 1200);
 }
 
@@ -197,7 +203,6 @@ function tabAndScroll()
 
 function keyCheck(list, key, index, defaultindex, testname)
 {
-  info(`keyCheck(${index}, ${key}, ${index}, ${defaultindex}, ${testname}, ${expectCommandEvent})`);
   var item = $("i" + index);
   var defaultitem = $("i" + defaultindex || 1);
 
@@ -262,8 +267,8 @@ function checkCursorNavigation()
   is(commandEventsCount, iswin ? 1 : 0, "selectedIndex after cursor down command event");
   is(list.menupopup.state, "open", "cursor down popup state");
   synthesizeKey("KEY_PageDown");
-  is(list.selectedIndex, iswin ? 2 : 1, "selectedIndex after page down");
-  is(commandEventsCount, iswin ? 1 : 0, "selectedIndex after page down command event");
+  is(list.selectedIndex, iswin ? 3 : 1, "selectedIndex after page down");
+  is(commandEventsCount, iswin ? 2 : 0, "selectedIndex after page down command event");
   is(list.menupopup.state, "open", "page down popup state");
 
   // Check whether cursor up and down wraps.

toolkit/content/tests/chrome/test_menulist_paging.xhtml

@@ -88,8 +88,8 @@ let tests = [
                                    ups: [6, 3, 0, 0] },
   { list: "menulist2", initial: 1, scroll: 0, downs: [4, 7, 8, 8],
                                    ups: [5, 2, 1] },
-  { list: "menulist3", initial: 1, scroll: -1, downs: [3, 6, 8, 8],
-                                   ups: [6, 3, 1] },
+  { list: "menulist3", initial: 1, scroll: -1, downs: [6, 8, 8],
+                                   ups: [3, 1, 1] },
   { list: "menulist4", initial: 5, scroll: 2, downs: [], ups: [] }
 ];
 

toolkit/content/tests/widgets/popup_shared.js

@@ -32,7 +32,6 @@ var gPopupTests = null;
 var gTestIndex = -1;
 var gTestStepIndex = 0;
 var gTestEventIndex = 0;
-var gActualEvents = [];
 var gAutoHide = false;
 var gExpectedEventDetails = null;
 var gExpectedTriggerNode = null;
@@ -79,14 +78,6 @@ function ok(condition, message) {
   }
 }
 
-function info(message) {
-  if (window.opener) {
-    window.opener.SimpleTest.info(message);
-  } else {
-    SimpleTest.info(message);
-  }
-}
-
 function is(left, right, message) {
   if (window.opener) {
     window.opener.SimpleTest.is(left, right, message);
@@ -132,8 +123,6 @@ function eventOccurred(event) {
       return;
     }
 
-    gActualEvents.push(`${event.type} ${event.target.id}`);
-
     var eventitem = events[gTestEventIndex].split(" ");
     var matches;
     if (eventitem[1] == "#tooltip") {
@@ -216,8 +205,6 @@ function eventOccurred(event) {
       if (events.length <= gTestEventIndex) {
         setTimeout(checkResult, 0);
       }
-    } else {
-      info(`Actual events so far: ${JSON.stringify(gActualEvents)}`);
     }
   }
 }
@@ -243,9 +230,7 @@ async function checkResult() {
 }
 
 function goNextStep() {
-  info(`events: ${JSON.stringify(gActualEvents)}`);
   gTestEventIndex = 0;
-  gActualEvents = [];
 
   var step = null;
   var test = gPopupTests[gTestIndex];
@@ -285,7 +270,6 @@ function goNextStepSync() {
     var test = gPopupTests[gTestIndex];
     // Set the location hash so it's easy to see which test is running
     document.location.hash = test.testname;
-    info("Starting " + test.testname);
 
     // skip the test if the condition returns false
     if ("condition" in test && !test.condition()) {

toolkit/content/tests/widgets/test_contextmenu_nested.xhtml

@@ -92,7 +92,7 @@ var popupTests = [
   events: [ "popuphiding innercontext", "popuphidden innercontext",
             "popuphiding outercontext", "popuphidden outercontext",
             "DOMMenuInactive innercontext",
-            "DOMMenuItemInactive outercontextmenu",
+            "DOMMenuItemInactive outercontextmenu", "DOMMenuItemInactive outercontextmenu",
             "DOMMenuInactive outercontext" ],
   test: () => $("outercontext").hidePopup(),
   result (testname) {
@@ -107,7 +107,7 @@ var popupTests = [
   events: [ "popuphiding innermain", "popuphidden innermain",
             "popuphiding outermain", "popuphidden outermain",
             "DOMMenuInactive innermain",
-            "DOMMenuItemInactive outermenu",
+            "DOMMenuItemInactive outermenu", "DOMMenuItemInactive outermenu",
             "DOMMenuInactive outermain" ],
 
   test: () => $("outermain").hidePopup(),

toolkit/content/tests/widgets/window_menubar.xhtml

@@ -14,7 +14,7 @@
   Need to investigate these tests a bit more. Some of the accessibility events
   are firing multiple times or in different orders in different circumstances.
   Note that this was also the case before bug 279703.
--->
+  -->
 
 <hbox style="margin-left: 275px; margin-top: 275px;">
 <menubar id="menubar">
@@ -89,39 +89,35 @@ window.opener.SimpleTest.waitForFocus(function () {
   startPopupTests(popupTests);
 }, window);
 
-const kIsWindows = navigator.platform.indexOf("Win") == 0;
-const kIsLinux = navigator.platform.includes("Linux");
-
 // On Linux, the first menu opens when F10 is pressed, but on other platforms
 // the menubar is focused but no menu is opened. This means that different events
 // fire.
 function pressF10Events()
 {
-  return kIsLinux ?
+  return navigator.platform.includes("Linux") ?
     [ "DOMMenuBarActive menubar", "DOMMenuItemActive filemenu", "popupshowing filepopup", "popupshown filepopup"] :
     [ "DOMMenuBarActive menubar", "DOMMenuItemActive filemenu" ];
 }
 
-function closeAfterF10Events()
+function closeAfterF10Events(extraInactive)
 {
-  if (kIsLinux) {
-    return [
-      "popuphiding filepopup",
-      "popuphidden filepopup",
-      "DOMMenuInactive filepopup",
-      "DOMMenuItemInactive filemenu",
-      "DOMMenuBarInactive menubar",
-    ];
+  if (navigator.platform.includes("Linux")) {
+    var events = [ "popuphiding filepopup", "popuphidden filepopup",
+                   "DOMMenuInactive filepopup", "DOMMenuBarInactive menubar",
+                   "DOMMenuItemInactive filemenu" ];
+    if (extraInactive)
+      events.push("DOMMenuItemInactive filemenu");
+    return events;
   }
 
-  return [ "DOMMenuItemInactive filemenu", "DOMMenuBarInactive menubar" ];
+  return [ "DOMMenuBarInactive menubar", "DOMMenuItemInactive filemenu" ];
 }
 
 var popupTests = [
 {
   testname: "press on menu",
-  events: [ "DOMMenuBarActive menubar",
-            "DOMMenuItemActive filemenu", "popupshowing filepopup", "popupshown filepopup" ],
+  events: [ "popupshowing filepopup", "DOMMenuBarActive menubar",
+            "DOMMenuItemActive filemenu", "popupshown filepopup" ],
   test() { synthesizeMouse(document.getElementById("filemenu"), 8, 8, { }); },
   result (testname) {
     checkActive(gFilePopup, "", testname);
@@ -176,23 +172,24 @@ var popupTests = [
       "DOMMenuItemInactive filemenu", "DOMMenuItemActive editmenu",
       "popuphiding filepopup", "popuphidden filepopup",
       // the popupshowing event gets fired when showing the edit menu.
-      "popupshowing editpopup",
-      "DOMMenuItemInactive item1",
-      "DOMMenuInactive filepopup",
+      // The item from the file menu doesn't get deactivated until the
+      // next item needs to be selected
+      "popupshowing editpopup", "DOMMenuItemInactive item1",
+      // not sure why the menu inactivated event is firing so late
+      "DOMMenuInactive filepopup"
     ];
     // finally, the first item is activated and popupshown is fired.
     // On Windows, don't skip disabled items.
-    if (kIsWindows) {
+    if (navigator.platform.indexOf("Win") == 0)
       elist.push("DOMMenuItemActive cut");
-    } else {
+    else
       elist.push("DOMMenuItemActive copy");
-    }
     elist.push("popupshown editpopup");
     return elist;
   },
   test() { synthesizeKey("KEY_ArrowRight"); },
   result(testname) {
-    var expected = kIsWindows ? "cut" : "copy";
+    var expected = (navigator.platform.indexOf("Win") == 0) ? "cut" : "copy";
     checkActive(document.getElementById("editpopup"), expected, testname);
     checkClosed("filemenu", testname);
     checkOpen("editmenu", testname);
@@ -204,24 +201,16 @@ var popupTests = [
   // the menu but not fire a command event
   testname: "enter on disabled",
   events() {
-    if (kIsWindows) {
-      return [
-        "popuphiding editpopup",
-        "popuphidden editpopup",
-        "DOMMenuItemInactive cut",
-        "DOMMenuInactive editpopup",
-        "DOMMenuItemInactive editmenu",
-        "DOMMenuBarInactive menubar",
-      ];
-    }
-    return [
-      "DOMMenuItemInactive copy",
-      "DOMMenuInactive editpopup",
-      "DOMMenuItemInactive editmenu",
-      "DOMMenuBarInactive menubar",
-      "command copy",
-      "popuphiding editpopup", "popuphidden editpopup",
-    ];
+    if (navigator.platform.indexOf("Win") == 0)
+      return [ "popuphiding editpopup", "popuphidden editpopup",
+               "DOMMenuItemInactive cut", "DOMMenuInactive editpopup",
+               "DOMMenuBarInactive menubar",
+               "DOMMenuItemInactive editmenu", "DOMMenuItemInactive editmenu" ];
+      return [ "DOMMenuItemInactive copy", "DOMMenuInactive editpopup",
+               "DOMMenuBarInactive menubar",
+               "DOMMenuItemInactive editmenu", "DOMMenuItemInactive editmenu",
+               "command copy", "popuphiding editpopup", "popuphidden editpopup",
+               "DOMMenuItemInactive copy" ];
   },
   test() { synthesizeKey("KEY_Enter"); },
   result(testname) {
@@ -233,13 +222,9 @@ var popupTests = [
   // pressing Alt + a key should open the corresponding menu
   testname: "open with accelerator",
   events() {
-    return [
-      "DOMMenuBarActive menubar",
-      "DOMMenuItemActive viewmenu",
-      "popupshowing viewpopup",
-      "DOMMenuItemActive toolbar",
-      "popupshown viewpopup",
-    ];
+    return [ "DOMMenuBarActive menubar",
+             "popupshowing viewpopup", "DOMMenuItemActive viewmenu",
+             "DOMMenuItemActive toolbar", "popupshown viewpopup" ];
   },
   test() { synthesizeKey("V", { altKey: true }); },
   result(testname) {
@@ -251,12 +236,11 @@ var popupTests = [
   // open the submenu with the cursor right key
   testname: "open submenu with cursor right",
   events() {
-    // on Windows, the disabled 'navigation' item can still be highlighted
-    if (kIsWindows) {
-      return ["popupshowing toolbarpopup", "DOMMenuItemActive navigation",
+    // on Windows, the disabled 'navigation' item can stll be highlihted
+    if (navigator.platform.indexOf("Win") == 0)
+      return [ "popupshowing toolbarpopup", "DOMMenuItemActive navigation",
                "popupshown toolbarpopup" ];
-    }
-    return [ "popupshowing toolbarpopup", "popupshown toolbarpopup" ];
+      return [ "popupshowing toolbarpopup", "popupshown toolbarpopup" ];
   },
   test() { synthesizeKey("KEY_ArrowRight"); },
   result(testname) {
@@ -268,23 +252,15 @@ var popupTests = [
   // close the submenu with the cursor left key
   testname: "close submenu with cursor left",
   events() {
-    if (kIsWindows) {
-      return [
-        "popuphiding toolbarpopup",
-        "popuphidden toolbarpopup",
-        "DOMMenuItemInactive navigation",
-        "DOMMenuInactive toolbarpopup",
-      ];
-    }
-    return [
-      "popuphiding toolbarpopup",
-      "popuphidden toolbarpopup",
-      "DOMMenuInactive toolbarpopup",
-    ];
-  },
-  test() {
-    synthesizeKey("KEY_ArrowLeft");
+    if (navigator.platform.indexOf("Win") == 0)
+      return [ "popuphiding toolbarpopup", "popuphidden toolbarpopup",
+               "DOMMenuItemInactive navigation", "DOMMenuInactive toolbarpopup",
+               "DOMMenuItemActive toolbar" ];
+      return [ "popuphiding toolbarpopup", "popuphidden toolbarpopup",
+               "DOMMenuInactive toolbarpopup",
+               "DOMMenuItemActive toolbar" ];
   },
+  test() { synthesizeKey("KEY_ArrowLeft"); },
   result(testname) {
     checkOpen("viewmenu", testname);
     checkClosed("toolbar", testname);
@@ -294,12 +270,11 @@ var popupTests = [
   // open the submenu with the enter key
   testname: "open submenu with enter",
   events() {
-    if (kIsWindows) {
-      // on Windows, the disabled 'navigation' item can stll be highlighted
+    // on Windows, the disabled 'navigation' item can stll be highlighted
+    if (navigator.platform.indexOf("Win") == 0)
       return [ "popupshowing toolbarpopup", "DOMMenuItemActive navigation",
                "popupshown toolbarpopup" ];
-    }
-    return [ "popupshowing toolbarpopup", "popupshown toolbarpopup" ];
+      return [ "popupshowing toolbarpopup", "popupshown toolbarpopup" ];
   },
   test() { synthesizeKey("KEY_Enter"); },
   result(testname) {
@@ -308,21 +283,16 @@ var popupTests = [
   },
 },
 {
+  // close the submenu with the escape key
   testname: "close submenu with escape",
   events() {
-    if (kIsWindows) {
-      return [
-        "popuphiding toolbarpopup",
-        "popuphidden toolbarpopup",
-        "DOMMenuItemInactive navigation",
-        "DOMMenuInactive toolbarpopup",
-      ];
-    }
-    return [
-      "popuphiding toolbarpopup",
-      "popuphidden toolbarpopup",
-      "DOMMenuInactive toolbarpopup",
-    ];
+    if (navigator.platform.indexOf("Win") == 0)
+      return [ "popuphiding toolbarpopup", "popuphidden toolbarpopup",
+               "DOMMenuItemInactive navigation", "DOMMenuInactive toolbarpopup",
+               "DOMMenuItemActive toolbar" ];
+      return [ "popuphiding toolbarpopup", "popuphidden toolbarpopup",
+               "DOMMenuInactive toolbarpopup",
+               "DOMMenuItemActive toolbar" ];
   },
   test() { synthesizeKey("KEY_Escape"); },
   result(testname) {
@@ -331,18 +301,15 @@ var popupTests = [
   },
 },
 {
+  // open the submenu with the enter key again
   testname: "open submenu with enter again",
-  condition() { return kIsWindows; },
+  condition() { return (navigator.platform.indexOf("Win") == 0) },
   events() {
     // on Windows, the disabled 'navigation' item can stll be highlighted
-    if (kIsWindows) {
-      return [
-        "popupshowing toolbarpopup",
-        "DOMMenuItemActive navigation",
-        "popupshown toolbarpopup"
-      ];
-    }
-    return [ "popupshowing toolbarpopup", "popupshown toolbarpopup" ];
+    if (navigator.platform.indexOf("Win") == 0)
+      return [ "popupshowing toolbarpopup", "DOMMenuItemActive navigation",
+               "popupshown toolbarpopup" ];
+      return [ "popupshowing toolbarpopup", "popupshown toolbarpopup" ];
   },
   test() { synthesizeKey("KEY_Enter"); },
   result(testname) {
@@ -353,22 +320,14 @@ var popupTests = [
 {
   // while a submenu is open, switch to the next toplevel menu with the cursor right key
   testname: "while a submenu is open, switch to the next menu with the cursor right",
-  condition() { return kIsWindows; },
-  events: [
-    "DOMMenuItemInactive viewmenu",
-    "DOMMenuItemActive helpmenu",
-    "popuphiding toolbarpopup",
-    "popuphidden toolbarpopup",
-    "popuphiding viewpopup",
-    "popuphidden viewpopup",
-    "popupshowing helppopup",
-    "DOMMenuItemInactive navigation",
-    "DOMMenuInactive toolbarpopup",
-    "DOMMenuItemInactive toolbar",
-    "DOMMenuInactive viewpopup",
-    "DOMMenuItemActive contents",
-    "popupshown helppopup"
-  ],
+  condition() { return (navigator.platform.indexOf("Win") == 0) },
+  events: [ "DOMMenuItemInactive viewmenu", "DOMMenuItemActive helpmenu",
+            "popuphiding toolbarpopup", "popuphidden toolbarpopup",
+            "popuphiding viewpopup", "popuphidden viewpopup",
+            "popupshowing helppopup", "DOMMenuItemInactive navigation",
+            "DOMMenuInactive toolbarpopup", "DOMMenuItemInactive toolbar",
+            "DOMMenuInactive viewpopup", "DOMMenuItemActive contents",
+            "popupshown helppopup" ],
   test() { synthesizeKey("KEY_ArrowRight"); },
   result(testname) {
     checkOpen("helpmenu", testname);
@@ -379,47 +338,23 @@ var popupTests = [
 {
   // close the main menu with the escape key
   testname: "close menubar menu with escape",
-  condition() { return kIsWindows; },
-  events: [
-    "popuphiding helppopup",
-    "popuphidden helppopup",
-    "DOMMenuItemInactive contents",
-    "DOMMenuInactive helppopup",
-  ],
+  condition() { return (navigator.platform.indexOf("Win") == 0) },
+  events: [ "popuphiding helppopup", "popuphidden helppopup",
+            "DOMMenuItemInactive contents", "DOMMenuInactive helppopup",
+            "DOMMenuBarInactive menubar", "DOMMenuItemInactive helpmenu" ],
   test() { synthesizeKey("KEY_Escape"); },
-  result(testname) {
-    checkClosed("helpmenu", testname);
-  },
+  result(testname) { checkClosed("viewmenu", testname); },
 },
 {
   // close the main menu with the escape key
   testname: "close menubar menu with escape",
-  condition() { return !kIsWindows; },
-  events: [
-    "popuphiding viewpopup",
-    "popuphidden viewpopup",
-    "DOMMenuItemInactive toolbar",
-    "DOMMenuInactive viewpopup",
-  ],
-  test() {
-    synthesizeKey("KEY_Escape");
-  },
-  result(testname) {
-    checkClosed("viewmenu", testname);
-  },
-},
-{
-  // Deactivate menubar with the escape key.
-  testname: "deactivate menubar menu with escape",
-  events: [
-    "DOMMenuItemInactive " + (kIsWindows ? "helpmenu" : "viewmenu"),
-    "DOMMenuBarInactive menubar",
-  ],
-  test() {
-    synthesizeKey("KEY_Escape");
-  },
-  result(testname) {
-  },
+  condition() { return (navigator.platform.indexOf("Win") != 0) },
+  events: [ "popuphiding viewpopup", "popuphidden viewpopup",
+            "DOMMenuItemInactive toolbar", "DOMMenuInactive viewpopup",
+            "DOMMenuBarInactive menubar",
+            "DOMMenuItemInactive viewmenu" ],
+  test() { synthesizeKey("KEY_Escape"); },
+  result(testname) { checkClosed("viewmenu", testname); },
 },
 {
   // Pressing Alt should highlight the first menu but not open it,
@@ -478,13 +413,9 @@ var popupTests = [
 {
   // pressing a character should act as an accelerator and open the menu
   testname: "accelerator on active menubar",
-  events: [
-    "DOMMenuItemInactive filemenu",
-    "DOMMenuItemActive helpmenu",
-    "popupshowing helppopup",
-    "DOMMenuItemActive contents",
-    "popupshown helppopup",
-  ],
+  events: [ "popupshowing helppopup",
+            "DOMMenuItemInactive filemenu", "DOMMenuItemActive helpmenu",
+            "DOMMenuItemActive contents", "popupshown helppopup" ],
   test() { sendChar("h"); },
   result(testname) {
     checkOpen("helpmenu", testname);
@@ -508,17 +439,13 @@ var popupTests = [
 {
   // check that pressing a menuitem's accelerator selects it
   testname: "menuitem accelerator",
-  events: [
-    "DOMMenuItemInactive contents",
-    "DOMMenuItemActive amenu",
-    "DOMMenuItemInactive amenu",
-    "DOMMenuInactive helppopup",
-    "DOMMenuItemInactive helpmenu",
-    "DOMMenuBarInactive menubar",
-    "command amenu",
-    "popuphiding helppopup",
-    "popuphidden helppopup",
-  ],
+  events: [ "DOMMenuItemInactive contents", "DOMMenuItemActive amenu",
+            "DOMMenuItemInactive amenu", "DOMMenuInactive helppopup",
+            "DOMMenuBarInactive menubar", "DOMMenuItemInactive helpmenu",
+            "DOMMenuItemInactive helpmenu",
+            "command amenu", "popuphiding helppopup", "popuphidden helppopup",
+            "DOMMenuItemInactive amenu",
+           ],
   test() { sendChar("m"); },
   result(testname) { checkClosed("helpmenu", testname); }
 },
@@ -529,25 +456,23 @@ var popupTests = [
   test() { synthesizeKey("KEY_F10"); },
   result(testname) {
     is(document.getElementById("filemenu").openedWithKey, true, testname + " openedWithKey");
-    if (kIsLinux) {
+    if (navigator.platform.includes("Linux"))
       checkOpen("filemenu", testname);
-    } else {
+    else
       checkClosed("filemenu", testname);
-    }
   },
 },
 {
   // pressing cursor left then down should open a menu
   testname: "cursor down on menu",
-  events: kIsLinux ?
+  events: (navigator.platform.includes("Linux")) ?
             [  "DOMMenuItemInactive filemenu", "DOMMenuItemActive helpmenu",
                // This is in a different order than the
                // "accelerator on active menubar" because menus opened from a
                // shortcut key are fired asynchronously
                "popuphiding filepopup", "popuphidden filepopup",
                "popupshowing helppopup",
-               "DOMMenuItemActive item1",
-               "DOMMenuItemInactive item1",
+               "DOMMenuItemActive item1", "DOMMenuItemInactive item1",
                "DOMMenuInactive filepopup",
                "popupshown helppopup" ] :
             [ "popupshowing helppopup", "DOMMenuItemInactive filemenu",
@@ -566,7 +491,7 @@ var popupTests = [
   // should not close because there is more than one item corresponding to
   // that letter
   testname: "menuitem with no accelerator",
-  events: kIsLinux ?
+  events: (navigator.platform.includes("Linux")) ?
            [ "DOMMenuItemActive one" ] :
            [ "DOMMenuItemInactive contents", "DOMMenuItemActive one" ],
   test() { sendChar("o"); },
@@ -588,7 +513,7 @@ var popupTests = [
   // pressing the letter again when the next item is disabled should still
   // select the disabled item
   testname: "menuitem with no accelerator disabled",
-  condition() { return kIsWindows; },
+  condition() { return (navigator.platform.indexOf("Win") == 0) },
   events: [ "DOMMenuItemInactive only", "DOMMenuItemActive other" ],
   test() { sendChar("o"); },
   result(testname) { }
@@ -598,20 +523,16 @@ var popupTests = [
   // selected and the menu closed
   testname: "menuitem with no accelerator single",
   events() {
-    let elist = [
-      "DOMMenuItemInactive other",
-      "DOMMenuItemActive third",
-      "DOMMenuItemInactive third",
-      "DOMMenuInactive helppopup",
-      "DOMMenuItemInactive helpmenu",
-      "DOMMenuBarInactive menubar",
-      "command third",
-      "popuphiding helppopup",
-      "popuphidden helppopup"
-    ];
-    if (!kIsWindows) {
+    var elist = [ "DOMMenuItemInactive other", "DOMMenuItemActive third",
+                  "DOMMenuItemInactive third", "DOMMenuInactive helppopup",
+                  "DOMMenuBarInactive menubar",
+                  "DOMMenuItemInactive helpmenu",
+                  "DOMMenuItemInactive helpmenu",
+                  "command third", "popuphiding helppopup",
+                  "popuphidden helppopup", "DOMMenuItemInactive third",
+                ];
+    if (!navigator.platform.includes("Win"))
       elist[0] = "DOMMenuItemInactive only";
-    }
     return elist;
   },
   test() { sendChar("t"); },
@@ -620,7 +541,7 @@ var popupTests = [
 {
   // pressing F10 should highlight the first menu but not open it
   testname: "F10 to activate menubar again",
-  condition() { return kIsWindows; },
+  condition() { return (navigator.platform.indexOf("Win") == 0) },
   events: [ "DOMMenuBarActive menubar", "DOMMenuItemActive filemenu" ],
   test() { synthesizeKey("KEY_F10"); },
   result(testname) { checkClosed("filemenu", testname); },
@@ -628,7 +549,7 @@ var popupTests = [
 {
   // pressing an accelerator for a disabled item should deactivate the menubar
   testname: "accelerator for disabled menu",
-  condition() { return kIsWindows; },
+  condition() { return (navigator.platform.indexOf("Win") == 0) },
   events: [ "DOMMenuItemInactive filemenu", "DOMMenuBarInactive menubar" ],
   test() { sendChar("s"); },
   result(testname) {
@@ -647,12 +568,8 @@ var popupTests = [
 },
 {
   testname: "press on second menu with shift",
-  events: [
-    "DOMMenuBarActive menubar",
-    "DOMMenuItemActive editmenu",
-    "popupshowing editpopup",
-    "popupshown editpopup",
-  ],
+  events: [ "popupshowing editpopup", "DOMMenuBarActive menubar",
+            "DOMMenuItemActive editmenu", "popupshown editpopup" ],
   test() {
     synthesizeMouse(document.getElementById("editmenu"), 8, 8, { shiftKey : true });
   },
@@ -672,14 +589,13 @@ var popupTests = [
 },
 {
   testname: "press on menuitem",
-  events: [
-    "DOMMenuInactive editpopup",
-    "DOMMenuItemInactive editmenu",
-    "DOMMenuBarInactive menubar",
-    "command copy",
-    "popuphiding editpopup",
-    "popuphidden editpopup",
-  ],
+  events: [ "DOMMenuInactive editpopup",
+            "DOMMenuBarInactive menubar",
+            "DOMMenuItemInactive editmenu",
+            "DOMMenuItemInactive editmenu",
+            "command copy", "popuphiding editpopup", "popuphidden editpopup",
+            "DOMMenuItemInactive copy",
+           ],
   test() {
     synthesizeMouse(document.getElementById("copy"), 8, 8, { });
   },
@@ -691,12 +607,8 @@ var popupTests = [
   // this test ensures that the menu can still be opened by clicking after selecting
   // a menuitem from the menu. See bug 399350.
   testname: "press on menu after menuitem selected",
-  events: [
-    "DOMMenuBarActive menubar",
-    "DOMMenuItemActive editmenu",
-    "popupshowing editpopup",
-    "popupshown editpopup",
-  ],
+  events: [ "popupshowing editpopup", "DOMMenuBarActive menubar",
+            "DOMMenuItemActive editmenu", "popupshown editpopup" ],
   test() { synthesizeMouse(document.getElementById("editmenu"), 8, 8, { }); },
   result (testname) {
     checkActive(document.getElementById("editpopup"), "", testname);
@@ -705,14 +617,13 @@ var popupTests = [
 },
 {  // try selecting a different command
   testname: "press on menuitem again",
-  events: [
-    "DOMMenuInactive editpopup",
-    "DOMMenuItemInactive editmenu",
-    "DOMMenuBarInactive menubar",
-    "command paste",
-    "popuphiding editpopup",
-    "popuphidden editpopup",
-  ],
+  events: [ "DOMMenuInactive editpopup",
+            "DOMMenuBarInactive menubar",
+            "DOMMenuItemInactive editmenu",
+            "DOMMenuItemInactive editmenu",
+            "command paste", "popuphiding editpopup", "popuphidden editpopup",
+            "DOMMenuItemInactive paste",
+           ],
   test() {
     synthesizeMouse(document.getElementById("paste"), 8, 8, { });
   },
@@ -727,7 +638,7 @@ var popupTests = [
 },
 {
   testname: "Deactivate menubar with tab key",
-  events: closeAfterF10Events(),
+  events: closeAfterF10Events(true),
   test() { synthesizeKey("KEY_Tab"); },
   result(testname) {
     is(document.getElementById("filemenu").openedWithKey, false, testname + " openedWithKey");
@@ -740,14 +651,8 @@ var popupTests = [
 },
 {
   testname: "Deactivate menubar with escape key",
-  events: closeAfterF10Events(),
-  test() {
-    synthesizeKey("KEY_Escape");
-    if (kIsLinux) {
-      // One to close the menu, one to deactivate the menubar.
-      synthesizeKey("KEY_Escape");
-    }
-  },
+  events: closeAfterF10Events(false),
+  test() { synthesizeKey("KEY_Escape"); },
   result(testname) {
     is(document.getElementById("filemenu").openedWithKey, false, testname + " openedWithKey");
   }
@@ -759,7 +664,7 @@ var popupTests = [
 },
 {
   testname: "Deactivate menubar with f10 key",
-  events: closeAfterF10Events(),
+  events: closeAfterF10Events(true),
   test() { synthesizeKey("KEY_F10"); },
   result(testname) {
     is(document.getElementById("filemenu").openedWithKey, false, testname + " openedWithKey");
@@ -767,14 +672,14 @@ var popupTests = [
 },
 {
   testname: "F10 to activate menubar for alt deactivation",
-  condition() { return kIsWindows; },
+  condition() { return (navigator.platform.indexOf("Win") == 0) },
   events: [ "DOMMenuBarActive menubar", "DOMMenuItemActive filemenu" ],
   test() { synthesizeKey("KEY_F10"); },
 },
 {
   testname: "Deactivate menubar with alt key",
-  condition() { return kIsWindows; },
-  events: [ "DOMMenuItemInactive filemenu", "DOMMenuBarInactive menubar" ],
+  condition() { return (navigator.platform.indexOf("Win") == 0) },
+  events: [ "DOMMenuBarInactive menubar", "DOMMenuItemInactive filemenu"  ],
   test() { synthesizeKey("KEY_Alt"); },
   result(testname) {
     is(document.getElementById("filemenu").openedWithKey, false, testname + " openedWithKey");
@@ -797,13 +702,9 @@ var popupTests = [
 
 {
   testname: "Open menu and press alt key by itself - open menu",
-  events: [
-    "DOMMenuBarActive menubar",
-    "DOMMenuItemActive filemenu",
-    "popupshowing filepopup",
-    "DOMMenuItemActive item1",
-    "popupshown filepopup",
-  ],
+  events: [ "DOMMenuBarActive menubar",
+            "popupshowing filepopup", "DOMMenuItemActive filemenu",
+            "DOMMenuItemActive item1", "popupshown filepopup" ],
   test() { synthesizeKey("F", { altKey: true }); },
   result (testname) {
     checkOpen("filemenu", testname);
@@ -811,14 +712,10 @@ var popupTests = [
 },
 {
   testname: "Open menu and press alt key by itself - close menu",
-  events: [
-    "popuphiding filepopup",
-    "popuphidden filepopup",
-    "DOMMenuItemInactive item1",
-    "DOMMenuInactive filepopup",
-    "DOMMenuItemInactive filemenu",
-    "DOMMenuBarInactive menubar",
-  ],
+  events: [ "popuphiding filepopup", "popuphidden filepopup",
+            "DOMMenuItemInactive item1", "DOMMenuInactive filepopup",
+            "DOMMenuBarInactive menubar", "DOMMenuItemInactive filemenu",
+            "DOMMenuItemInactive filemenu" ],
   test() {
     synthesizeKey("KEY_Alt");
   },
@@ -827,18 +724,16 @@ var popupTests = [
   }
 },
 
-// Following 4 tests are a test of bug 616797, don't insert any new tests
+// Fllowing 4 tests are a test of bug 616797, don't insert any new tests
 // between them.
 {
   testname: "Open file menu by accelerator",
-  condition() { return kIsWindows; },
-  events: [
-    "DOMMenuBarActive menubar",
-    "DOMMenuItemActive filemenu",
-    "popupshowing filepopup",
-    "DOMMenuItemActive item1",
-    "popupshown filepopup"
-  ],
+  condition() { return (navigator.platform.indexOf("Win") == 0) },
+  events() {
+    return [ "DOMMenuBarActive menubar", "popupshowing filepopup",
+             "DOMMenuItemActive filemenu", "DOMMenuItemActive item1",
+             "popupshown filepopup" ];
+  },
   test() {
     synthesizeKey("KEY_Alt", {type: "keydown"});
     synthesizeKey("f", {altKey: true});
@@ -847,23 +742,24 @@ var popupTests = [
 },
 {
   testname: "Close file menu by click at outside of popup menu",
-  condition() { return kIsWindows; },
-  events: [
-    "popuphiding filepopup",
-    "popuphidden filepopup",
-    "DOMMenuItemInactive item1",
-    "DOMMenuInactive filepopup",
-    "DOMMenuItemInactive filemenu",
-    "DOMMenuBarInactive menubar",
-  ],
+  condition() { return (navigator.platform.indexOf("Win") == 0) },
+  events() {
+    return [ "popuphiding filepopup", "popuphidden filepopup",
+             "DOMMenuItemInactive item1", "DOMMenuInactive filepopup",
+             "DOMMenuBarInactive menubar", "DOMMenuItemInactive filemenu",
+             "DOMMenuItemInactive filemenu" ];
+  },
   test() {
+    // XXX hidePopup() causes DOMMenuItemInactive event to be fired twice.
     document.getElementById("filepopup").hidePopup();
   }
 },
 {
   testname: "Alt keydown set focus the menubar",
-  condition() { return kIsWindows; },
-  events: [ "DOMMenuBarActive menubar", "DOMMenuItemActive filemenu" ],
+  condition() { return (navigator.platform.indexOf("Win") == 0) },
+  events() {
+    return [ "DOMMenuBarActive menubar", "DOMMenuItemActive filemenu" ];
+  },
   test() {
     synthesizeKey("KEY_Alt");
   },
@@ -873,8 +769,10 @@ var popupTests = [
 },
 {
   testname: "unset focus the menubar",
-  condition() { return kIsWindows; },
-  events: [ "DOMMenuItemInactive filemenu", "DOMMenuBarInactive menubar" ],
+  condition() { return (navigator.platform.indexOf("Win") == 0) },
+  events() {
+    return [ "DOMMenuBarInactive menubar", "DOMMenuItemInactive filemenu" ];
+  },
   test() {
     synthesizeKey("KEY_Alt");
   }
@@ -884,8 +782,10 @@ var popupTests = [
 {
   testname: "Alt key state before deactivating the window shouldn't prevent " +
             "next Alt key handling",
-  condition() { return kIsWindows; },
-  events: [ "DOMMenuBarActive menubar", "DOMMenuItemActive filemenu" ],
+  condition() { return (navigator.platform.indexOf("Win") == 0) },
+  events() {
+    return [ "DOMMenuBarActive menubar", "DOMMenuItemActive filemenu" ];
+  },
   test() {
     synthesizeKey("KEY_Alt", {type: "keydown"});
     synthesizeKey("KEY_Tab", {type: "keydown"}); // cancels the Alt key

toolkit/content/widgets/menulist.js

@@ -7,9 +7,6 @@
 // This is loaded into all XUL windows. Wrap in a block to prevent
 // leaking to window scope.
 {
-  const { AppConstants } = ChromeUtils.importESModule(
-    "resource://gre/modules/AppConstants.sys.mjs"
-  );
   const MozXULMenuElement = MozElements.MozElementMixin(XULMenuElement);
   const MenuBaseControl = MozElements.BaseControlMixin(MozXULMenuElement);
 
@@ -41,35 +38,20 @@
       this.addEventListener(
         "keypress",
         event => {
-          if (
-            event.defaultPrevented ||
-            event.altKey ||
-            event.ctrlKey ||
-            event.metaKey
-          ) {
+          if (event.altKey || event.ctrlKey || event.metaKey) {
             return;
           }
 
           if (
-            AppConstants.platform === "macosx" &&
-            !this.open &&
+            !event.defaultPrevented &&
             (event.keyCode == KeyEvent.DOM_VK_UP ||
-              event.keyCode == KeyEvent.DOM_VK_DOWN)
-          ) {
-            // This should open the menulist on macOS, see
-            // XULButtonElement::PostHandleEvent.
-            return;
-          }
-
-          if (
-            event.keyCode == KeyEvent.DOM_VK_UP ||
-            event.keyCode == KeyEvent.DOM_VK_DOWN ||
-            event.keyCode == KeyEvent.DOM_VK_PAGE_UP ||
-            event.keyCode == KeyEvent.DOM_VK_PAGE_DOWN ||
-            event.keyCode == KeyEvent.DOM_VK_HOME ||
-            event.keyCode == KeyEvent.DOM_VK_END ||
-            event.keyCode == KeyEvent.DOM_VK_BACK_SPACE ||
-            event.charCode > 0
+              event.keyCode == KeyEvent.DOM_VK_DOWN ||
+              event.keyCode == KeyEvent.DOM_VK_PAGE_UP ||
+              event.keyCode == KeyEvent.DOM_VK_PAGE_DOWN ||
+              event.keyCode == KeyEvent.DOM_VK_HOME ||
+              event.keyCode == KeyEvent.DOM_VK_END ||
+              event.keyCode == KeyEvent.DOM_VK_BACK_SPACE ||
+              event.charCode > 0)
           ) {
             // Moving relative to an item: start from the currently selected item
             this.activeChild = this.mSelectedInternal;

toolkit/content/xul.css

@@ -225,14 +225,6 @@ html|label.toolbarbutton-badge:empty {
 .toolbarbutton-text,
 .toolbarbutton-badge-stack,
 .toolbarbutton-menu-dropmarker,
-.menubar-left,
-.menubar-text,
-.menu-text,
-.menu-iconic-text,
-.menu-iconic-highlightable-text,
-.menu-iconic-left,
-.menu-right,
-.menu-accel-container,
 .button-box {
   /* Preserves legacy behavior */
   pointer-events: none;
@@ -360,6 +352,13 @@ tooltip {
   text-shadow: none;
 }
 
+:is(toolbarbutton[type="menu"], button[type="menu"], menulist, menu) > :is(menupopup, panel) {
+  /* TODO: These could be regular abspos frames (just not in the top layer),
+   * once nsMenuFrame is ported to modern flex layout. */
+  position: static;
+  -moz-top-layer: none;
+}
+
 tooltip {
   appearance: auto;
   -moz-default-appearance: tooltip;

toolkit/themes/shared/menulist-shared.css

@@ -39,7 +39,6 @@
 
 #label-box {
   min-width: 0;
-  pointer-events: none;
 }
 
 #label-box:not([native]) {
@@ -48,15 +47,11 @@
   font-weight: 600;
 }
 
-dropmarker {
-  pointer-events: none;
-}
-
 dropmarker:not([native]) {
   display: -moz-box;
   appearance: none;
   width: 12px;
-  height: unset;
+  height: 12px;
 }
 
 dropmarker:not([native])::part(icon) {

toolkit/themes/windows/global/popup.css

@@ -28,6 +28,8 @@ panel {
     appearance: auto;
     -moz-default-appearance: menupopup;
 
+    -moz-box-layout: legacy;
+
     /* Native menus are always light */
     color-scheme: light !important;
 

widget/Theme.cpp

@@ -1132,11 +1132,15 @@ bool Theme::DoDrawWidgetBackground(PaintBackendData& aPaintData,
   const DocumentState docState = pc->Document()->GetDocumentState();
   ElementState elementState = GetContentState(aFrame, aAppearance);
   if (aAppearance == StyleAppearance::MozMenulistArrowButton) {
+    bool isHTML = IsHTMLContent(aFrame);
+    nsIFrame* parentFrame = aFrame->GetParent();
+    bool isMenulist = !isHTML && parentFrame->IsMenuFrame();
     // HTML select and XUL menulist dropdown buttons get state from the
     // parent.
-    nsIFrame* parentFrame = aFrame->GetParent();
-    aFrame = parentFrame;
-    elementState = GetContentState(parentFrame, aAppearance);
+    if (isHTML || isMenulist) {
+      aFrame = parentFrame;
+      elementState = GetContentState(parentFrame, aAppearance);
+    }
   }
 
   // Paint the outline iff we're asked to draw overflow and we have

widget/gtk/nsNativeThemeGTK.cpp

@@ -17,7 +17,7 @@
 #include "nsNameSpaceManager.h"
 #include "nsGfxCIID.h"
 #include "nsTransform2D.h"
-#include "nsXULPopupManager.h"
+#include "nsMenuFrame.h"
 #include "tree/nsTreeBodyFrame.h"
 #include "prlink.h"
 #include "nsGkAtoms.h"
@@ -31,7 +31,6 @@
 #include <gtk/gtk.h>
 
 #include "gfxContext.h"
-#include "mozilla/dom/XULButtonElement.h"
 #include "mozilla/gfx/BorrowedContext.h"
 #include "mozilla/gfx/HelpersCairo.h"
 #include "mozilla/gfx/PathHelpers.h"
@@ -247,9 +246,14 @@ bool nsNativeThemeGTK::GetGtkWidgetAndState(StyleAppearance aAppearance,
           aAppearance == StyleAppearance::Radiomenuitem ||
           aAppearance == StyleAppearance::Menuseparator ||
           aAppearance == StyleAppearance::Menuarrow) {
-        auto* item = dom::XULButtonElement::FromNode(aFrame->GetContent());
-        if (item && item->IsOnMenuBar()) {
-          aState->inHover = CheckBooleanAttr(aFrame, nsGkAtoms::open);
+        bool isTopLevel = false;
+        nsMenuFrame* menuFrame = do_QueryFrame(aFrame);
+        if (menuFrame) {
+          isTopLevel = menuFrame->IsOnMenuBar();
+        }
+
+        if (isTopLevel) {
+          aState->inHover = menuFrame->IsOpen();
         } else {
           aState->inHover = CheckBooleanAttr(aFrame, nsGkAtoms::menuactive);
         }
@@ -515,8 +519,8 @@ bool nsNativeThemeGTK::GetGtkWidgetAndState(StyleAppearance aAppearance,
       aGtkWidgetType = MOZ_GTK_MENUBAR;
       break;
     case StyleAppearance::Menuitem: {
-      auto* item = dom::XULButtonElement::FromNode(aFrame->GetContent());
-      if (item && item->IsOnMenuBar()) {
+      nsMenuFrame* menuFrame = do_QueryFrame(aFrame);
+      if (menuFrame && menuFrame->IsOnMenuBar()) {
         aGtkWidgetType = MOZ_GTK_MENUBARITEM;
         break;
       }

widget/nsNativeTheme.cpp

@@ -18,6 +18,7 @@
 #include "nsPIDOMWindow.h"
 #include "nsProgressFrame.h"
 #include "nsMeterFrame.h"
+#include "nsMenuFrame.h"
 #include "nsRangeFrame.h"
 #include "nsCSSRendering.h"
 #include "ImageContainer.h"

widget/windows/SystemStatusBar.cpp

@@ -14,10 +14,10 @@
 #include "mozilla/LinkedList.h"
 #include "mozilla/StaticPtr.h"
 #include "mozilla/widget/IconLoader.h"
-#include "mozilla/dom/XULButtonElement.h"
 #include "nsComputedDOMStyle.h"
 #include "nsIContentPolicy.h"
 #include "nsISupports.h"
+#include "nsMenuFrame.h"
 #include "nsMenuPopupFrame.h"
 #include "nsXULPopupManager.h"
 #include "nsIDocShell.h"
@@ -109,7 +109,8 @@ nsresult StatusBarEntry::Init() {
 
   // First, look at the content node's "image" attribute.
   nsAutoString imageURIString;
-  bool hasImageAttr = mMenu->GetAttr(nsGkAtoms::image, imageURIString);
+  bool hasImageAttr =
+      mMenu->GetAttr(kNameSpaceID_None, nsGkAtoms::image, imageURIString);
 
   nsresult rv;
   nsCOMPtr<nsIURI> iconURI;
@@ -210,13 +211,14 @@ LRESULT StatusBarEntry::OnMessage(HWND hWnd, UINT msg, WPARAM wp, LPARAM lp) {
   if (msg == WM_USER &&
       (LOWORD(lp) == NIN_SELECT || LOWORD(lp) == NIN_KEYSELECT ||
        LOWORD(lp) == WM_CONTEXTMENU)) {
-    auto* menu = dom::XULButtonElement::FromNode(mMenu);
+    nsMenuFrame* menu = do_QueryFrame(mMenu->GetPrimaryFrame());
     if (!menu) {
       return TRUE;
     }
 
-    nsMenuPopupFrame* popupFrame = menu->GetMenuPopup(FlushType::None);
-    if (NS_WARN_IF(!popupFrame)) {
+    nsMenuPopupFrame* popupFrame = menu->GetPopup();
+    MOZ_DIAGNOSTIC_ASSERT(popupFrame);
+    if (!popupFrame) {
       return TRUE;
     }
 
@@ -246,7 +248,7 @@ LRESULT StatusBarEntry::OnMessage(HWND hWnd, UINT msg, WPARAM wp, LPARAM lp) {
       nsEventStatus status = nsEventStatus_eIgnore;
       WidgetMouseEvent event(true, eXULSystemStatusBarClick, nullptr,
                              WidgetMouseEvent::eReal);
-      RefPtr<nsPresContext> presContext = popupFrame->PresContext();
+      RefPtr<nsPresContext> presContext = menu->PresContext();
       EventDispatcher::Dispatch(mMenu, presContext, &event, nullptr, &status);
       return DefWindowProc(hWnd, msg, wp, lp);
     }

widget/windows/nsNativeThemeWin.cpp

@@ -20,7 +20,6 @@
 #include "mozilla/StaticPrefs_layout.h"
 #include "mozilla/StaticPrefs_widget.h"
 #include "mozilla/WindowsVersion.h"
-#include "mozilla/dom/XULButtonElement.h"
 #include "nsColor.h"
 #include "nsComboboxControlFrame.h"
 #include "nsDeviceContext.h"
@@ -30,6 +29,7 @@
 #include "nsIFrame.h"
 #include "nsLayoutUtils.h"
 #include "nsLookAndFeel.h"
+#include "nsMenuFrame.h"
 #include "nsNameSpaceManager.h"
 #include "Theme.h"
 #include "nsPresContext.h"
@@ -132,8 +132,12 @@ static int32_t GetClassicWindowFrameButtonState(ElementState elementState) {
 }
 
 static bool IsTopLevelMenu(nsIFrame* aFrame) {
-  auto* menu = dom::XULButtonElement::FromNodeOrNull(aFrame->GetContent());
-  return menu && menu->IsOnMenuBar();
+  bool isTopLevel(false);
+  nsMenuFrame* menuFrame = do_QueryFrame(aFrame);
+  if (menuFrame) {
+    isTopLevel = menuFrame->IsOnMenuBar();
+  }
+  return isTopLevel;
 }
 
 static MARGINS GetCheckboxMargins(HANDLE theme, HDC hdc) {
@@ -1163,12 +1167,14 @@ nsresult nsNativeThemeWin::GetThemePartAndState(nsIFrame* aFrame,
       return NS_OK;
     }
     case StyleAppearance::MozMenulistArrowButton: {
+      bool isHTML = IsHTMLContent(aFrame);
+      nsIFrame* parentFrame = aFrame->GetParent();
+      bool isMenulist = !isHTML && parentFrame->IsMenuFrame();
       bool isOpen = false;
 
       // HTML select and XUL menulist dropdown buttons get state from the
       // parent.
-      nsIFrame* parentFrame = aFrame->GetParent();
-      aFrame = parentFrame;
+      if (isHTML || isMenulist) aFrame = parentFrame;
 
       ElementState elementState = GetContentState(aFrame, aAppearance);
       aPart = CBP_DROPMARKER_VISTA;
@@ -1176,18 +1182,22 @@ nsresult nsNativeThemeWin::GetThemePartAndState(nsIFrame* aFrame,
       // For HTML controls with author styling, we should fall
       // back to the old dropmarker style to avoid clashes with
       // author-specified backgrounds and borders (bug #441034)
-      if (IsWidgetStyled(aFrame->PresContext(), aFrame,
-                         StyleAppearance::Menulist)) {
+      if (isHTML && IsWidgetStyled(aFrame->PresContext(), aFrame,
+                                   StyleAppearance::Menulist))
         aPart = CBP_DROPMARKER;
-      }
 
       if (elementState.HasState(ElementState::DISABLED)) {
         aState = TS_DISABLED;
         return NS_OK;
       }
 
-      if (nsComboboxControlFrame* ccf = do_QueryFrame(aFrame)) {
-        isOpen = ccf->IsDroppedDown();
+      if (isHTML) {
+        nsComboboxControlFrame* ccf = do_QueryFrame(aFrame);
+        isOpen = (ccf && ccf->IsDroppedDown());
+      } else
+        isOpen = IsOpenButton(aFrame);
+
+      if (isHTML) {
         if (isOpen) {
           /* Hover is propagated, but we need to know whether we're hovering
            * just the combobox frame, not the dropdown frame. But, we can't get
@@ -1204,7 +1214,6 @@ nsresult nsNativeThemeWin::GetThemePartAndState(nsIFrame* aFrame,
          * hover effect. When the frame isn't isn't HTML content, we cheat and
          * force the dropdown state to be normal. (Bug 430434)
          */
-        isOpen = IsOpenButton(aFrame);
         aState = TS_NORMAL;
         return NS_OK;
       }
@@ -1213,7 +1222,7 @@ nsresult nsNativeThemeWin::GetThemePartAndState(nsIFrame* aFrame,
 
       // Dropdown button active state doesn't need :hover.
       if (elementState.HasState(ElementState::ACTIVE)) {
-        if (isOpen) {
+        if (isOpen && (isHTML || isMenulist)) {
           // XXX Button should look active until the mouse is released, but
           //     without making it look active when the popup is clicked.
           return NS_OK;
@@ -1239,13 +1248,17 @@ nsresult nsNativeThemeWin::GetThemePartAndState(nsIFrame* aFrame,
     case StyleAppearance::Menuitem:
     case StyleAppearance::Checkmenuitem:
     case StyleAppearance::Radiomenuitem: {
+      bool isTopLevel = false;
+      bool isOpen = false;
+      bool isHover = false;
+      nsMenuFrame* menuFrame = do_QueryFrame(aFrame);
       ElementState elementState = GetContentState(aFrame, aAppearance);
 
-      auto* menu = dom::XULButtonElement::FromNodeOrNull(aFrame->GetContent());
+      isTopLevel = IsTopLevelMenu(aFrame);
 
-      const bool isTopLevel = IsTopLevelMenu(aFrame);
-      const bool isOpen = menu && menu->IsMenuPopupOpen();
-      const bool isHover = IsMenuActive(aFrame, aAppearance);
+      if (menuFrame) isOpen = menuFrame->IsOpen();
+
+      isHover = IsMenuActive(aFrame, aAppearance);
 
       if (isTopLevel) {
         aPart = MENU_BARITEM;
@@ -2781,33 +2794,35 @@ nsresult nsNativeThemeWin::ClassicGetThemePartAndState(
     case StyleAppearance::Menuitem:
     case StyleAppearance::Checkmenuitem:
     case StyleAppearance::Radiomenuitem: {
+      bool isTopLevel = false;
+      bool isOpen = false;
+      nsMenuFrame* menuFrame = do_QueryFrame(aFrame);
       ElementState elementState = GetContentState(aFrame, aAppearance);
 
-      auto* menu = dom::XULButtonElement::FromNodeOrNull(aFrame->GetContent());
-
-      const bool isTopLevel = IsTopLevelMenu(aFrame);
-      const bool isOpen = menu && menu->IsMenuPopupOpen();
-
       // We indicate top-level-ness using aPart. 0 is a normal menu item,
       // 1 is a top-level menu item. The state of the item is composed of
       // DFCS_* flags only.
       aPart = 0;
       aState = 0;
 
+      if (menuFrame) {
+        // If this is a real menu item, we should check if it is part of the
+        // main menu bar or not, and if it is a container, as these affect
+        // rendering.
+        isTopLevel = menuFrame->IsOnMenuBar();
+        isOpen = menuFrame->IsOpen();
+      }
+
       if (elementState.HasState(ElementState::DISABLED)) {
         aState |= DFCS_INACTIVE;
       }
 
       if (isTopLevel) {
         aPart = 1;
-        if (isOpen) {
-          aState |= DFCS_PUSHED;
-        }
+        if (isOpen) aState |= DFCS_PUSHED;
       }
 
-      if (IsMenuActive(aFrame, aAppearance)) {
-        aState |= DFCS_HOT;
-      }
+      if (IsMenuActive(aFrame, aAppearance)) aState |= DFCS_HOT;
 
       return NS_OK;
     }
@@ -2858,9 +2873,13 @@ nsresult nsNativeThemeWin::ClassicGetThemePartAndState(
       aState = DFCS_SCROLLCOMBOBOX;
 
       nsIFrame* parentFrame = aFrame->GetParent();
+      bool isHTML = IsHTMLContent(aFrame);
+      bool isMenulist = !isHTML && parentFrame->IsMenuFrame();
+      bool isOpen = false;
+
       // HTML select and XUL menulist dropdown buttons get state from the
       // parent.
-      aFrame = parentFrame;
+      if (isHTML || isMenulist) aFrame = parentFrame;
 
       ElementState elementState = GetContentState(aFrame, aAppearance);
 
@@ -2869,18 +2888,15 @@ nsresult nsNativeThemeWin::ClassicGetThemePartAndState(
         return NS_OK;
       }
 
-      bool isOpen = false;
-      if (nsComboboxControlFrame* ccf = do_QueryFrame(aFrame)) {
-        isOpen = ccf->IsDroppedDown();
-      } else {
+      if (isHTML) {
+        nsComboboxControlFrame* ccf = do_QueryFrame(aFrame);
+        isOpen = (ccf && ccf->IsDroppedDown());
+      } else
         isOpen = IsOpenButton(aFrame);
-      }
 
       // XXX Button should look active until the mouse is released, but
       //     without making it look active when the popup is clicked.
-      if (isOpen) {
-        return NS_OK;
-      }
+      if (isOpen && (isHTML || isMenulist)) return NS_OK;
 
       // Dropdown button active state doesn't need :hover.
       if (elementState.HasState(ElementState::ACTIVE))