Bug 1313131, don't wrap when using cursor navigation in menus onr dropdowns on Mac, or dropdowns on Windows, r=ksteuber
☠☠ backed out by 68e56e111cb0 ☠ ☠
authorNeil Deakin <neil@mozilla.com>
Thu, 17 Nov 2016 09:28:27 -0500
changeset 323013 4ed7a34ea7ab5e1f9febafca8133c9959e151db5
parent 323012 deec8c2ba9314dadb8a4e80581f8cba467de854f
child 323014 9af8f4f33471454d4e645f22900f50c64ddf4316
push id84025
push userneil@mozilla.com
push dateThu, 17 Nov 2016 14:28:51 +0000
treeherdermozilla-inbound@09093d38540e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersksteuber
bugs1313131
milestone53.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 1313131, don't wrap when using cursor navigation in menus onr dropdowns on Mac, or dropdowns on Windows, r=ksteuber
accessible/tests/mochitest/events/test_focus_listcontrols.xul
layout/xul/nsMenuBarFrame.cpp
layout/xul/nsMenuFrame.cpp
layout/xul/nsMenuPopupFrame.cpp
layout/xul/nsXULPopupManager.cpp
layout/xul/nsXULPopupManager.h
toolkit/content/tests/chrome/popup_trigger.js
toolkit/content/tests/chrome/test_menulist_keynav.xul
toolkit/content/tests/widgets/popup_shared.js
--- a/accessible/tests/mochitest/events/test_focus_listcontrols.xul
+++ b/accessible/tests/mochitest/events/test_focus_listcontrols.xul
@@ -47,17 +47,17 @@
       gQueue.push(new synthFocus("emptyrichlistbox", new focusChecker("emptyrichlistbox")));
 
       gQueue.push(new synthFocus("menulist"));
       gQueue.push(new synthClick("menulist", new focusChecker("ml_tangerine")));
       gQueue.push(new synthDownKey("ml_tangerine", new focusChecker("ml_marmalade")));
       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")));
 } else {
       todo(false, "Bug 746531 - timeouts of last three menulist tests on OS X");
 }
 
       var textentry = getAccessible("emenulist").firstChild;
@@ -169,16 +169,17 @@ if (!MAC) {
         </richlistitem>
       </richlistbox>
       <richlistbox id="emptyrichlistbox" seltype="multiple"/>
 
       <menulist id="menulist">
         <menupopup>
           <menuitem id="ml_tangerine" label="tangerine trees"/>
           <menuitem id="ml_marmalade" label="marmalade skies"/>
+          <menuitem id="ml_strawberry" label="strawberry fields"/>
         </menupopup>
       </menulist>
       <menulist id="emenulist" editable="true">
         <menupopup>
           <menuitem id="eml_tangerine" label="tangerine trees"/>
           <menuitem id="eml_marmalade" label="marmalade skies"/>
         </menupopup>
       </menulist>
--- a/layout/xul/nsMenuBarFrame.cpp
+++ b/layout/xul/nsMenuBarFrame.cpp
@@ -142,17 +142,17 @@ nsMenuBarFrame::ToggleMenuActiveState()
   else {
     // if the menu bar is already selected (eg. mouseover), deselect it
     if (mCurrentMenu)
       mCurrentMenu->SelectMenu(false);
 
     // 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);
       firstFrame->SelectMenu(true);
       
       // Track this item for keyboard navigation.
       mCurrentMenu = firstFrame;
     }
--- a/layout/xul/nsMenuFrame.cpp
+++ b/layout/xul/nsMenuFrame.cpp
@@ -999,31 +999,31 @@ nsMenuFrame::UpdateMenuSpecialState()
    * selected item.  That, however, would be a lot more work, and I don't think
    * it's better at all.
    */
 
   /* walk siblings, looking for the other checked item with the same name */
   // 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);
     if (sib != this) {
       if (menu && menu->GetMenuType() == eMenuType_Radio &&
           menu->IsChecked() && menu->GetRadioGroupName() == mGroupName) {
         /* uncheck the old item */
         sib->GetContent()->UnsetAttr(kNameSpaceID_None, nsGkAtoms::checked,
                                      true);
         /* XXX in DEBUG, check to make sure that there aren't two checked items */
         return;
       }
     }
-    sib = nsXULPopupManager::GetNextMenuItem(GetParent(), menu, true);
+    sib = nsXULPopupManager::GetNextMenuItem(GetParent(), menu, true, true);
     if (sib == firstMenuItem) {
       break;
     }
   } 
 }
 
 void 
 nsMenuFrame::BuildAcceleratorText(bool aNotify)
--- a/layout/xul/nsMenuPopupFrame.cpp
+++ b/layout/xul/nsMenuPopupFrame.cpp
@@ -1833,17 +1833,17 @@ void nsMenuPopupFrame::ChangeByPage(bool
 
   nsMenuFrame* newMenu = nullptr;
   nsIFrame* currentMenu = mCurrentMenu;
   if (!currentMenu) {
     // If there is no current menu item, get the first item. When moving up,
     // 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;
     }
   }
 
   if (currentMenu) {
     nscoord scrollHeight = mRect.height;
     nsIScrollableFrame *scrollframe = GetScrollFrame(this);
@@ -2061,17 +2061,17 @@ nsMenuPopupFrame::FindMenuWithShortcut(n
 
   sLastKeyTime = keyTime;
 
   // NOTE: If you crashed here due to a bogus |immediateParent| it is 
   //       possible that the menu whose shortcut is being looked up has 
   //       been destroyed already.  One strategy would be to 
   //       setTimeout(<func>,0) as detailed in:
   //       <http://bugzilla.mozilla.org/show_bug.cgi?id=126675#c32>
-  nsIFrame* firstMenuItem = nsXULPopupManager::GetNextMenuItem(immediateParent, nullptr, true);
+  nsIFrame* firstMenuItem = nsXULPopupManager::GetNextMenuItem(immediateParent, nullptr, true, false);
   nsIFrame* currFrame = firstMenuItem;
 
   int32_t menuAccessKey = -1;
   nsMenuBarListener::GetMenuAccessKey(&menuAccessKey);
 
   // We start searching from first child. This process is divided into two parts
   //   -- before current and after current -- by the current item
   while (currFrame) {
@@ -2126,17 +2126,17 @@ nsMenuPopupFrame::FindMenuWithShortcut(n
         // If there is more than one char typed, the current item has highest priority,
         //   otherwise the item next to current has highest priority
         if (currFrame == frameBefore)
           return frameBefore;
       }
     }
 
     nsMenuFrame* menu = do_QueryFrame(currFrame);
-    currFrame = nsXULPopupManager::GetNextMenuItem(immediateParent, menu, true);
+    currFrame = nsXULPopupManager::GetNextMenuItem(immediateParent, menu, true, true);
     if (currFrame == firstMenuItem)
       break;
   }
 
   doAction = (isMenu && (matchCount == 1 || matchShortcutCount == 1));
 
   if (matchShortcutCount == 1) // We have one matched shortcut item
     return frameShortcut;
--- a/layout/xul/nsXULPopupManager.cpp
+++ b/layout/xul/nsXULPopupManager.cpp
@@ -940,17 +940,17 @@ nsXULPopupManager::ShowPopupCallback(nsI
     if (mPopups)
       oldmenu = mPopups->Content();
     item->SetParent(mPopups);
     mPopups = item;
     SetCaptureState(oldmenu);
   }
 
   if (aSelectFirstItem) {
-    nsMenuFrame* next = GetNextMenuItem(aPopupFrame, nullptr, true);
+    nsMenuFrame* next = GetNextMenuItem(aPopupFrame, nullptr, true, false);
     aPopupFrame->SetCurrentMenuItem(next);
   }
 
   if (ismenu)
     UpdateMenuItems(aPopup);
 
   // Caret visibility may have been affected, ensure that
   // the caret isn't now drawn when it shouldn't be.
@@ -2202,18 +2202,18 @@ nsXULPopupManager::HandleKeyboardNavigat
     return true;
 
   // no popup handled the key, so check the active menubar, if any
   if (mActiveMenuBar) {
     nsMenuFrame* currentMenu = mActiveMenuBar->GetCurrentMenuItem();
   
     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;
     }
     else if (NS_DIRECTION_IS_BLOCK(theDirection)) {
       // Open the menu and select its first item.
       if (currentMenu) {
         nsCOMPtr<nsIContent> content = currentMenu->GetContent();
         ShowMenu(content, true, false);
@@ -2238,17 +2238,17 @@ nsXULPopupManager::HandleKeyboardNavigat
 
   aFrame->ClearIncrementalString();
 
   // This method only gets called if we're open.
   if (!currentMenu && NS_DIRECTION_IS_INLINE(aDir)) {
     // 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;
       }
     }
     return false;
   }
 
@@ -2272,24 +2272,38 @@ nsXULPopupManager::HandleKeyboardNavigat
     }
   }
 
   // For block progression, we can move in either direction
   if (NS_DIRECTION_IS_BLOCK(aDir) ||
       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);
       return true;
     }
   }
   else if (currentMenu && isContainer && isOpen) {
     if (aDir == eNavigationDirection_Start) {
@@ -2401,17 +2415,18 @@ nsXULPopupManager::HandleKeyboardEventWi
     aKeyEvent->AsEvent()->PreventDefault();
   }
   return true;
 }
 
 nsMenuFrame*
 nsXULPopupManager::GetNextMenuItem(nsContainerFrame* aParent,
                                    nsMenuFrame* aStart,
-                                   bool aIsPopup)
+                                   bool aIsPopup,
+                                   bool aWrap)
 {
   nsPresContext* presContext = aParent->PresContext();
   auto insertion = presContext->PresShell()->
     FrameConstructor()->GetInsertionPoint(aParent->GetContent(), nullptr);
   nsContainerFrame* immediateParent = insertion.mParentFrame;
   if (!immediateParent)
     immediateParent = aParent;
 
@@ -2436,16 +2451,20 @@ nsXULPopupManager::GetNextMenuItem(nsCon
       currFrame = currFrame->PrincipalChildList().FirstChild();
     else if (!currFrame->GetNextSibling() &&
              currFrame->GetParent()->GetContent()->IsXULElement(nsGkAtoms::menugroup))
       currFrame = currFrame->GetParent()->GetNextSibling();
     else
       currFrame = currFrame->GetNextSibling();
   }
 
+  if (!aWrap) {
+    return aStart;
+  }
+
   currFrame = immediateParent->PrincipalChildList().FirstChild();
 
   // Still don't have anything. Try cycling from the beginning.
   while (currFrame && currFrame != aStart) {
     // See if it's a menu item.
     nsIContent* currFrameContent = currFrame->GetContent();
     if (IsValidMenuItem(currFrameContent, aIsPopup)) {
       return do_QueryFrame(currFrame);
@@ -2462,17 +2481,18 @@ nsXULPopupManager::GetNextMenuItem(nsCon
 
   // No luck. Just return our start value.
   return aStart;
 }
 
 nsMenuFrame*
 nsXULPopupManager::GetPreviousMenuItem(nsContainerFrame* aParent,
                                        nsMenuFrame* aStart,
-                                       bool aIsPopup)
+                                       bool aIsPopup,
+                                       bool aWrap)
 {
   nsPresContext* presContext = aParent->PresContext();
   auto insertion = presContext->PresShell()->
     FrameConstructor()->GetInsertionPoint(aParent->GetContent(), nullptr);
   nsContainerFrame* immediateParent = insertion.mParentFrame;
   if (!immediateParent)
     immediateParent = aParent;
 
@@ -2501,16 +2521,20 @@ nsXULPopupManager::GetPreviousMenuItem(n
     }
     else if (!currFrame->GetPrevSibling() &&
              currFrame->GetParent()->GetContent()->IsXULElement(nsGkAtoms::menugroup))
       currFrame = currFrame->GetParent()->GetPrevSibling();
     else
       currFrame = currFrame->GetPrevSibling();
   }
 
+  if (!aWrap) {
+    return aStart;
+  }
+
   currFrame = frames.LastChild();
 
   // Still don't have anything. Try cycling from the end.
   while (currFrame && currFrame != aStart) {
     // See if it's a menu item.
     nsIContent* currFrameContent = currFrame->GetContent();
     if (IsValidMenuItem(currFrameContent, aIsPopup)) {
       return do_QueryFrame(currFrame);
--- a/layout/xul/nsXULPopupManager.h
+++ b/layout/xul/nsXULPopupManager.h
@@ -363,22 +363,26 @@ public:
   //
   // Both methods will loop around the beginning or end if needed.
   //
   // aParent - the parent menubar or menupopup
   // aStart - the menu/menuitem to start navigation from. GetPreviousMenuItem
   //          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
   // for items on a menubar.
   static bool IsValidMenuItem(nsIContent* aContent, bool aOnPopup);
 
   // inform the popup manager that a menu bar has been activated or deactivated,
   // either because one of its menus has opened or closed, or that the menubar
--- a/toolkit/content/tests/chrome/popup_trigger.js
+++ b/toolkit/content/tests/chrome/popup_trigger.js
@@ -83,26 +83,30 @@ var popupTests = [
   testname: "cursor down no selection",
   events: [ "DOMMenuItemActive item1" ],
   test: function() { synthesizeKey("VK_DOWN", { }); },
   result: function(testname) { checkActive(gMenuPopup, "item1", testname); }
 },
 {
   // 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); }
 },
 {
   // check that pressing cursor down highlights the second item
   testname: "cursor down",
   events: [ "DOMMenuItemInactive item1", "DOMMenuItemActive item2" ],
   test: function() { synthesizeKey("VK_DOWN", { }); },
--- a/toolkit/content/tests/chrome/test_menulist_keynav.xul
+++ b/toolkit/content/tests/chrome/test_menulist_keynav.xul
@@ -36,76 +36,85 @@ SimpleTest.waitForExplicitFinish();
 var gShowPopup = false;
 var gModifiers = 0;
 var gOpenPhase = false;
 
 var list = $("list");
 let expectCommandEvent;
 
 var iswin = (navigator.platform.indexOf("Win") == 0);
+var ismac = (navigator.platform.indexOf("Mac") == 0);
 
 function runTests()
 {
   list.focus();
 
   // 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");
 
   pressLetter();
 }
 
 function pressLetter()
 {
   // A command event should be fired only if the menulist is closed, or on Windows,
   // where items are selected immediately.
   expectCommandEvent = !gOpenPhase || iswin;
 
   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");
   }
 
   setTimeout(pressedAgain, 1200);
 }
 
 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;
     tabAndScroll();
   }
   else {
      // Run the letter tests again with the popup open
     info("list open phase");
@@ -128,17 +137,17 @@ function differentPressed()
     list.open = true;
   }
 }
 
 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");
     is(document.activeElement, $("button2"), "tab from menulist focused button");
   }
 
   // now make sure that using a key scrolls the menu correctly
 
@@ -171,21 +180,23 @@ function tabAndScroll()
   synthesizeKey("VK_DOWN", { });
   is(item.getBoundingClientRect().top, originalPosition - rowdiff, "position of item 10");
 
   list.open = false;
 
   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)
 {
   var expectedModifiers = (gModifiers == 1);
   is(event.shiftKey, expectedModifiers, "shift key pressed");
   is(event.ctrlKey, expectedModifiers, "ctrl key pressed");
   is(event.altKey, expectedModifiers, "alt key pressed");
@@ -238,16 +249,30 @@ function checkCursorNavigation()
   synthesizeKey("VK_DOWN", { });
   is(list.selectedIndex, iswin ? 2 : 1, "selectedIndex after cursor down");
   is(commandEventsCount, iswin ? 1 : 0, "selectedIndex after cursor down command event");
   is(list.menupopup.state, "open", "cursor down popup state");
   synthesizeKey("VK_PAGE_DOWN", { });
   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;
 }
 
 function done()
 {
   SimpleTest.finish();
 }
 
--- a/toolkit/content/tests/widgets/popup_shared.js
+++ b/toolkit/content/tests/widgets/popup_shared.js
@@ -242,18 +242,21 @@ function goNextStepSync()
     // start with the first step if there are any
     var step = null;
     if ("steps" in test)
       step = test.steps[gTestStepIndex];
 
     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();
   }
 }
 
 function openMenu(menu)
 {