Bug 1266963, stop propagation before other steps, r=masayuki
authorNeil Deakin <neil@mozilla.com>
Wed, 04 May 2016 10:07:45 -0400 (2016-05-04)
changeset 296136 1d23af51e88680888da7ba6709c04130a977718f
parent 296135 4def9166eb5b7f17d382a1e9a3cd5f34c8074aa1
child 296137 fa8d527717fa2f01edaf958789a4d683b976dd90
push id30233
push userryanvm@gmail.com
push dateThu, 05 May 2016 18:57:26 +0000 (2016-05-05)
treeherdermozilla-central@0177462aac74 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmasayuki
bugs1266963
milestone49.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1266963, stop propagation before other steps, r=masayuki
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",