Bug 1149745, on Windows, menulist should select the value when the cursor keys are used to navigate items, r=neil

This commit is contained in:
Neil Deakin 2015-06-26 09:32:25 -07:00
parent f19c7ac78a
commit 80de8e4360
13 changed files with 160 additions and 40 deletions

View File

@ -51,9 +51,11 @@
gQueue.push(new synthDownKey("ml_tangerine", new focusChecker("ml_marmalade")));
gQueue.push(new synthEscapeKey("ml_marmalade", new focusChecker("menulist")));
if (!MAC) {
gQueue.push(new synthDownKey("menulist", new nofocusChecker("ml_marmalade")));
gQueue.push(new synthOpenComboboxKey("menulist", new focusChecker("ml_marmalade")));
gQueue.push(new synthEnterKey("ml_marmalade", new focusChecker("menulist")));
// On Windows, items get selected during navigation.
let expectedItem = WIN ? "ml_tangerine" : "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");
}
@ -73,7 +75,7 @@ if (!MAC) {
gQueue.push(new changeCurrentItem("listbox", "lb_item1"));
gQueue.push(new changeCurrentItem("richlistbox", "rlb_item1"));
if (!MAC) {
gQueue.push(new changeCurrentItem("menulist", "ml_tangerine"));
gQueue.push(new changeCurrentItem("menulist", WIN ? "ml_marmalade" : "ml_tangerine"));
}
gQueue.push(new changeCurrentItem("emenulist", "eml_tangerine"));

View File

@ -57,6 +57,8 @@ add_task(function*() {
yield openSelectPopup(selectPopup);
let isWindows = navigator.platform.indexOf("Win") >= 0;
is(menulist.selectedIndex, 1, "Initial selection");
is(selectPopup.firstChild.localName, "menucaption", "optgroup is caption");
is(selectPopup.firstChild.getAttribute("label"), "First Group", "optgroup label");
@ -65,17 +67,20 @@ add_task(function*() {
EventUtils.synthesizeKey("KEY_ArrowDown", { code: "ArrowDown" });
is(menulist.menuBoxObject.activeChild, menulist.getItemAtIndex(2), "Select item 2");
is(menulist.selectedIndex, isWindows ? 2 : 1, "Select item 2 selectedIndex");
EventUtils.synthesizeKey("KEY_ArrowDown", { code: "ArrowDown" });
is(menulist.menuBoxObject.activeChild, menulist.getItemAtIndex(3), "Select item 3");
is(menulist.selectedIndex, isWindows ? 3 : 1, "Select item 3 selectedIndex");
EventUtils.synthesizeKey("KEY_ArrowDown", { code: "ArrowDown" });
// On Windows, one can navigate on disabled menuitems
let expectedIndex = (navigator.platform.indexOf("Win") >= 0) ? 5 : 9;
let expectedIndex = isWindows ? 5 : 9;
is(menulist.menuBoxObject.activeChild, menulist.getItemAtIndex(expectedIndex),
"Skip optgroup header and disabled items select item 7");
is(menulist.selectedIndex, isWindows ? 5 : 1, "Select or skip disabled item selectedIndex");
for (let i = 0; i < 10; i++) {
is(menulist.getItemAtIndex(i).disabled, i >= 4 && i <= 7, "item " + i + " disabled")
@ -83,6 +88,7 @@ add_task(function*() {
EventUtils.synthesizeKey("KEY_ArrowUp", { code: "ArrowUp" });
is(menulist.menuBoxObject.activeChild, menulist.getItemAtIndex(3), "Select item 3 again");
is(menulist.selectedIndex, isWindows ? 3 : 1, "Select item 3 selectedIndex");
yield hideSelectPopup(selectPopup);

View File

@ -317,7 +317,8 @@ private:
NS_IMETHODIMP
nsMenuBarFrame::ChangeMenuItem(nsMenuFrame* aMenuItem,
bool aSelectFirstItem)
bool aSelectFirstItem,
bool aFromKey)
{
if (mCurrentMenu == aMenuItem)
return NS_OK;

View File

@ -35,7 +35,9 @@ public:
virtual nsMenuFrame* GetCurrentMenuItem() override;
NS_IMETHOD SetCurrentMenuItem(nsMenuFrame* aMenuItem) override;
virtual void CurrentMenuIsBeingDestroyed() override;
NS_IMETHOD ChangeMenuItem(nsMenuFrame* aMenuItem, bool aSelectFirstItem) override;
NS_IMETHOD ChangeMenuItem(nsMenuFrame* aMenuItem,
bool aSelectFirstItem,
bool aFromKey) override;
NS_IMETHOD SetActive(bool aActiveFlag) override;

View File

@ -477,7 +477,7 @@ nsMenuFrame::HandleEvent(nsPresContext* aPresContext,
// Submenus don't get closed up immediately.
}
else if (this == menuParent->GetCurrentMenuItem()) {
menuParent->ChangeMenuItem(nullptr, false);
menuParent->ChangeMenuItem(nullptr, false, false);
}
}
}
@ -490,7 +490,7 @@ nsMenuFrame::HandleEvent(nsPresContext* aPresContext,
}
// Let the menu parent know we're the new item.
menuParent->ChangeMenuItem(this, false);
menuParent->ChangeMenuItem(this, false, false);
NS_ENSURE_TRUE(weakFrame.IsAlive(), NS_OK);
NS_ENSURE_TRUE(menuParent, NS_OK);
@ -1433,7 +1433,7 @@ nsMenuFrame::SetActiveChild(nsIDOMElement* aChild)
if (!aChild) {
// Remove the current selection
popupFrame->ChangeMenuItem(nullptr, false);
popupFrame->ChangeMenuItem(nullptr, false, false);
return NS_OK;
}
@ -1441,7 +1441,7 @@ nsMenuFrame::SetActiveChild(nsIDOMElement* aChild)
nsMenuFrame* menu = do_QueryFrame(child->GetPrimaryFrame());
if (menu)
popupFrame->ChangeMenuItem(menu, false);
popupFrame->ChangeMenuItem(menu, false, false);
return NS_OK;
}

View File

@ -30,8 +30,11 @@ public:
// new item aMenuItem. For a menubar, if another menu is already open, the
// new menu aMenuItem is opened. In this case, if aSelectFirstItem is true,
// select the first item in it. For menupopups, the menu is not opened and
// the aSelectFirstItem argument is not used.
NS_IMETHOD ChangeMenuItem(nsMenuFrame* aMenuItem, bool aSelectFirstItem) = 0;
// the aSelectFirstItem argument is not used. The aFromKey argument indicates
// that the keyboard was used to navigate to the new menu item.
NS_IMETHOD ChangeMenuItem(nsMenuFrame* aMenuItem,
bool aSelectFirstItem,
bool aFromKey) = 0;
// returns true if the menupopup is open. For menubars, returns false.
virtual bool IsOpen() = 0;

View File

@ -45,6 +45,7 @@
#include "nsThemeConstants.h"
#include "nsTransitionManager.h"
#include "nsDisplayList.h"
#include "nsIDOMXULSelectCntrlItemEl.h"
#include "mozilla/EventDispatcher.h"
#include "mozilla/EventStateManager.h"
#include "mozilla/EventStates.h"
@ -1754,7 +1755,7 @@ void nsMenuPopupFrame::ChangeByPage(bool aIsUp)
// Select the new menuitem.
if (newMenu) {
ChangeMenuItem(newMenu, false);
ChangeMenuItem(newMenu, false, true);
}
}
@ -1785,7 +1786,8 @@ nsMenuPopupFrame::CurrentMenuIsBeingDestroyed()
NS_IMETHODIMP
nsMenuPopupFrame::ChangeMenuItem(nsMenuFrame* aMenuItem,
bool aSelectFirstItem)
bool aSelectFirstItem,
bool aFromKey)
{
if (mCurrentMenu == aMenuItem)
return NS_OK;
@ -1812,6 +1814,26 @@ nsMenuPopupFrame::ChangeMenuItem(nsMenuFrame* aMenuItem,
if (aMenuItem) {
EnsureMenuItemIsVisible(aMenuItem);
aMenuItem->SelectMenu(true);
// On Windows, a menulist should update its value whenever navigation was
// done by the keyboard.
#ifdef XP_WIN
if (aFromKey && IsOpen()) {
nsIFrame* parentMenu = GetParent();
if (parentMenu) {
nsCOMPtr<nsIDOMXULMenuListElement> menulist = do_QueryInterface(parentMenu->GetContent());
if (menulist) {
// Fire a command event as the new item, but we don't want to close
// the menu, blink it, or update any other state of the menuitem. The
// command event will cause the item to be selected.
nsContentUtils::DispatchXULCommand(aMenuItem->GetContent(),
nsContentUtils::IsCallerChrome(),
nullptr, PresContext()->PresShell(),
false, false, false, false);
}
}
}
#endif
}
mCurrentMenu = aMenuItem;

View File

@ -171,7 +171,9 @@ public:
virtual nsMenuFrame* GetCurrentMenuItem() override;
NS_IMETHOD SetCurrentMenuItem(nsMenuFrame* aMenuItem) override;
virtual void CurrentMenuIsBeingDestroyed() override;
NS_IMETHOD ChangeMenuItem(nsMenuFrame* aMenuItem, bool aSelectFirstItem) override;
NS_IMETHOD ChangeMenuItem(nsMenuFrame* aMenuItem,
bool aSelectFirstItem,
bool aFromKey) override;
// as popups are opened asynchronously, the popup pending state is used to
// prevent multiple requests from attempting to open the same popup twice

View File

@ -1988,7 +1988,7 @@ nsXULPopupManager::HandleShortcutNavigation(nsIDOMKeyEvent* aKeyEvent,
bool action;
nsMenuFrame* result = aFrame->FindMenuWithShortcut(aKeyEvent, action);
if (result) {
aFrame->ChangeMenuItem(result, false);
aFrame->ChangeMenuItem(result, false, true);
if (action) {
WidgetGUIEvent* evt = aKeyEvent->GetInternalNSEvent()->AsGUIEvent();
nsMenuFrame* menuToOpen = result->Enter(evt);
@ -2069,7 +2069,7 @@ nsXULPopupManager::HandleKeyboardNavigation(uint32_t aKeyCode)
nsMenuFrame* nextItem = (theDirection == eNavigationDirection_End) ?
GetNextMenuItem(mActiveMenuBar, currentMenu, false) :
GetPreviousMenuItem(mActiveMenuBar, currentMenu, false);
mActiveMenuBar->ChangeMenuItem(nextItem, true);
mActiveMenuBar->ChangeMenuItem(nextItem, true, true);
return true;
}
else if (NS_DIRECTION_IS_BLOCK(theDirection)) {
@ -2105,7 +2105,7 @@ nsXULPopupManager::HandleKeyboardNavigationInPopup(nsMenuChainItem* item,
if (aDir == eNavigationDirection_End) {
nsMenuFrame* nextItem = GetNextMenuItem(aFrame, nullptr, true);
if (nextItem) {
aFrame->ChangeMenuItem(nextItem, false);
aFrame->ChangeMenuItem(nextItem, false, true);
return true;
}
}
@ -2147,7 +2147,7 @@ nsXULPopupManager::HandleKeyboardNavigationInPopup(nsMenuChainItem* item,
nextItem = GetPreviousMenuItem(aFrame, nullptr, true);
if (nextItem) {
aFrame->ChangeMenuItem(nextItem, false);
aFrame->ChangeMenuItem(nextItem, false, true);
return true;
}
}

View File

@ -193,6 +193,7 @@ function test_menulist_open(element, scroller)
element.appendItem("Scroll Item 1", "scrollitem1");
element.appendItem("Scroll Item 2", "scrollitem2");
element.focus();
element.selectedIndex = 0;
/*
// bug 530504, mousewheel while menulist is open should not scroll menulist
@ -207,13 +208,14 @@ function test_menulist_open(element, scroller)
window.removeEventListener("DOMMouseScroll", mouseScrolled, false);
*/
// bug 543065, hovering the mouse over an item should highlight it and not
// scroll the parent
// bug 543065, hovering the mouse over an item should highlight it, not
// scroll the parent, and not change the selected index.
var item = element.menupopup.childNodes[1];
synthesizeMouse(element.menupopup.childNodes[1], 2, 2, { type: "mousemove" });
synthesizeMouse(element.menupopup.childNodes[1], 6, 6, { type: "mousemove" });
is(element.menuBoxObject.activeChild, item, "activeChild after menu highlight " + element.id);
is(element.selectedIndex, 0, "selectedIndex after menu highlight " + element.id);
is(scroller.scrollTop, 0, "scroll position after menu highlight " + element.id);
element.open = false;

View File

@ -19,6 +19,14 @@
</menupopup>
</menulist>
<button id="button2" label="Two"/>
<menulist id="list2">
<menupopup id="popup" onpopupshown="checkCursorNavigation();" onpopuphidden="done()">
<menuitem id="b1" label="One"/>
<menuitem id="b2" label="Two" selected="true"/>
<menuitem id="b3" label="Three"/>
<menuitem id="b4" label="Four"/>
</menupopup>
</menulist>
<script class="testbody" type="application/javascript">
<![CDATA[
@ -27,6 +35,10 @@ SimpleTest.waitForExplicitFinish();
var gShowPopup = false;
var gModifiers = 0;
var gOpenPhase = false;
var list = $("list");
let expectCommandEvent;
var iswin = (navigator.platform.indexOf("Win") == 0);
@ -34,9 +46,11 @@ function runTests()
{
var list = $("list");
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) {
expectCommandEvent = true;
keyCheck(list, "VK_DOWN", 2, "cursor down");
keyCheck(list, "VK_DOWN", iswin ? "2b" : 3, "cursor down skip disabled");
keyCheck(list, "VK_UP", 2, "cursor up skip disabled");
@ -51,19 +65,35 @@ function runTests()
synthesizeKey("VK_UP", { altKey: navigator.platform.indexOf("Mac") == -1 });
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");
SpecialPowers.setIntPref("ui.menu.incremental_search.timeout", 0); // prevent to timeout
keyCheck(list, "T", 2, "letter pressed");
SpecialPowers.clearUserPref("ui.menu.incremental_search.timeout");
setTimeout(pressedAgain, 1200);
if (!gOpenPhase) {
SpecialPowers.setIntPref("ui.menu.incremental_search.timeout", 0); // prevent to timeout
keyCheck(list, "T", 2, "same letter pressed");
SpecialPowers.clearUserPref("ui.menu.incremental_search.timeout");
}
setTimeout(pressedAgain, 1200);
}
function pressedAgain()
{
var list = $("list");
keyCheck(list, "T", iswin ? "2b" : 3, "letter pressed again");
SpecialPowers.setIntPref("ui.menu.incremental_search.timeout", 0); // prevent to timeout
keyCheck(list, "W", 2, "second letter pressed");
@ -74,8 +104,32 @@ function pressedAgain()
function differentPressed()
{
var list = $("list");
keyCheck(list, "O", 1, "different letter pressed");
if (gOpenPhase) {
list.open = false;
tabAndScroll();
}
else {
// Run the letter tests again with the popup open
info("list open phase");
list.selectedItem = $("i1");
gShowPopup = true;
list.open = true;
gOpenPhase = true;
list.addEventListener("popupshown", function popupShownListener() {
list.removeEventListener("popupshown", popupShownListener, false);
pressLetter();
}, false);
}
}
function tabAndScroll()
{
if (navigator.platform.indexOf("Mac") == -1) {
$("button1").focus();
synthesizeKeyExpectEvent("VK_TAB", { }, list, "focus", "focus to menulist");
@ -84,7 +138,6 @@ function differentPressed()
}
// now make sure that using a key scrolls the menu correctly
gShowPopup = true;
for (let i = 0; i < 65; i++) {
list.appendItem("Item" + i, "item" + i);
@ -123,8 +176,8 @@ function differentPressed()
function keyCheck(list, key, index, testname)
{
var item = $("i" + index);
synthesizeKeyExpectEvent(key, { }, item, "command", testname);
is(list.selectedItem, item, testname + " selectedItem");
synthesizeKeyExpectEvent(key, { }, item, expectCommandEvent ? "command" : "!command", testname);
is(list.selectedItem, expectCommandEvent ? item : $("i1"), testname + " selectedItem");
}
function checkModifiers(event)
@ -154,13 +207,13 @@ function checkEnterWithModifiers()
ok(!list.open, "list closed on enter press");
list.removeEventListener("popuphidden", checkEnterWithModifiers, false);
list.addEventListener("popuphidden", done, false);
list.addEventListener("popuphidden", verifyPopupOnClose, false);
list.open = true;
synthesizeKey("VK_RETURN", { shiftKey: true, ctrlKey: true, altKey: true, metaKey: true });
}
function done()
function verifyPopupOnClose()
{
is(gModifiers, 2, "modifiers checked when set");
@ -168,6 +221,35 @@ function done()
ok(!list.open, "list closed on enter press with modifiers");
list.removeEventListener("popuphidden", done, false);
list = $("list2");
list.focus();
list.open = true;
}
function checkCursorNavigation()
{
var list = $("list2");
var commandEventsCount = 0;
list.addEventListener("command", event => {
is(event.target, list.selectedItem, "command event fired on selected item");
commandEventsCount++;
}, false);
is(list.selectedIndex, 1, "selectedIndex before cursor down");
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");
list.open = false;
}
function done()
{
SimpleTest.finish();
}

View File

@ -19,6 +19,7 @@ this.EXPORTED_SYMBOLS = [
this.SelectContentHelper = function (aElement, aGlobal) {
this.element = aElement;
this.initialSelection = aElement[aElement.selectedIndex] || null;
this.global = aGlobal;
this.init();
this.showDropDown();
@ -60,16 +61,16 @@ this.SelectContentHelper.prototype = {
receiveMessage: function(message) {
switch (message.name) {
case "Forms:SelectDropDownItem":
if (this.element.selectedIndex != message.data.value) {
this.element.selectedIndex = message.data.value;
this.element.selectedIndex = message.data.value;
break;
case "Forms:DismissedDropDown":
if (this.initialSelection != this.element.item[this.element.selectedIndex]) {
let event = this.element.ownerDocument.createEvent("Events");
event.initEvent("change", true, true);
this.element.dispatchEvent(event);
}
//intentional fall-through
case "Forms:DismissedDropDown":
this.uninit();
break;
}

View File

@ -31,9 +31,6 @@ this.SelectParentHelper = {
},
handleEvent: function(event) {
let popup = event.currentTarget;
let menulist = popup.parentNode;
switch (event.type) {
case "command":
if (event.target.hasAttribute("value")) {
@ -41,14 +38,14 @@ this.SelectParentHelper = {
value: event.target.value
});
}
popup.hidePopup();
break;
case "popuphidden":
currentBrowser.messageManager.sendAsyncMessage("Forms:DismissedDropDown", {});
currentBrowser = null;
let popup = event.target;
this._unregisterListeners(popup);
menulist.hidden = true;
popup.parentNode.hidden = true;
break;
}
},