Bug 179567 Cannot dismiss menu by clicking on menubar p=aaron r=bryner sr=jag

layout/xul/base/src/nsIMenuParent.h

@@ -151,6 +151,8 @@ public:
   
   NS_IMETHOD IsMenuBar(PRBool& isMenuBar) = 0;
   NS_IMETHOD ConsumeOutsideClicks(PRBool& aConsumeOutsideClicks) = 0;
+  NS_IMETHOD ClearRecentlyRolledUp() = 0;
+  NS_IMETHOD RecentlyRolledUp(nsIMenuFrame *aMenuFrame, PRBool *aJustRolledUp) = 0;
 
   NS_IMETHOD DismissChain() = 0;
   NS_IMETHOD HideChain() = 0;

layout/xul/base/src/nsMenuBarFrame.cpp

@@ -649,6 +649,8 @@ nsMenuBarFrame::Enter()
   if (!mCurrentMenu)
     return NS_OK;
 
+  ClearRecentlyRolledUp();
+
   // See if our menu is open.
   PRBool isOpen = PR_FALSE;
   mCurrentMenu->MenuIsOpen(isOpen);
@@ -665,6 +667,27 @@ nsMenuBarFrame::Enter()
   return NS_OK;
 }
 
+NS_IMETHODIMP
+nsMenuBarFrame::ClearRecentlyRolledUp()
+{
+  // We're no longer in danger of popping down a menu from the same 
+  // click on the menubar, which was supposed to toggle the menu closed
+  mRecentRollupMenu = nsnull;
+
+  return NS_OK;
+}
+
+NS_IMETHODIMP
+nsMenuBarFrame::RecentlyRolledUp(nsIMenuFrame *aMenuFrame, PRBool *aJustRolledUp)
+{
+  // Don't let a click reopen a menu that was just rolled up
+  // from the same click. Otherwise, the user can't click on
+  // a menubar item to toggle its submenu closed.
+  *aJustRolledUp = (mRecentRollupMenu == aMenuFrame);
+
+  return NS_OK;
+}
+
 NS_IMETHODIMP
 nsMenuBarFrame::HideChain()
 {
@@ -674,10 +697,13 @@ nsMenuBarFrame::HideChain()
   if (nsMenuFrame::sDismissalListener)
     nsMenuFrame::sDismissalListener->Unregister();
 
+  ClearRecentlyRolledUp();
   if (mCurrentMenu) {
     mCurrentMenu->ActivateMenu(PR_FALSE);
     mCurrentMenu->SelectMenu(PR_FALSE);
+    mRecentRollupMenu = mCurrentMenu;
   }
+
   return NS_OK;
 }
 

layout/xul/base/src/nsMenuBarFrame.h

@@ -77,6 +77,8 @@ public:
   NS_IMETHOD IsMenuBar(PRBool& isMenuBar) { isMenuBar = PR_TRUE; return NS_OK; };
   NS_IMETHOD ConsumeOutsideClicks(PRBool& aConsumeOutsideClicks) \
     {aConsumeOutsideClicks = PR_FALSE; return NS_OK;};
+  NS_IMETHOD ClearRecentlyRolledUp();
+  NS_IMETHOD RecentlyRolledUp(nsIMenuFrame *aMenuFrame, PRBool *aJustRolledUp);
 
   NS_IMETHOD SetIsContextMenu(PRBool aIsContextMenu) { return NS_OK; };
   NS_IMETHOD GetIsContextMenu(PRBool& aIsContextMenu) { aIsContextMenu = PR_FALSE; return NS_OK; }; 
@@ -140,6 +142,10 @@ protected:
   PRBool mIsActive; // Whether or not the menu bar is active (a menu item is highlighted or shown).
   nsIMenuFrame* mCurrentMenu; // The current menu that is active.
 
+  // Can contain a menu that was rolled up via nsIMenuDismissalListener::Rollup()
+  // if nothing has happened since the last click. Otherwise, contains nsnull.
+  nsIMenuFrame* mRecentRollupMenu; 
+
   nsIDOMEventReceiver* mTarget;
 
   // XXX Hack

layout/xul/base/src/nsMenuBarListener.cpp

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

layout/xul/base/src/nsMenuFrame.cpp

@@ -536,7 +536,33 @@ nsMenuFrame::ToggleMenuState()
     OpenMenu(PR_FALSE);
   }
   else {
-    OpenMenu(PR_TRUE);
+    PRBool justRolledUp = PR_FALSE;
+    if (mMenuParent) {
+      mMenuParent->RecentlyRolledUp(this, &justRolledUp);
+    }
+    if (justRolledUp) {
+      // Don't let a click reopen a menu that was just rolled up
+      // from the same click. Otherwise, the user can't click on
+      // a menubar item to toggle its submenu closed.
+      OpenMenu(PR_FALSE);
+      SelectMenu(PR_TRUE);
+    }
+    else {
+      OpenMenu(PR_TRUE);
+    }
+  }
+
+  if (mMenuParent) {
+    // Make sure the current menu which is being toggled on
+    // the menubar is highlighted
+    mMenuParent->SetActive(PR_FALSE);
+    mMenuParent->SetActive(PR_TRUE);
+    mMenuParent->SetCurrentMenuItem(this);
+    // We've successfully prevented the same click from both
+    // dismissing and reopening this menu. 
+    // Clear the recent rollup state so we don't prevent
+    // this menu from being opened by the next click.
+    mMenuParent->ClearRecentlyRolledUp();
   }
 
   return NS_OK;
@@ -1231,6 +1257,9 @@ nsMenuFrame::KeyboardNavigation(PRUint32 aKeyCode, PRBool& aHandledFlag)
 NS_IMETHODIMP
 nsMenuFrame::Escape(PRBool& aHandledFlag)
 {
+  if (mMenuParent) {
+    mMenuParent->ClearRecentlyRolledUp();
+  }
   nsIFrame* frame = mPopupFrames.FirstChild();
   if (frame) {
     nsMenuPopupFrame* popup = (nsMenuPopupFrame*)frame;
@@ -1641,9 +1670,17 @@ nsMenuFrame::Execute(nsGUIEvent *aEvent)
   SelectMenu(PR_FALSE);
 
   // Now hide all of the open menus.
-  if (mMenuParent)
+  if (mMenuParent) {
     mMenuParent->HideChain();
 
+    // Since menu was not dismissed via click outside menu
+    // we don't want to keep track of this rollup.
+    // Otherwise, we keep track so that the same click 
+    // won't both dismiss and then reopen a menu.
+    mMenuParent->ClearRecentlyRolledUp();
+  }
+
+
   nsEventStatus status = nsEventStatus_eIgnore;
   nsMouseEvent event;
   event.eventStructType = NS_EVENT;

layout/xul/base/src/nsMenuPopupFrame.h

@@ -92,6 +92,8 @@ public:
   NS_IMETHOD GetIsActive(PRBool& isActive) { isActive = PR_FALSE; return NS_OK; };
   NS_IMETHOD IsMenuBar(PRBool& isMenuBar) { isMenuBar = PR_FALSE; return NS_OK; };
   NS_IMETHOD ConsumeOutsideClicks(PRBool& aConsumeOutsideClicks);
+  NS_IMETHOD ClearRecentlyRolledUp() {return NS_OK;}
+  NS_IMETHOD RecentlyRolledUp(nsIMenuFrame *aMenuFrame, PRBool *aJustRolledUp) {*aJustRolledUp = PR_FALSE; return NS_OK;}
   NS_IMETHOD SetIsContextMenu(PRBool aIsContextMenu) { mIsContextMenu = aIsContextMenu; return NS_OK; };
   NS_IMETHOD GetIsContextMenu(PRBool& aIsContextMenu) { aIsContextMenu = mIsContextMenu; return NS_OK; };