Bug 397012, menus don't close on second click, r+sr=bz

layout/xul/base/src/nsIMenuParent.h

@@ -85,6 +85,10 @@ public:
   // cleared. This should return true if the menu should be deselected
   // by the caller.
   virtual PRBool MenuClosed() = 0;
+
+  // return true if aMenuFrame is the menu that was recently closed. The
+  // recently closed menu state is cleared by this method.
+  virtual PRBool IsRecentlyClosed(nsMenuFrame* aMenuFrame) = 0;
 };
 
 #endif

layout/xul/base/src/nsMenuBarFrame.cpp

@@ -85,6 +85,8 @@ nsMenuBarFrame::nsMenuBarFrame(nsIPresShell* aShell, nsStyleContext* aContext):
     mMenuBarListener(nsnull),
     mStayActive(PR_FALSE),
     mIsActive(PR_FALSE),
+    mCurrentMenu(nsnull),
+    mRecentlyClosedMenu(nsnull),
     mTarget(nsnull),
     mCaretWasVisible(PR_FALSE)
 {
@@ -338,6 +340,7 @@ nsMenuBarFrame::SetCurrentMenuItem(nsMenuFrame* aMenuItem)
 
   NS_ENSURE_TRUE(weakFrame.IsAlive(), NS_OK);
   mCurrentMenu = aMenuItem;
+  mRecentlyClosedMenu = nsnull;
 
   return NS_OK;
 }
@@ -461,6 +464,7 @@ nsMenuBarFrame::MenuClosed()
 {
   SetActive(PR_FALSE);
   if (!mIsActive && mCurrentMenu) {
+    SetRecentlyClosed(mCurrentMenu);
     mCurrentMenu->SelectMenu(PR_FALSE);
     mCurrentMenu = nsnull;
     return PR_TRUE;

layout/xul/base/src/nsMenuBarFrame.h

@@ -79,6 +79,19 @@ public:
 
   PRBool IsMenuOpen() { return mCurrentMenu && mCurrentMenu->IsOpen(); }
 
+  // return true if aMenuFrame was the recently closed menu, clearing the
+  // the recent menu state in the process.
+  PRBool IsRecentlyClosed(nsMenuFrame* aMenuFrame)
+  {
+    PRBool match = (aMenuFrame == mRecentlyClosedMenu);
+    mRecentlyClosedMenu = nsnull;
+    return match;
+  }
+  void SetRecentlyClosed(nsMenuFrame* aRecentlyClosedMenu)
+  {
+    mRecentlyClosedMenu = aRecentlyClosedMenu;
+  }
+
   void InstallKeyboardNavigator();
   void RemoveKeyboardNavigator();
 
@@ -137,6 +150,12 @@ protected:
   // be null if no menu is active.
   nsMenuFrame* mCurrentMenu;
 
+  // When a menu is closed by clicking the menu label, the menu is rolled up
+  // and the mouse event is fired at the menu. The menu that was closed is
+  // stored here, to avoid having it reopen again during the mouse event.
+  // This is OK to be a weak reference as it is never dereferenced.
+  nsMenuFrame* mRecentlyClosedMenu;
+
   nsIDOMEventTarget* mTarget;
 
 private:

layout/xul/base/src/nsMenuBarListener.cpp

@@ -193,6 +193,8 @@ nsMenuBarListener::KeyUp(nsIDOMEvent* aKeyEvent)
 nsresult
 nsMenuBarListener::KeyPress(nsIDOMEvent* aKeyEvent)
 {
+  mMenuBarFrame->SetRecentlyClosed(nsnull);
+
   // if event has already been handled, bail
   nsCOMPtr<nsIDOMNSUIEvent> uiEvent ( do_QueryInterface(aKeyEvent) );
   if ( uiEvent ) {
@@ -382,6 +384,7 @@ nsMenuBarListener::MouseDown(nsIDOMEvent* aMouseEvent)
 nsresult 
 nsMenuBarListener::MouseUp(nsIDOMEvent* aMouseEvent)
 {
+  mMenuBarFrame->SetRecentlyClosed(nsnull);
   return NS_OK; // means I am NOT consuming event
 }
 

layout/xul/base/src/nsMenuFrame.cpp

@@ -543,7 +543,7 @@ nsMenuFrame::ToggleMenuState()
 {
   if (IsOpen())
     CloseMenu(PR_FALSE);
-  else
+  else if (!mMenuParent->IsRecentlyClosed(this))
     OpenMenu(PR_FALSE);
 }
 

layout/xul/base/src/nsMenuPopupFrame.h

@@ -156,6 +156,8 @@ public:
 
   virtual PRBool MenuClosed() { return PR_TRUE; }
 
+  virtual PRBool IsRecentlyClosed(nsMenuFrame* aMenuFrame) { return PR_FALSE; }
+
   NS_IMETHOD GetWidget(nsIWidget **aWidget);
 
   // The dismissal listener gets created and attached to the window.