Bug 392314, alt key should close menu, add some key closing menus tests, r+sr=bz,a=dbaron

layout/xul/base/src/nsMenuBarFrame.cpp

@@ -255,6 +255,8 @@ nsMenuBarFrame::FindMenuWithShortcut(nsIDOMKeyEvent* aKeyEvent)
 {
   PRUint32 charCode;
   aKeyEvent->GetCharCode(&charCode);
+  if (!charCode) // no character was pressed so just return  
+    return nsnull;
 
   // Enumerate over our list of frames.
   nsIFrame* immediateParent = nsnull;

layout/xul/base/src/nsXULPopupManager.cpp

@@ -1604,8 +1604,10 @@ nsXULPopupManager::IsValidMenuItem(nsPresContext* aPresContext,
 nsresult
 nsXULPopupManager::KeyUp(nsIDOMEvent* aKeyEvent)
 {
-  aKeyEvent->StopPropagation();
-  aKeyEvent->PreventDefault();
+  if (mCurrentMenu) {
+    aKeyEvent->StopPropagation();
+    aKeyEvent->PreventDefault();
+  }
 
   return NS_OK; // I am consuming event
 }
@@ -1613,6 +1615,10 @@ nsXULPopupManager::KeyUp(nsIDOMEvent* aKeyEvent)
 nsresult
 nsXULPopupManager::KeyDown(nsIDOMEvent* aKeyEvent)
 {
+  // don't do anything if a menu isn't open
+  if (!mCurrentMenu)
+    return NS_OK;
+
   PRInt32 menuAccessKey = -1;
 
   // If the key just pressed is the access key (usually Alt),
@@ -1726,8 +1732,12 @@ nsXULPopupManager::KeyPress(nsIDOMEvent* aKeyEvent)
     HandleShortcutNavigation(keyEvent, nsnull);
   }
 
-  aKeyEvent->StopPropagation();
-  aKeyEvent->PreventDefault();
+  if (mCurrentMenu) {
+    // if a menu is open, it consumes the key event
+    aKeyEvent->StopPropagation();
+    aKeyEvent->PreventDefault();
+  }
+
   return NS_OK; // I am consuming event
 }
 

toolkit/content/tests/widgets/window_menubar.xul

@@ -1,8 +1,11 @@
 <?xml version="1.0"?>
 <?xml-stylesheet href="chrome://global/skin" type="text/css"?>
 
+<!-- the condition in the focus event handler is because pressing Tab
+     unfocuses and refocuses the window on Windows -->
+
 <window title="Popup Tests"
-  onfocus="runTests()"
+  onfocus="if(gTestIndex == -1) runTests()"
   xmlns:html="http://www.w3.org/1999/xhtml"
   xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul">
 
@@ -492,6 +495,49 @@ var popupTests = [
     checkClosed("editmenu", testname);
   }
 },
+{
+  testname: "F10 to activate menubar for tab deactivation",
+  events: [ "DOMMenuBarActive menubar", "DOMMenuItemActive filemenu" ],
+  test: function() { synthesizeKey("VK_F10", { }); },
+},
+{
+  testname: "Deactivate menubar with tab key",
+  events: [ "DOMMenuBarInactive menubar", "DOMMenuItemInactive filemenu" ],
+  test: function() { synthesizeKey("VK_TAB", { }); },
+},
+{
+  testname: "F10 to activate menubar for escape deactivation",
+  events: [ "DOMMenuBarActive menubar", "DOMMenuItemActive filemenu" ],
+  test: function() { synthesizeKey("VK_F10", { }); },
+},
+{
+  testname: "Deactivate menubar with escape key",
+  events: [ "DOMMenuBarInactive menubar", "DOMMenuItemInactive filemenu" ],
+  test: function() { synthesizeKey("VK_ESCAPE", { }); },
+},
+{
+  testname: "F10 to activate menubar for f10 deactivation",
+  events: [ "DOMMenuBarActive menubar", "DOMMenuItemActive filemenu" ],
+  test: function() { synthesizeKey("VK_F10", { }); },
+},
+{
+  testname: "Deactivate menubar with f10 key",
+  events: [ "DOMMenuBarInactive menubar", "DOMMenuItemInactive filemenu"  ],
+  test: function() { synthesizeKey("VK_F10", { }); },
+},
+{
+  testname: "F10 to activate menubar for alt deactivation",
+  condition: function() { return (navigator.platform.indexOf("Win") == 0) },
+  events: [ "DOMMenuBarActive menubar", "DOMMenuItemActive filemenu" ],
+  test: function() { synthesizeKey("VK_F10", { }); },
+},
+{
+  testname: "Deactivate menubar with alt key",
+  condition: function() { return (navigator.platform.indexOf("Win") == 0) },
+  events: [ "DOMMenuBarInactive menubar", "DOMMenuItemInactive filemenu"  ],
+  test: function() { synthesizeKey("VK_ALT", { }); },
+}
+
 ];
 
 ]]>