Bug 501496 part.10 nsXULPopupManager should handle keydown events for non-printable keys r=enndeakin

layout/xul/base/public/nsXULPopupManager.h

@@ -592,6 +592,13 @@ public:
     return HandleKeyboardNavigationInPopup(nullptr, aFrame, aDir);
   }
 
+  /**
+   * Handles the keyboard event with keyCode value. Returns true if the event
+   * has been handled.
+   */
+  bool HandleKeyboardEventWithKeyCode(nsIDOMKeyEvent* aKeyEvent,
+                                      nsMenuChainItem* aTopVisibleMenuItem);
+
   nsresult KeyUp(nsIDOMKeyEvent* aKeyEvent);
   nsresult KeyDown(nsIDOMKeyEvent* aKeyEvent);
   nsresult KeyPress(nsIDOMKeyEvent* aKeyEvent);

layout/xul/base/src/nsXULPopupManager.cpp

@@ -1950,6 +1950,90 @@ nsXULPopupManager::HandleKeyboardNavigationInPopup(nsMenuChainItem* item,
   return false;
 }
 
+bool
+nsXULPopupManager::HandleKeyboardEventWithKeyCode(
+                        nsIDOMKeyEvent* aKeyEvent,
+                        nsMenuChainItem* aTopVisibleMenuItem)
+{
+  uint32_t keyCode;
+  aKeyEvent->GetKeyCode(&keyCode);
+
+  // Escape should close panels, but the other keys should have no effect.
+  if (aTopVisibleMenuItem &&
+      aTopVisibleMenuItem->PopupType() != ePopupTypeMenu) {
+    if (keyCode == nsIDOMKeyEvent::DOM_VK_ESCAPE) {
+      HidePopup(aTopVisibleMenuItem->Content(), false, false, false);
+      aKeyEvent->StopPropagation();
+      aKeyEvent->PreventDefault();
+    }
+    return true;
+  }
+
+  bool consume = (mPopups || mActiveMenuBar);
+  switch (keyCode) {
+    case nsIDOMKeyEvent::DOM_VK_LEFT:
+    case nsIDOMKeyEvent::DOM_VK_RIGHT:
+    case nsIDOMKeyEvent::DOM_VK_UP:
+    case nsIDOMKeyEvent::DOM_VK_DOWN:
+    case nsIDOMKeyEvent::DOM_VK_HOME:
+    case nsIDOMKeyEvent::DOM_VK_END:
+      HandleKeyboardNavigation(keyCode);
+      break;
+
+    case nsIDOMKeyEvent::DOM_VK_ESCAPE:
+      // Pressing Escape hides one level of menus only. If no menu is open,
+      // check if a menubar is active and inform it that a menu closed. Even
+      // though in this latter case, a menu didn't actually close, the effect
+      // ends up being the same. Similar for the tab key below.
+      if (aTopVisibleMenuItem) {
+        HidePopup(aTopVisibleMenuItem->Content(), false, false, false);
+      } else if (mActiveMenuBar) {
+        mActiveMenuBar->MenuClosed();
+      }
+      break;
+
+    case nsIDOMKeyEvent::DOM_VK_TAB:
+#ifndef XP_MACOSX
+    case nsIDOMKeyEvent::DOM_VK_F10:
+#endif
+      // close popups or deactivate menubar when Tab or F10 are pressed
+      if (aTopVisibleMenuItem) {
+        Rollup(0, nullptr);
+      } else if (mActiveMenuBar) {
+        mActiveMenuBar->MenuClosed();
+      }
+      break;
+
+    case nsIDOMKeyEvent::DOM_VK_ENTER:
+    case nsIDOMKeyEvent::DOM_VK_RETURN: {
+      // If there is a popup open, check if the current item needs to be opened.
+      // Otherwise, tell the active menubar, if any, to activate the menu. The
+      // Enter method will return a menu if one needs to be opened as a result.
+      nsMenuFrame* menuToOpen = nullptr;
+      nsGUIEvent* GUIEvent = DOMKeyEventToGUIEvent(aKeyEvent);
+      if (aTopVisibleMenuItem) {
+        menuToOpen = aTopVisibleMenuItem->Frame()->Enter(GUIEvent);
+      } else if (mActiveMenuBar) {
+        menuToOpen = mActiveMenuBar->Enter(GUIEvent);
+      }
+      if (menuToOpen) {
+        nsCOMPtr<nsIContent> content = menuToOpen->GetContent();
+        ShowMenu(content, true, false);
+      }
+      break;
+    }
+
+    default:
+      return false;
+  }
+
+  if (consume) {
+    aKeyEvent->StopPropagation();
+    aKeyEvent->PreventDefault();
+  }
+  return true;
+}
+
 nsMenuFrame*
 nsXULPopupManager::GetNextMenuItem(nsIFrame* aParent,
                                    nsMenuFrame* aStart,
@@ -2067,6 +2151,13 @@ nsXULPopupManager::HandleEvent(nsIDOMEvent* aEvent)
   nsCOMPtr<nsIDOMKeyEvent> keyEvent = do_QueryInterface(aEvent);
   NS_ENSURE_TRUE(keyEvent, NS_ERROR_UNEXPECTED);
 
+  //handlers shouldn't be triggered by non-trusted events.
+  bool trustedEvent = false;
+  aEvent->GetIsTrusted(&trustedEvent);
+  if (!trustedEvent) {
+    return NS_OK;
+  }
+
   nsAutoString eventType;
   keyEvent->GetType(eventType);
   if (eventType.EqualsLiteral("keyup")) {
@@ -2107,6 +2198,10 @@ nsXULPopupManager::KeyDown(nsIDOMKeyEvent* aKeyEvent)
   if (item && item->Frame()->IsMenuLocked())
     return NS_OK;
 
+  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;
@@ -2142,104 +2237,32 @@ nsXULPopupManager::KeyDown(nsIDOMKeyEvent* aKeyEvent)
         else if (mActiveMenuBar)
           mActiveMenuBar->MenuClosed();
       }
+      aKeyEvent->PreventDefault();
     }
   }
 
-  // Since a menu was open, eat the event to keep other event
+  // Since a menu was open, stop propagation of the event to keep other event
   // listeners from becoming confused.
   aKeyEvent->StopPropagation();
-  aKeyEvent->PreventDefault();
-  return NS_OK; // I am consuming event
+  return NS_OK;
 }
 
 nsresult
 nsXULPopupManager::KeyPress(nsIDOMKeyEvent* aKeyEvent)
 {
   // Don't check prevent default flag -- menus always get first shot at key events.
-  // When a menu is open, the prevent default flag on a keypress is always set, so
-  // that no one else uses the key event.
 
   nsMenuChainItem* item = GetTopVisibleMenu();
-  if (item && item->Frame()->IsMenuLocked())
+  if (item &&
+      (item->Frame()->IsMenuLocked() || item->PopupType() != ePopupTypeMenu)) {
     return NS_OK;
-
-  //handlers shouldn't be triggered by non-trusted events.
-  bool trustedEvent = false;
-  if (aKeyEvent) {
-    aKeyEvent->GetIsTrusted(&trustedEvent);
   }
 
-  if (!trustedEvent)
-    return NS_OK;
-
   nsCOMPtr<nsIDOMKeyEvent> keyEvent = do_QueryInterface(aKeyEvent);
   NS_ENSURE_TRUE(keyEvent, NS_ERROR_UNEXPECTED);
-  uint32_t theChar;
-  keyEvent->GetKeyCode(&theChar);
-
-  // Escape should close panels, but the other keys should have no effect.
-  if (item && item->PopupType() != ePopupTypeMenu) {
-    if (theChar == NS_VK_ESCAPE) {
-      HidePopup(item->Content(), false, false, false);
-      aKeyEvent->StopPropagation();
-      aKeyEvent->PreventDefault();
-    }
-    return NS_OK;
-  }
-
   // if a menu is open or a menubar is active, it consumes the key event
   bool consume = (mPopups || mActiveMenuBar);
-
-  if (theChar == NS_VK_LEFT ||
-      theChar == NS_VK_RIGHT ||
-      theChar == NS_VK_UP ||
-      theChar == NS_VK_DOWN ||
-      theChar == NS_VK_HOME ||
-      theChar == NS_VK_END) {
-    HandleKeyboardNavigation(theChar);
-  }
-  else if (theChar == NS_VK_ESCAPE) {
-    // Pressing Escape hides one level of menus only. If no menu is open,
-    // check if a menubar is active and inform it that a menu closed. Even
-    // though in this latter case, a menu didn't actually close, the effect
-    // ends up being the same. Similar for the tab key below.
-    if (item)
-      HidePopup(item->Content(), false, false, false);
-    else if (mActiveMenuBar)
-      mActiveMenuBar->MenuClosed();
-  }
-  else if (theChar == NS_VK_TAB
-#ifndef XP_MACOSX
-           || theChar == NS_VK_F10
-#endif
-  ) {
-    // close popups or deactivate menubar when Tab or F10 are pressed
-    if (item)
-      Rollup(0, nullptr);
-    else if (mActiveMenuBar)
-      mActiveMenuBar->MenuClosed();
-  }
-  else if (theChar == NS_VK_ENTER ||
-           theChar == NS_VK_RETURN) {
-    // If there is a popup open, check if the current item needs to be opened.
-    // Otherwise, tell the active menubar, if any, to activate the menu. The
-    // Enter method will return a menu if one needs to be opened as a result.
-    nsMenuFrame* menuToOpen = nullptr;
-    nsMenuChainItem* item = GetTopVisibleMenu();
-    nsGUIEvent* evt = DOMKeyEventToGUIEvent(aKeyEvent);
-    if (item)
-      menuToOpen = item->Frame()->Enter(evt);
-    else if (mActiveMenuBar)
-      menuToOpen = mActiveMenuBar->Enter(evt);
-    if (menuToOpen) {
-      nsCOMPtr<nsIContent> content = menuToOpen->GetContent();
-      ShowMenu(content, true, false);
-    }
-  }
-  else {
-    HandleShortcutNavigation(keyEvent, nullptr);
-  }
-
+  HandleShortcutNavigation(keyEvent, nullptr);
   if (consume) {
     aKeyEvent->StopPropagation();
     aKeyEvent->PreventDefault();