Opening a panel from within an oncommand handler of a menuitem fails. It seems as the panel is auto-hidden when the menuitem's containing popup is closed. Opening the panel asynchronously (i.e. setTimeout) works.

This is very easy fix. The call to Rollup in nsXULMenuCommandEvent::Run should call HidePopup with the menu to close instead so that any new popups that were opened aren't rolled up as well. Mano, does this fix your case? If so, I'll create a test for this too.

Yes, thanks Neil.

This changes it so that: 1. In HidePopup, don't close up children that aren't submenus. 2. In nsXULCommandEvent::Run, make sure to call HidePopup with the parent popup of the menuitem that was executed, rather than Rollup, as Rollup always closes the topmost popup, and in this case, a new panel was opened above it during the preceding command event.

So I'm not sure I follow... In HidePopup, we could have menus that are on top of us but are not submenus? And are we looking for the bottommost menu (as the comments say) or the topmost one (as the variable naming seems to indicate)? How do |foundMenu| and |item| differ at that point in the code, if at all?

(In reply to comment #5) > So I'm not sure I follow... In HidePopup, we could have menus that are on top > of us but are not submenus? There could be popups that aren't menus. And are we looking for the bottommost menu (as the > comments say) or the topmost one (as the variable naming seems to indicate)? It should be topmost, I will fix the comment. > > How do |foundMenu| and |item| differ at that point in the code, if at all? > At the 'if (foundMenu)' line, foundMenu and item will be the same. I changed the code to use foundMenu consistently and just reuse 'item' as a temporary variable.

Comment on attachment 277392 [details] [diff] [review] include test >Index: layout/xul/base/src/nsXULPopupManager.cpp >+ // find the bottomost menu and close that menu first. In synchronous mode, "find the topmost menu with only menus between it and foundMenu and close ...". >+ if (foundMenu->IsMenu()) { Why this check? Document. r+sr=bzbarsky wth that

Comment on attachment 277392 [details] [diff] [review] include test Will fix the comments.

Comment on attachment 277392 [details] [diff] [review] include test a1.9=dbaron