Bug 1266963 - Stop propagation before other steps. r=masayuki, a=abillings
authorNeil Deakin <neil@mozilla.com>
Wed, 04 May 2016 10:07:45 -0400
changeset 333091 0636ef46d7aad9ee5ad0882088586b47588b3199
parent 333090 31d3b32e75623bce05d0e9cacf9d1ae887bcbe77
child 333092 ccf0f53adb66c86d81a39b53e32092489fb5f0b5
push id6048
push userkmoir@mozilla.com
push dateMon, 06 Jun 2016 19:02:08 +0000
treeherdermozilla-beta@46d72a56c57d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmasayuki, abillings
bugs1266963
milestone48.0a2
Bug 1266963 - Stop propagation before other steps. r=masayuki, a=abillings
layout/xul/nsXULPopupManager.cpp
toolkit/content/tests/widgets/window_menubar.xul
--- a/layout/xul/nsXULPopupManager.cpp
+++ b/layout/xul/nsXULPopupManager.cpp
@@ -2561,16 +2561,22 @@ nsXULPopupManager::KeyDown(nsIDOMKeyEven
   if (HandleKeyboardEventWithKeyCode(aKeyEvent, item)) {
     return NS_OK;
   }
 
   // don't do anything if a menu isn't open or a menubar isn't active
   if (!mActiveMenuBar && (!item || item->PopupType() != ePopupTypeMenu))
     return NS_OK;
 
+  // Since a menu was open, stop propagation of the event to keep other event
+  // listeners from becoming confused.
+  if (!item || item->IgnoreKeys() != eIgnoreKeys_Handled) {
+    aKeyEvent->AsEvent()->StopPropagation();
+  }
+
   int32_t menuAccessKey = -1;
 
   // If the key just pressed is the access key (usually Alt),
   // dismiss and unfocus the menu.
 
   nsMenuBarListener::GetMenuAccessKey(&menuAccessKey);
   if (menuAccessKey) {
     uint32_t theChar;
@@ -2591,29 +2597,25 @@ nsXULPopupManager::KeyDown(nsIDOMKeyEven
         aKeyEvent->GetMetaKey(&meta);
       if (!(ctrl || alt || shift || meta)) {
         // The access key just went down and no other
         // modifiers are already down.
         if (mPopups)
           Rollup(0, false, nullptr, nullptr);
         else if (mActiveMenuBar)
           mActiveMenuBar->MenuClosed();
+
+        // Clear the item to avoid bugs as it may have been deleted during rollup.
+        item = nullptr; 
       }
       aKeyEvent->AsEvent()->StopPropagation();
       aKeyEvent->AsEvent()->PreventDefault();
     }
   }
 
-  // Since a menu was open, stop propagation of the event to keep other event
-  // listeners from becoming confused.
-
-  if (!item || item->IgnoreKeys() != eIgnoreKeys_Handled) {
-    aKeyEvent->AsEvent()->StopPropagation();
-  }
-
   aKeyEvent->AsEvent()->StopCrossProcessForwarding();
   return NS_OK;
 }
 
 nsresult
 nsXULPopupManager::KeyPress(nsIDOMKeyEvent* aKeyEvent)
 {
   // Don't check prevent default flag -- menus always get first shot at key events.
--- a/toolkit/content/tests/widgets/window_menubar.xul
+++ b/toolkit/content/tests/widgets/window_menubar.xul
@@ -701,16 +701,40 @@ var popupTests = [
     synthesizeKey("VK_ALT", { type: "keydown" });
     synthesizeKey("VK_ALT", { type: "keyup" });
   },
   result: function (testname) {
     checkActive(document.getElementById("menubar"), "", testname);
   }
 },
 
+{
+  testname: "Open menu and press alt key by itself - open menu",
+  events: [ "DOMMenuBarActive menubar",
+            "popupshowing filepopup", "DOMMenuItemActive filemenu",
+            "DOMMenuItemActive item1", "popupshown filepopup" ],
+  test: function() { synthesizeKey("F", { altKey: true }); },
+  result: function (testname) {
+    checkOpen("filemenu", testname);
+  }
+},
+{
+  testname: "Open menu and press alt key by itself - close menu",
+  events: [ "popuphiding filepopup", "popuphidden filepopup",
+            "DOMMenuItemInactive item1", "DOMMenuInactive filepopup",
+            "DOMMenuBarInactive menubar", "DOMMenuItemInactive filemenu",
+            "DOMMenuItemInactive filemenu" ],
+  test: function() {
+    synthesizeKey("VK_ALT", { });
+  },
+  result: function (testname) {
+    checkClosed("filemenu", testname);
+  }
+},
+
 // Fllowing 4 tests are a test of bug 616797, don't insert any new tests
 // between them.
 {
   testname: "Open file menu by accelerator",
   condition: function() { return (navigator.platform.indexOf("Win") == 0) },
   events: function() {
     return [ "DOMMenuBarActive menubar", "popupshowing filepopup",
              "DOMMenuItemActive filemenu", "DOMMenuItemActive item1",