Bug 397606, don't deactivate menubar when switching menus, r+sr=bz, a=damon

layout/xul/base/public/nsXULPopupManager.h

@@ -336,6 +336,10 @@ public:
   // if a popup manager could not be allocated
   static nsXULPopupManager* GetInstance();
 
+  // get the frame for a content node aContent if the frame's type
+  // matches aFrameType. Otherwise, return null.
+  nsIFrame* GetFrameOfTypeForContent(nsIContent* aContent, nsIAtom* aFrameType);
+
   // 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
@@ -577,10 +581,6 @@ protected:
   nsXULPopupManager();
   ~nsXULPopupManager();
 
-  // get the frame for a content node aContent if the frame's type
-  // matches aFrameType. Otherwise, return null.
-  nsIFrame* GetFrameOfTypeForContent(nsIContent* aContent, nsIAtom* aFrameType);
-
   // get the nsMenuFrame, if any, for the given content node
   nsMenuFrame* GetMenuFrameForContent(nsIContent* aContent);
 

layout/xul/base/src/nsMenuBarFrame.cpp

@@ -83,6 +83,7 @@ NS_NewMenuBarFrame(nsIPresShell* aPresShell, nsStyleContext* aContext)
 nsMenuBarFrame::nsMenuBarFrame(nsIPresShell* aShell, nsStyleContext* aContext):
   nsBoxFrame(aShell, aContext),
     mMenuBarListener(nsnull),
+    mStayActive(PR_FALSE),
     mIsActive(PR_FALSE),
     mTarget(nsnull),
     mCaretWasVisible(PR_FALSE)
@@ -129,6 +130,10 @@ nsMenuBarFrame::SetActive(PRBool aActiveFlag)
     return NS_OK;
 
   if (!aActiveFlag) {
+    // 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.
@@ -347,10 +352,12 @@ nsMenuBarFrame::CurrentMenuIsBeingDestroyed()
 class nsMenuBarSwitchMenu : public nsRunnable
 {
 public:
-  nsMenuBarSwitchMenu(nsIContent *aOldMenu,
+  nsMenuBarSwitchMenu(nsIContent* aMenuBar,
+                      nsIContent *aOldMenu,
                       nsIContent *aNewMenu,
                       PRBool aSelectFirstItem)
-    : mOldMenu(aOldMenu), mNewMenu(aNewMenu), mSelectFirstItem(aSelectFirstItem)
+    : mMenuBar(aMenuBar), mOldMenu(aOldMenu), mNewMenu(aNewMenu),
+      mSelectFirstItem(aSelectFirstItem)
   {
   }
 
@@ -360,8 +367,23 @@ public:
     if (!pm)
       return NS_ERROR_UNEXPECTED;
 
-    if (mOldMenu)
+    // 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 = nsnull;
+    if (mOldMenu && mNewMenu) {
+      menubar = static_cast<nsMenuBarFrame *>
+        (pm->GetFrameOfTypeForContent(mMenuBar, nsGkAtoms::menuBarFrame));
+      menubar->SetStayActive(PR_TRUE);
+    }
+
+    if (mOldMenu) {
+      nsWeakFrame weakMenuBar(menubar);
       pm->HidePopup(mOldMenu, PR_FALSE, PR_FALSE, PR_FALSE);
+      // clear the flag again
+      if (mNewMenu && weakMenuBar.IsAlive())
+        menubar->SetStayActive(PR_FALSE);
+    }
+
     if (mNewMenu)
       pm->ShowMenu(mNewMenu, mSelectFirstItem, PR_FALSE);
 
@@ -369,6 +391,7 @@ public:
   }
 
 private:
+  nsCOMPtr<nsIContent> mMenuBar;
   nsCOMPtr<nsIContent> mOldMenu;
   nsCOMPtr<nsIContent> mNewMenu;
   PRBool mSelectFirstItem;
@@ -417,7 +440,7 @@ nsMenuBarFrame::ChangeMenuItem(nsMenuFrame* aMenuItem,
   // use an event so that hiding and showing can be done synchronously, which
   // avoids flickering
   nsCOMPtr<nsIRunnable> event =
-    new nsMenuBarSwitchMenu(aOldMenu, aNewMenu, aSelectFirstItem);
+    new nsMenuBarSwitchMenu(GetContent(), aOldMenu, aNewMenu, aSelectFirstItem);
   return NS_DispatchToCurrentThread(event);
 }
 

layout/xul/base/src/nsMenuBarFrame.h

@@ -92,6 +92,9 @@ public:
 
 // Non-interface helpers
 
+  void
+  SetStayActive(PRBool 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();
@@ -125,7 +128,11 @@ public:
 protected:
   nsMenuBarListener* mMenuBarListener; // The listener that tells us about key and mouse events.
 
-  PRBool mIsActive; // Whether or not the menu bar is active (a menu item is highlighted or shown).
+  // flag that is temporarily set when switching from one menu on the menubar to another
+  // to indicate that the menubar should not be deactivated.
+  PRPackedBool mStayActive;
+
+  PRPackedBool mIsActive; // Whether or not the menu bar is active (a menu item is highlighted or shown).
   // The current menu that is active (highlighted), which may not be open. This will
   // be null if no menu is active.
   nsMenuFrame* mCurrentMenu;

toolkit/content/tests/widgets/window_menubar.xul

@@ -154,12 +154,7 @@ var popupTests = [
       // next item needs to be selected
       "popupshowing editpopup", "DOMMenuItemInactive item1",
       // not sure why the menu inactivated event is firing so late
-      "DOMMenuInactive filepopup", "DOMMenuBarInactive menubar",
-      // the new menu gets disabled and reenabled again, I suspect a
-      // check isn't done to see if the old value equals the new value
-      // somewhere
-      "DOMMenuItemInactive editmenu",
-      "DOMMenuBarActive menubar", "DOMMenuItemActive editmenu",
+      "DOMMenuInactive filepopup",
       // finally, the first item is activated and popupshown is fired
       "DOMMenuItemActive copy", "popupshown editpopup"
     ];