Bug 392652, nsXULPopupManager::Rollup should hide popups synchronously, r+sr=bz, a=mconnor
authorenndeakin@sympatico.ca
Tue, 18 Sep 2007 08:00:43 -0700
changeset 6051 fa0615afc29b8c13c42c5a43e0726d229075425e
parent 6050 7ce641c5c4adbb23fd6a985dd6c6ac220d9b0c5b
child 6052 6f3243c0eebf4fde104d8be1b367197710f3e712
push id1
push userroot
push dateMon, 20 Oct 2014 17:29:22 +0000
reviewersmconnor
bugs392652
milestone1.9a8pre
Bug 392652, nsXULPopupManager::Rollup should hide popups synchronously, r+sr=bz, a=mconnor
layout/xul/base/public/nsXULPopupManager.h
layout/xul/base/src/nsMenuBarFrame.cpp
layout/xul/base/src/nsMenuFrame.cpp
layout/xul/base/src/nsXULPopupManager.cpp
toolkit/content/tests/widgets/popup_trigger.js
toolkit/content/tests/widgets/window_menubar.xul
--- a/layout/xul/base/public/nsXULPopupManager.h
+++ b/layout/xul/base/public/nsXULPopupManager.h
@@ -74,17 +74,18 @@ class nsMenuFrame;
 class nsMenuPopupFrame;
 class nsMenuBarFrame;
 class nsIMenuParent;
 class nsIDOMKeyEvent;
 
 enum nsPopupType {
   ePopupTypePanel,
   ePopupTypeMenu,
-  ePopupTypeTooltip
+  ePopupTypeTooltip,
+  ePopupTypeAny = 0xF000 // used only to pass to GetTopPopup
 };
 
 // when a menu command is executed, the closemenu attribute may be used
 // to define how the menu should be closed up
 enum CloseMenuMode {
   CloseMenuMode_Auto, // close up the chain of menus, default value
   CloseMenuMode_None, // don't close up any menus
   CloseMenuMode_Single // close up only the menu the command is inside
@@ -478,17 +479,18 @@ public:
 
   /**
    * Return true if the popup for the supplied menu parent is open.
    */
   PRBool IsPopupOpenForMenuParent(nsIMenuParent* aMenuParent);
 
   /**
    * Return the frame for the topmost open popup of a given type, or null if
-   * no popup of that type is open.
+   * no popup of that type is open. If aType is ePopupTypeAny, a menu of any
+   * type is returned, except for popups in the mPanels list.
    */
   nsIFrame* GetTopPopup(nsPopupType aType);
 
   /**
    * Return an array of all the open popup frames for menus, in order from
    * top to bottom.
    */
   nsTArray<nsIFrame *> GetOpenPopups();
--- a/layout/xul/base/src/nsMenuBarFrame.cpp
+++ b/layout/xul/base/src/nsMenuBarFrame.cpp
@@ -293,18 +293,21 @@ nsMenuBarFrame::FindMenuWithShortcut(nsI
   // behavior on Windows - this item is on the menu bar, beep and deactivate the menu bar
   if (mIsActive) {
     nsCOMPtr<nsISound> soundInterface = do_CreateInstance("@mozilla.org/sound;1");
     if (soundInterface)
       soundInterface->Beep();
   }
 
   nsXULPopupManager* pm = nsXULPopupManager::GetInstance();
-  if (pm)
-    pm->Rollup();
+  if (pm) {
+    nsIFrame* popup = pm->GetTopPopup(ePopupTypeAny);
+    if (popup)
+      pm->HidePopup(popup->GetContent(), PR_TRUE, PR_TRUE, PR_TRUE);
+  }
 
   SetCurrentMenuItem(nsnull);
   SetActive(PR_FALSE);
 
 #endif  // #ifdef XP_WIN
 
   return nsnull;
 }
--- a/layout/xul/base/src/nsMenuFrame.cpp
+++ b/layout/xul/base/src/nsMenuFrame.cpp
@@ -819,18 +819,21 @@ nsMenuFrame::SetDebug(nsBoxLayoutState& 
 nsMenuFrame*
 nsMenuFrame::Enter()
 {
   if (IsDisabled()) {
 #ifdef XP_WIN
     // behavior on Windows - close the popup chain
     if (mMenuParent) {
       nsXULPopupManager* pm = nsXULPopupManager::GetInstance();
-      if (pm)
-        pm->Rollup();
+      if (pm) {
+        nsIFrame* popup = pm->GetTopPopup(ePopupTypeAny);
+        if (popup)
+          pm->HidePopup(popup->GetContent(), PR_TRUE, PR_TRUE, PR_TRUE);
+      }
     }
 #endif   // #ifdef XP_WIN
     // this menu item was disabled - exit
     return nsnull;
   }
 
   if (!IsOpen()) {
     // The enter key press applies to us.
--- a/layout/xul/base/src/nsXULPopupManager.cpp
+++ b/layout/xul/base/src/nsXULPopupManager.cpp
@@ -155,18 +155,19 @@ nsXULPopupManager*
 nsXULPopupManager::GetInstance()
 {
   return sInstance;
 }
 
 NS_IMETHODIMP
 nsXULPopupManager::Rollup()
 {
-  if (mCurrentMenu)
-    HidePopup(mCurrentMenu->Content(), PR_TRUE, PR_TRUE, PR_TRUE);
+  nsMenuChainItem* item = GetTopVisibleMenu();
+  if (item)
+    HidePopup(item->Content(), PR_TRUE, PR_TRUE, PR_FALSE);
   return NS_OK;
 }
 
 ////////////////////////////////////////////////////////////////////////
 NS_IMETHODIMP nsXULPopupManager::ShouldRollupOnMouseWheelEvent(PRBool *aShouldRollup) 
 {
   // should rollup only for autocomplete widgets
   // XXXndeakin this should really be something the popup has more control over
@@ -979,17 +980,17 @@ nsXULPopupManager::IsPopupOpenForMenuPar
 nsIFrame*
 nsXULPopupManager::GetTopPopup(nsPopupType aType)
 {
   if (aType == ePopupTypePanel && mPanels)
     return mPanels->Frame();
 
   nsMenuChainItem* item = GetTopVisibleMenu();
   while (item) {
-    if (item->PopupType() == aType)
+    if (item->PopupType() == aType || aType == ePopupTypeAny)
       return item->Frame();
     item = item->GetParent();
   }
 
   return nsnull;
 }
 
 nsTArray<nsIFrame *>
@@ -1861,12 +1862,12 @@ nsXULMenuCommandEvent::Run()
     commandEvent.isShift = mShift;
     commandEvent.isControl = mControl;
     commandEvent.isAlt = mAlt;
     commandEvent.isMeta = mMeta;
     shell->HandleDOMEventWithTarget(mMenu, &commandEvent, &status);
   }
 
   if (popup && mCloseMenuMode != CloseMenuMode_None)
-    pm->HidePopup(popup, mCloseMenuMode == CloseMenuMode_Auto, PR_TRUE, PR_TRUE);
+    pm->HidePopup(popup, mCloseMenuMode == CloseMenuMode_Auto, PR_TRUE, PR_FALSE);
 
   return NS_OK;
 }
--- a/toolkit/content/tests/widgets/popup_trigger.js
+++ b/toolkit/content/tests/widgets/popup_trigger.js
@@ -246,18 +246,19 @@ var popupTests = [
   test: function(testname, step) { gMenuPopup.openPopup(null, "after_start", 6, 8, false); },
   result: function(testname, step) {
     var rect = gMenuPopup.getBoundingClientRect();
     ok(rect.left == 6 && rect.top == 8 && rect.right && rect.bottom, testname);
   }
 },
 {
   testname: "activate menuitem with mouse",
-  events: [ "DOMMenuInactive thepopup", "command item3", "DOMMenuItemInactive item3",
-            "popuphiding thepopup", "popuphidden thepopup" ],
+  events: [ "DOMMenuInactive thepopup", "command item3",
+            "popuphiding thepopup", "popuphidden thepopup",
+            "DOMMenuItemInactive item3" ],
   test: function(testname, step) {
     var item3 = document.getElementById("item3");
     synthesizeMouse(item3, 4, 4, { });
   },
   result: function(testname, step) { checkClosed("trigger", testname); }
 },
 {
   testname: "close popup",
@@ -279,18 +280,18 @@ var popupTests = [
 },
 {
   // check that pressing a menuitem's accelerator selects it. Note that
   // the menuitem with the M accesskey overrides the earlier menuitem that
   // begins with M.
   testname: "menuitem accelerator",
   events: [ "DOMMenuItemActive amenu", "DOMMenuItemInactive amenu",
             "DOMMenuInactive thepopup",
-            "command amenu", "DOMMenuItemInactive amenu",
-            "popuphiding thepopup", "popuphidden thepopup",
+            "command amenu", "popuphiding thepopup", "popuphidden thepopup",
+            "DOMMenuItemInactive amenu"
            ],
   test: function() { synthesizeKey("M", { }); },
   result: function(testname) { checkClosed("trigger", testname); }
 },
 {
   testname: "open context popup at screen",
   events: [ "popupshowing thepopup", "popupshown thepopup" ],
   test: function(testname, step) {
@@ -409,18 +410,18 @@ var popupTests = [
   }
 },
 {
   // when only one menuitem starting with that letter exists, it should be
   // selected and the menu closed
   testname: "menuitem with non accelerator single",
   events: [ "DOMMenuItemInactive item1", "DOMMenuItemActive amenu",
             "DOMMenuItemInactive amenu", "DOMMenuInactive thepopup",
-            "command amenu", "DOMMenuItemInactive amenu",
-            "popuphiding thepopup", "popuphidden thepopup",
+            "command amenu", "popuphiding thepopup", "popuphidden thepopup",
+            "DOMMenuItemInactive amenu",
            ],
   test: function() { synthesizeKey("M", { }); },
   result: function(testname) {
     checkClosed("trigger", testname);
     checkActive(gMenuPopup, "", testname);
   }
 },
 {
@@ -428,17 +429,17 @@ var popupTests = [
   events: [ "popupshowing thepopup", "popupshown thepopup" ],
   test: function(testname, step) { openMenu(gTrigger); },
   result: function(testname, step) {
     checkOpen("trigger", testname);
     if (gIsMenu)
       compareEdge(gTrigger, gMenuPopup, "after_start", 0, 0, testname);
   }
 },
-{ end: true,
+{end: true,
   testname: "open submenu with open property",
   events: [ "popupshowing submenupopup", "DOMMenuItemActive submenu",
             "popupshown submenupopup" ],
   test: function(testname, step) { openMenu(document.getElementById("submenu")); },
   result: function(testname, step) {
     checkOpen("trigger", testname);
     checkOpen("submenu", testname);
     // XXXndeakin
@@ -548,18 +549,18 @@ var popupTests = [
     checkActive(gMenuPopup, "", testname);
   }
 },
 {
   testname: "select and enter on menuitem",
   condition: function() { return gIsMenu; },
   events: [ "DOMMenuItemActive item1", "DOMMenuItemInactive item1",
             "DOMMenuInactive thepopup", "command item1",
-            "DOMMenuItemInactive item1",
-            "popuphiding thepopup", "popuphidden thepopup" ],
+            "popuphiding thepopup", "popuphidden thepopup",
+            "DOMMenuItemInactive item1" ],
   test: function(testname, step) {
     synthesizeKey("VK_DOWN", { });
     synthesizeKey("VK_ENTER", { });
   },
   result: function(testname, step) { checkClosed("trigger", testname); }
 },
 {
   testname: "focus trigger and key to open",
--- a/toolkit/content/tests/widgets/window_menubar.xul
+++ b/toolkit/content/tests/widgets/window_menubar.xul
@@ -185,18 +185,18 @@ var popupTests = [
       return [ "popuphiding editpopup", "popuphidden editpopup",
                "DOMMenuItemInactive cut", "DOMMenuInactive editpopup",
                "DOMMenuBarInactive menubar",
                "DOMMenuItemInactive editmenu", "DOMMenuItemInactive editmenu" ];
     else
       return [ "DOMMenuItemInactive copy", "DOMMenuInactive editpopup",
                "DOMMenuBarInactive menubar",
                "DOMMenuItemInactive editmenu", "DOMMenuItemInactive editmenu",
-               "command copy", "DOMMenuItemInactive copy",
-               "popuphiding editpopup", "popuphidden editpopup" ];
+               "command copy", "popuphiding editpopup", "popuphidden editpopup",
+               "DOMMenuItemInactive copy" ];
   },
   test: function() { synthesizeKey("VK_ENTER", { }); },
   result: function(testname) { checkClosed("editmenu", testname); }
 },
 {
   // pressing Alt + a key should open the corresponding menu
   testname: "open with accelerator",
   events: function() {
@@ -353,18 +353,18 @@ var popupTests = [
 },
 {
   // check that pressing a menuitem's accelerator selects it
   testname: "menuitem accelerator",
   events: [ "DOMMenuItemInactive contents", "DOMMenuItemActive amenu",
             "DOMMenuItemInactive amenu", "DOMMenuInactive helppopup",
             "DOMMenuBarInactive menubar", "DOMMenuItemInactive helpmenu",
             "DOMMenuItemInactive helpmenu",
-            "command amenu", "DOMMenuItemInactive amenu",
-            "popuphiding helppopup", "popuphidden helppopup",
+            "command amenu", "popuphiding helppopup", "popuphidden helppopup",
+            "DOMMenuItemInactive amenu",
            ],
   test: function() { synthesizeKey("M", { }); },
   result: function(testname) { checkClosed("helpmenu", testname); }
 },
 {
   // pressing F10 should highlight the first menu but not open it
   testname: "F10 to activate menubar",
   events: [ "DOMMenuBarActive menubar", "DOMMenuItemActive filemenu" ],
@@ -418,18 +418,18 @@ var popupTests = [
   // selected and the menu closed
   testname: "menuitem with no accelerator single",
   events: function() {
     var elist = [ "DOMMenuItemInactive other", "DOMMenuItemActive third",
                   "DOMMenuItemInactive third", "DOMMenuInactive helppopup",
                   "DOMMenuBarInactive menubar",
                   "DOMMenuItemInactive helpmenu",
                   "DOMMenuItemInactive helpmenu",
-                  "command third", "DOMMenuItemInactive third",
-                  "popuphiding helppopup", "popuphidden helppopup",
+                  "command third", "popuphiding helppopup",
+                  "popuphidden helppopup", "DOMMenuItemInactive third",
                 ];
     if (navigator.platform.indexOf("Win") == -1)
       elist[0] = "DOMMenuItemInactive only";
     return elist;
   },
   test: function() { synthesizeKey("T", { }); },
   result: function(testname) { checkClosed("helpmenu", testname); }
 },
@@ -480,18 +480,18 @@ var popupTests = [
   }
 },
 {
   testname: "press on menuitem",
   events: [ "DOMMenuInactive editpopup",
             "DOMMenuBarInactive menubar",
             "DOMMenuItemInactive editmenu",
             "DOMMenuItemInactive editmenu",
-            "command copy", "DOMMenuItemInactive copy",
-            "popuphiding editpopup", "popuphidden editpopup",
+            "command copy", "popuphiding editpopup", "popuphidden editpopup",
+            "DOMMenuItemInactive copy",
            ],
   test: function() {
     synthesizeMouse(document.getElementById("copy"), 8, 8, { });
   },
   result: function (testname) {
     checkClosed("editmenu", testname);
   }
 },