387499 - keys (Up/Down/any character) no longer working in collapsed XUL combo boxes

Status: RESOLVED FIXED

(Core :: XUL, defect)

Description

[Joanmarie Diggs]

Steps to reproduce:

1. Navigate to a combo box such as the "When Minefield Starts:" combo box in Minefield's Preference's dialog or the "Style:" combo box on the Display pane for Thunderbird's Preferences dialog.

2. With the combo box collapsed, use Up/Down Arrow to move among the choices.

Expected results:  Up/Down Arrow would allow you to navigate among items in the combo box.

Actual results: Up/Down Arrow do nothing.

This seems to have been introduced in the 5th July builds.

Comment 2

[Neil Deakin]

Attachment: handle keyboard and shortcut navigation for closed menulists

Comment 3

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 272397 [details] [diff] [review]
handle keyboard and shortcut navigation for closed menulists

>Index: layout/xul/base/public/nsXULPopupManager.h
>+  PRBool HandleKeyboardNavigationInPopup(nsMenuChainItem* aItem,
>+                                         nsMenuPopupFrame* aFrame,
>+                                         nsNavigationDirection aDir);

I would almost rather have two separate methods, one taking a frame (public) and one taking an nsMenuChainItem* (protected) and have them both call this (private) utility method.  They could even inline to do that.  Makes callers simpler to code without having random nsnulls around.

>Index: layout/xul/base/src/nsXULPopupManager.cpp
nsXULPopupManager::HandleKeyboardNavigationInPopup(nsMenuChainItem* 
>+  NS_ASSERTION(item || aFrame, "aFrame and item are both null");

If you do leave the current signature, you need to either assert that !item || !aFrame, or to assert that if both are non-null then aFrame == item->Frame().  In other words, that the frame to work on is well-defined.

And perhaps document what things don't happen if |item| is null.

With that, sr=bzbarsky.

Comment 4

[Firefox Bug Husbandry Bot]

https://bugzilla.mozilla.org/show_bug.cgi?id=1527381