Commit 155453c3 authored by Colomban Wendling's avatar Colomban Wendling
Browse files

Bug 1166825, do not focus the first item in the menu when open with F10,... 

Bug 1166825, do not focus the first item in the menu when open with F10, helped by Samuel Thibault, r=enndeakin

This is consistent with other apps and toolkits (GTK, LibreOffice), and
gives blind people a better experience by providing a consistent
desktop experience, and consistent behavior of the right arrow key
regardless of the first focusable element.

---
 layout/xul/nsMenuBarListener.cpp                 |    2 +-
 layout/xul/nsXULPopupManager.cpp                 |   17 ++++++++++++++++-
 toolkit/content/tests/widgets/window_menubar.xul |   14 ++++++++------
 3 files changed, 25 insertions(+), 8 deletions(-)
parent fde62512

layout/xul/nsMenuBarListener.cpp

+1 −1
Original line number Diff line number Diff line
@@ -313,7 +313,7 @@ nsMenuBarListener::KeyPress(nsIDOMEvent* aKeyEvent) 
        if (mMenuBarFrame->IsActive()) {

#ifdef MOZ_WIDGET_GTK

          // In GTK, this also opens the first menu.

          mMenuBarFrame->GetCurrentMenuItem()->OpenMenu(true);

          mMenuBarFrame->GetCurrentMenuItem()->OpenMenu(false);

#endif

          aKeyEvent->StopPropagation();

          aKeyEvent->PreventDefault();

layout/xul/nsXULPopupManager.cpp

+16 −1
Original line number Diff line number Diff line
@@ -2193,6 +2193,21 @@ nsXULPopupManager::HandleKeyboardNavigation(uint32_t aKeyCode) 
                 aKeyCode <= nsIDOMKeyEvent::DOM_VK_DOWN, "Illegal key code");

  theDirection = NS_DIRECTION_FROM_KEY_CODE(itemFrame, aKeyCode);



  bool selectFirstItem = true;

#ifdef MOZ_WIDGET_GTK

  nsMenuFrame* currentItem = nullptr;

  if (item && mActiveMenuBar && NS_DIRECTION_IS_INLINE(theDirection)) {

    currentItem = item->Frame()->GetCurrentMenuItem();

    // If nothing is selected in the menu and we have a menubar, let it

    // handle the movement not to steal focus from it.

    if (!currentItem) {

      item = nullptr;

    }

  }

  // On menu change, only select first item if an item is already selected.

  selectFirstItem = currentItem != nullptr;

#endif



  // if a popup is open, first check for navigation within the popup

  if (item && HandleKeyboardNavigationInPopup(item, theDirection))

    return true;
@@ -2205,7 +2220,7 @@ nsXULPopupManager::HandleKeyboardNavigation(uint32_t aKeyCode) 
      nsMenuFrame* nextItem = (theDirection == eNavigationDirection_End) ?

                              GetNextMenuItem(mActiveMenuBar, currentMenu, false, true) :

                              GetPreviousMenuItem(mActiveMenuBar, currentMenu, false, true);

      mActiveMenuBar->ChangeMenuItem(nextItem, true, true);

      mActiveMenuBar->ChangeMenuItem(nextItem, selectFirstItem, true);

      return true;

    }

    else if (NS_DIRECTION_IS_BLOCK(theDirection)) {

toolkit/content/tests/widgets/window_menubar.xul

+8 −6
Original line number Diff line number Diff line
@@ -95,14 +95,14 @@ window.opener.SimpleTest.waitForFocus(function () { 
function pressF10Events()

{

  return navigator.platform.indexOf("Linux") >= 0 ?

    [ "DOMMenuBarActive menubar", "DOMMenuItemActive filemenu", "popupshowing filepopup", "DOMMenuItemActive item1", "popupshown filepopup"] :

    [ "DOMMenuBarActive menubar", "DOMMenuItemActive filemenu", "popupshowing filepopup", "popupshown filepopup"] :

    [ "DOMMenuBarActive menubar", "DOMMenuItemActive filemenu" ];

}



function closeAfterF10Events(extraInactive)

{

  if (navigator.platform.indexOf("Linux") >= 0) {

    var events = [ "popuphiding filepopup", "popuphidden filepopup", "DOMMenuItemInactive item1",

    var events = [ "popuphiding filepopup", "popuphidden filepopup",

                   "DOMMenuInactive filepopup", "DOMMenuBarInactive menubar",

                   "DOMMenuItemInactive filemenu" ];

    if (extraInactive)
@@ -479,9 +479,9 @@ var popupTests = [ 
               // "accelerator on active menubar" because menus opened from a

               // shortcut key are fired asynchronously

               "popuphiding filepopup", "popuphidden filepopup",

               "popupshowing helppopup", "DOMMenuItemInactive item1",

               "DOMMenuItemActive item2", "DOMMenuItemInactive item2",

               "DOMMenuInactive filepopup", "DOMMenuItemActive contents",

               "popupshowing helppopup",

               "DOMMenuItemActive item1", "DOMMenuItemInactive item1",

               "DOMMenuInactive filepopup",

               "popupshown helppopup" ] :

            [ "popupshowing helppopup", "DOMMenuItemInactive filemenu",

              "DOMMenuItemActive helpmenu",
@@ -499,7 +499,9 @@ var popupTests = [ 
  // should not close because there is more than one item corresponding to

  // that letter

  testname: "menuitem with no accelerator",

  events: [ "DOMMenuItemInactive contents", "DOMMenuItemActive one" ],

  events: (navigator.platform.indexOf("Linux") >= 0) ?

           [ "DOMMenuItemActive one" ] :

           [ "DOMMenuItemInactive contents", "DOMMenuItemActive one" ],

  test: function() { sendChar("o"); },

  result: function(testname) { checkOpen("helpmenu", testname); }

},