diff --git a/accessible/tests/mochitest/events/test_focus_listcontrols.xul b/accessible/tests/mochitest/events/test_focus_listcontrols.xul index db45048e28e1..291d8f815eff 100644 --- a/accessible/tests/mochitest/events/test_focus_listcontrols.xul +++ b/accessible/tests/mochitest/events/test_focus_listcontrols.xul @@ -52,7 +52,7 @@ gQueue.push(new synthEscapeKey("ml_marmalade", new focusChecker("menulist"))); if (!MAC) { // On Windows, items get selected during navigation. - let expectedItem = WIN ? "ml_tangerine" : "ml_marmalade"; + let expectedItem = WIN ? "ml_strawberry" : "ml_marmalade"; gQueue.push(new synthDownKey("menulist", new nofocusChecker(expectedItem))); gQueue.push(new synthOpenComboboxKey("menulist", new focusChecker(expectedItem))); gQueue.push(new synthEnterKey(expectedItem, new focusChecker("menulist"))); @@ -174,6 +174,7 @@ if (!MAC) { + diff --git a/layout/xul/nsMenuBarFrame.cpp b/layout/xul/nsMenuBarFrame.cpp index 80aa98fbd7fd..b2398a47512e 100644 --- a/layout/xul/nsMenuBarFrame.cpp +++ b/layout/xul/nsMenuBarFrame.cpp @@ -147,7 +147,7 @@ nsMenuBarFrame::ToggleMenuActiveState() // Set the active menu to be the top left item (e.g., the File menu). // We use an attribute called "menuactive" to track the current // active menu. - nsMenuFrame* firstFrame = nsXULPopupManager::GetNextMenuItem(this, nullptr, false); + nsMenuFrame* firstFrame = nsXULPopupManager::GetNextMenuItem(this, nullptr, false, false); if (firstFrame) { // Activate the menu bar SetActive(true); diff --git a/layout/xul/nsMenuFrame.cpp b/layout/xul/nsMenuFrame.cpp index ea968fab5ab4..66d3a7b347a0 100644 --- a/layout/xul/nsMenuFrame.cpp +++ b/layout/xul/nsMenuFrame.cpp @@ -1004,7 +1004,7 @@ nsMenuFrame::UpdateMenuSpecialState() // get the first sibling in this menu popup. This frame may be it, and if we're // being called at creation time, this frame isn't yet in the parent's child list. // All I'm saying is that this may fail, but it's most likely alright. - nsIFrame* firstMenuItem = nsXULPopupManager::GetNextMenuItem(GetParent(), nullptr, true); + nsIFrame* firstMenuItem = nsXULPopupManager::GetNextMenuItem(GetParent(), nullptr, true, false); nsIFrame* sib = firstMenuItem; while (sib) { nsMenuFrame* menu = do_QueryFrame(sib); @@ -1018,7 +1018,7 @@ nsMenuFrame::UpdateMenuSpecialState() return; } } - sib = nsXULPopupManager::GetNextMenuItem(GetParent(), menu, true); + sib = nsXULPopupManager::GetNextMenuItem(GetParent(), menu, true, true); if (sib == firstMenuItem) { break; } diff --git a/layout/xul/nsMenuPopupFrame.cpp b/layout/xul/nsMenuPopupFrame.cpp index 6e706e49c228..4c19bee2e955 100644 --- a/layout/xul/nsMenuPopupFrame.cpp +++ b/layout/xul/nsMenuPopupFrame.cpp @@ -1838,7 +1838,7 @@ void nsMenuPopupFrame::ChangeByPage(bool aIsUp) // just use this as the newMenu and leave currentMenu null so that no // check for a later element is performed. When moving down, set currentMenu // so that we look for one page down from the first item. - newMenu = nsXULPopupManager::GetNextMenuItem(this, nullptr, true); + newMenu = nsXULPopupManager::GetNextMenuItem(this, nullptr, true, false); if (!aIsUp) { currentMenu = newMenu; } @@ -2066,7 +2066,7 @@ nsMenuPopupFrame::FindMenuWithShortcut(nsIDOMKeyEvent* aKeyEvent, bool& doAction // been destroyed already. One strategy would be to // setTimeout(,0) as detailed in: // - nsIFrame* firstMenuItem = nsXULPopupManager::GetNextMenuItem(immediateParent, nullptr, true); + nsIFrame* firstMenuItem = nsXULPopupManager::GetNextMenuItem(immediateParent, nullptr, true, false); nsIFrame* currFrame = firstMenuItem; int32_t menuAccessKey = -1; @@ -2131,7 +2131,7 @@ nsMenuPopupFrame::FindMenuWithShortcut(nsIDOMKeyEvent* aKeyEvent, bool& doAction } nsMenuFrame* menu = do_QueryFrame(currFrame); - currFrame = nsXULPopupManager::GetNextMenuItem(immediateParent, menu, true); + currFrame = nsXULPopupManager::GetNextMenuItem(immediateParent, menu, true, true); if (currFrame == firstMenuItem) break; } diff --git a/layout/xul/nsXULPopupManager.cpp b/layout/xul/nsXULPopupManager.cpp index fe23655d7a27..a4d98f7f2052 100644 --- a/layout/xul/nsXULPopupManager.cpp +++ b/layout/xul/nsXULPopupManager.cpp @@ -945,7 +945,7 @@ nsXULPopupManager::ShowPopupCallback(nsIContent* aPopup, } if (aSelectFirstItem) { - nsMenuFrame* next = GetNextMenuItem(aPopupFrame, nullptr, true); + nsMenuFrame* next = GetNextMenuItem(aPopupFrame, nullptr, true, false); aPopupFrame->SetCurrentMenuItem(next); } @@ -2207,8 +2207,8 @@ nsXULPopupManager::HandleKeyboardNavigation(uint32_t aKeyCode) if (NS_DIRECTION_IS_INLINE(theDirection)) { nsMenuFrame* nextItem = (theDirection == eNavigationDirection_End) ? - GetNextMenuItem(mActiveMenuBar, currentMenu, false) : - GetPreviousMenuItem(mActiveMenuBar, currentMenu, false); + GetNextMenuItem(mActiveMenuBar, currentMenu, false, true) : + GetPreviousMenuItem(mActiveMenuBar, currentMenu, false, true); mActiveMenuBar->ChangeMenuItem(nextItem, true, true); return true; } @@ -2243,7 +2243,7 @@ nsXULPopupManager::HandleKeyboardNavigationInPopup(nsMenuChainItem* item, // We've been opened, but we haven't had anything selected. // We can handle End, but our parent handles Start. if (aDir == eNavigationDirection_End) { - nsMenuFrame* nextItem = GetNextMenuItem(aFrame, nullptr, true); + nsMenuFrame* nextItem = GetNextMenuItem(aFrame, nullptr, true, false); if (nextItem) { aFrame->ChangeMenuItem(nextItem, false, true); return true; @@ -2277,14 +2277,28 @@ nsXULPopupManager::HandleKeyboardNavigationInPopup(nsMenuChainItem* item, NS_DIRECTION_IS_BLOCK_TO_EDGE(aDir)) { nsMenuFrame* nextItem; - if (aDir == eNavigationDirection_Before) - nextItem = GetPreviousMenuItem(aFrame, currentMenu, true); - else if (aDir == eNavigationDirection_After) - nextItem = GetNextMenuItem(aFrame, currentMenu, true); - else if (aDir == eNavigationDirection_First) - nextItem = GetNextMenuItem(aFrame, nullptr, true); - else - nextItem = GetPreviousMenuItem(aFrame, nullptr, true); + if (aDir == eNavigationDirection_Before || + aDir == eNavigationDirection_After) { + // Cursor navigation does not wrap on Mac or for menulists on Windows. + bool wrap = +#ifdef XP_WIN + aFrame->IsMenuList() ? false : true; +#elif defined XP_MACOSX + false; +#else + true; +#endif + + if (aDir == eNavigationDirection_Before) { + nextItem = GetPreviousMenuItem(aFrame, currentMenu, true, wrap); + } else { + nextItem = GetNextMenuItem(aFrame, currentMenu, true, wrap); + } + } else if (aDir == eNavigationDirection_First) { + nextItem = GetNextMenuItem(aFrame, nullptr, true, false); + } else { + nextItem = GetPreviousMenuItem(aFrame, nullptr, true, false); + } if (nextItem) { aFrame->ChangeMenuItem(nextItem, false, true); @@ -2406,7 +2420,8 @@ nsXULPopupManager::HandleKeyboardEventWithKeyCode( nsMenuFrame* nsXULPopupManager::GetNextMenuItem(nsContainerFrame* aParent, nsMenuFrame* aStart, - bool aIsPopup) + bool aIsPopup, + bool aWrap) { nsPresContext* presContext = aParent->PresContext(); auto insertion = presContext->PresShell()-> @@ -2441,6 +2456,10 @@ nsXULPopupManager::GetNextMenuItem(nsContainerFrame* aParent, currFrame = currFrame->GetNextSibling(); } + if (!aWrap) { + return aStart; + } + currFrame = immediateParent->PrincipalChildList().FirstChild(); // Still don't have anything. Try cycling from the beginning. @@ -2467,7 +2486,8 @@ nsXULPopupManager::GetNextMenuItem(nsContainerFrame* aParent, nsMenuFrame* nsXULPopupManager::GetPreviousMenuItem(nsContainerFrame* aParent, nsMenuFrame* aStart, - bool aIsPopup) + bool aIsPopup, + bool aWrap) { nsPresContext* presContext = aParent->PresContext(); auto insertion = presContext->PresShell()-> @@ -2506,6 +2526,10 @@ nsXULPopupManager::GetPreviousMenuItem(nsContainerFrame* aParent, currFrame = currFrame->GetPrevSibling(); } + if (!aWrap) { + return aStart; + } + currFrame = frames.LastChild(); // Still don't have anything. Try cycling from the end. diff --git a/layout/xul/nsXULPopupManager.h b/layout/xul/nsXULPopupManager.h index 14d5bfef6025..b68b2d8ce1cb 100644 --- a/layout/xul/nsXULPopupManager.h +++ b/layout/xul/nsXULPopupManager.h @@ -368,12 +368,16 @@ public: // returns the item before it, while GetNextMenuItem returns the // item after it. // aIsPopup - true for menupopups, false for menubars + // aWrap - true to wrap around to the beginning and continue searching if not + // found. False to end at the beginning or end of the menu. static nsMenuFrame* GetPreviousMenuItem(nsContainerFrame* aParent, nsMenuFrame* aStart, - bool aIsPopup); + bool aIsPopup, + bool aWrap); static nsMenuFrame* GetNextMenuItem(nsContainerFrame* aParent, nsMenuFrame* aStart, - bool aIsPopup); + bool aIsPopup, + bool aWrap); // returns true if the menu item aContent is a valid menuitem which may // be navigated to. aIsPopup should be true for items on a popup, or false diff --git a/toolkit/content/tests/chrome/popup_trigger.js b/toolkit/content/tests/chrome/popup_trigger.js index db0fdf3b093f..18a1fe63affb 100644 --- a/toolkit/content/tests/chrome/popup_trigger.js +++ b/toolkit/content/tests/chrome/popup_trigger.js @@ -88,16 +88,20 @@ var popupTests = [ { // check that pressing cursor up wraps and highlights the last item testname: "cursor up wrap", - events: [ "DOMMenuItemInactive item1", "DOMMenuItemActive last" ], + events: function () { + // No wrapping on menus on Mac + return platformIsMac() ? [] : [ "DOMMenuItemInactive item1", "DOMMenuItemActive last" ] + }, test: function() { synthesizeKey("VK_UP", { }); }, result: function(testname) { - checkActive(gMenuPopup, "last", testname); + checkActive(gMenuPopup, platformIsMac() ? "item1" : "last", testname); } }, { // check that pressing cursor down wraps and highlights the first item testname: "cursor down wrap", - events: [ "DOMMenuItemInactive last", "DOMMenuItemActive item1" ], + condition: function () { return !platformIsMac() }, + events: ["DOMMenuItemInactive last", "DOMMenuItemActive item1" ], test: function() { synthesizeKey("VK_DOWN", { }); }, result: function(testname) { checkActive(gMenuPopup, "item1", testname); } }, diff --git a/toolkit/content/tests/chrome/test_menulist_keynav.xul b/toolkit/content/tests/chrome/test_menulist_keynav.xul index dc7083b10436..ec7e86b5e2b3 100644 --- a/toolkit/content/tests/chrome/test_menulist_keynav.xul +++ b/toolkit/content/tests/chrome/test_menulist_keynav.xul @@ -41,6 +41,7 @@ var list = $("list"); let expectCommandEvent; var iswin = (navigator.platform.indexOf("Win") == 0); +var ismac = (navigator.platform.indexOf("Mac") == 0); function runTests() { @@ -48,20 +49,28 @@ function runTests() // on Mac, up and cursor keys open the menu, but on other platforms, the // cursor keys navigate between items without opening the menu - if (navigator.platform.indexOf("Mac") == -1) { + if (!ismac) { expectCommandEvent = true; - keyCheck(list, "VK_DOWN", 2, "cursor down"); - keyCheck(list, "VK_DOWN", 3, "cursor down skip disabled"); - keyCheck(list, "VK_UP", 2, "cursor up skip disabled"); - keyCheck(list, "VK_UP", 1, "cursor up"); - keyCheck(list, "VK_UP", 4, "cursor up wrap"); - keyCheck(list, "VK_DOWN", 1, "cursor down wrap"); + keyCheck(list, "VK_DOWN", 2, 1, "cursor down"); + keyCheck(list, "VK_DOWN", 3, 1, "cursor down skip disabled"); + keyCheck(list, "VK_UP", 2, 1, "cursor up skip disabled"); + keyCheck(list, "VK_UP", 1, 1, "cursor up"); + + // On Windows, wrapping doesn't occur. + keyCheck(list, "VK_UP", iswin ? 1 : 4, 1, "cursor up wrap"); + + list.selectedIndex = 4; + list.menuBoxObject.activeChild = list.selectedItem; + keyCheck(list, "VK_DOWN", iswin ? 4 : 1, 4, "cursor down wrap"); + + list.selectedIndex = 0; + list.menuBoxObject.activeChild = list.selectedItem; } // check that attempting to open the menulist does not change the selection - synthesizeKey("VK_DOWN", { altKey: navigator.platform.indexOf("Mac") == -1 }); + synthesizeKey("VK_DOWN", { altKey: !ismac }); is(list.selectedItem, $("i1"), "open menulist down selectedItem"); - synthesizeKey("VK_UP", { altKey: navigator.platform.indexOf("Mac") == -1 }); + synthesizeKey("VK_UP", { altKey: !ismac }); is(list.selectedItem, $("i1"), "open menulist up selectedItem"); list.selectedItem = $("i1"); @@ -78,11 +87,11 @@ function pressLetter() synthesizeKey("G", { }); is(list.selectedItem, $("i1"), "letter pressed not found selectedItem"); - keyCheck(list, "T", 2, "letter pressed"); + keyCheck(list, "T", 2, 1, "letter pressed"); if (!gOpenPhase) { SpecialPowers.setIntPref("ui.menu.incremental_search.timeout", 0); // prevent to timeout - keyCheck(list, "T", 2, "same letter pressed"); + keyCheck(list, "T", 2, 1, "same letter pressed"); SpecialPowers.clearUserPref("ui.menu.incremental_search.timeout"); } @@ -91,16 +100,16 @@ function pressLetter() function pressedAgain() { - keyCheck(list, "T", 3, "letter pressed again"); + keyCheck(list, "T", 3, 1, "letter pressed again"); SpecialPowers.setIntPref("ui.menu.incremental_search.timeout", 0); // prevent to timeout - keyCheck(list, "W", 2, "second letter pressed"); + keyCheck(list, "W", 2, 1, "second letter pressed"); SpecialPowers.clearUserPref("ui.menu.incremental_search.timeout"); setTimeout(differentPressed, 1200); } function differentPressed() { - keyCheck(list, "O", 1, "different letter pressed"); + keyCheck(list, "O", 1, 1, "different letter pressed"); if (gOpenPhase) { list.open = false; @@ -133,7 +142,7 @@ function tabAndScroll() { list = $("list"); - if (navigator.platform.indexOf("Mac") == -1) { + if (!ismac) { $("button1").focus(); synthesizeKeyExpectEvent("VK_TAB", { }, list, "focus", "focus to menulist"); synthesizeKeyExpectEvent("VK_TAB", { }, $("button2"), "focus", "focus to button"); @@ -176,11 +185,13 @@ function tabAndScroll() checkEnter(); } -function keyCheck(list, key, index, testname) +function keyCheck(list, key, index, defaultindex, testname) { var item = $("i" + index); + var defaultitem = $("i" + defaultindex || 1); + synthesizeKeyExpectEvent(key, { }, item, expectCommandEvent ? "command" : "!command", testname); - is(list.selectedItem, expectCommandEvent ? item : $("i1"), testname + " selectedItem"); + is(list.selectedItem, expectCommandEvent ? item : defaultitem, testname + " selectedItem" + "----" + list.selectedItem.id); } function checkModifiers(event) @@ -243,6 +254,20 @@ function checkCursorNavigation() is(list.selectedIndex, iswin ? 3 : 1, "selectedIndex after page down"); is(commandEventsCount, iswin ? 2 : 0, "selectedIndex after page down command event"); is(list.menupopup.state, "open", "page down popup state"); + + // Check whether cursor up and down wraps. + list.selectedIndex = 0; + list.menuBoxObject.activeChild = list.selectedItem; + synthesizeKey("VK_UP", { }); + is(list.menuBoxObject.activeChild, + document.getElementById(iswin || ismac ? "b1" : "b4"), "cursor up wrap while open"); + + list.selectedIndex = 3; + list.menuBoxObject.activeChild = list.selectedItem; + synthesizeKey("VK_DOWN", { }); + is(list.menuBoxObject.activeChild, + document.getElementById(iswin || ismac ? "b4" : "b1"), "cursor down wrap while open"); + list.open = false; } diff --git a/toolkit/content/tests/widgets/popup_shared.js b/toolkit/content/tests/widgets/popup_shared.js index 7c95704f1c87..87052cca752a 100644 --- a/toolkit/content/tests/widgets/popup_shared.js +++ b/toolkit/content/tests/widgets/popup_shared.js @@ -247,8 +247,11 @@ function goNextStepSync() test.test(test.testname, step); // no events to check for so just check the result - if (!("events" in test)) + if (!("events" in test)) { checkResult(); + } else if (typeof test.events == "function" && !test.events().length) { + checkResult(); + } } else { finish();