Steps to reproduce : 1. Open the History subview 2. Click "clear recent History" 3. The dialog appears, but the menu panel stays open. 4. Click one time on "clear now", the panel is dismissed 5. Click a second time History is cleared. The panel should be dismissed when the clear dialog is opened or bug 596723 should be resolved.

Not requiring for Australis:M7.

WFM on OS X.

So first off, to reproduce, move the history panel into the toolbar. With this patch (and, on OS X, the patch for bug 886317), this now works. However, on OS X I'm seeing a rendering glitch in some cases... Pretty sure that's not our fault though? Will attach a screenshot in a second.

Also, I forgot to mention on the patch upload, I'm not very fond of exposing these methods to the wider world, but I don't think we should duplicate them either. Open to other methods of code sharing / deduplication.

Comment on attachment 774784 [details] [diff] [review] Clicking on clear recent history in the history subview doesn't dismiss the panel Chucking this in mconley's queue instead. :-o

Comment on attachment 774784 [details] [diff] [review] Clicking on clear recent history in the history subview doesn't dismiss the panel At least on Windows 7, this bug still persists after building with this patch...

So, why didn't it work with the previous patch? Because the click/keypress handlers only fired on bubble - but fixing that alone wouldn't be enough, because registerMenuPanel passes the PanelUI-contents, not the actual <panel> node, which means the won't capture things in subviews. The command handler I added needs to be on the window because for buttons with a command attribute, the command event gets redirected to their <command> element, which means we never see it otherwise. Of course, we could fix this by just fixing the "clear history" item, but we'd then run into the same problem with other buttons, so I went with this instead...

Comment on attachment 776329 [details] [diff] [review] Clicking on clear recent history in the history subview doesn't dismiss the panel, Hmm, I thought setting r= in the patch header would make bzexport Do the Right Thing. I guess not. Sorry for the spam!

Comment on attachment 776329 [details] [diff] [review] Clicking on clear recent history in the history subview doesn't dismiss the panel, Review of attachment 776329 [details] [diff] [review]: ----------------------------------------------------------------- So this looks pretty good. If I'm not mistaken, we might actually be able to remove the key/click handlers on the main panel too. I think capturing command listener on window is less hacky than keyboard/click handler duplicated everywhere. So this idea has my +1. Two little nits on the code (see below). Not giving an r+ because it seems to regress the subview behaviour of the History widget. STR: 1) With the History widget in the menu panel, click on it to open the history subview 2) Move the History widget to the toolbar, and click on it to open its panel 3) Click on "Clear Recent History" in the new panel. Close the window that appears without clearing any history. 4) Move the History widget back to the menu panel 5) Click on the History widget to open the subview again ER: History subview should open. AR: Button is clicked, but nothing happens. My guess is this code isn't playing nicely with CustomizablehandleWidgetCommand. ::: browser/components/customizableui/content/panelUI.js @@ +185,5 @@ > > let multiView = document.createElement("panelmultiview"); > tempPanel.appendChild(multiView); > multiView.setMainView(viewNode); > + CustomizableUI.ensureButtonsClosePanel(tempPanel); I think this should be renamed to addPanelCloseListeners - just so that the relationship between it and removePanelCloseListeners is more obvious. ::: browser/components/customizableui/src/CustomizableUI.jsm @@ +1987,2 @@ > } > + Nit - trailing whitespace.

So this fixes the naming, the extraneous whitespace, and the issue outlined in comment 10. Unfortunately it seems we can't do without the keyboard/click works because it breaks the work done in bug 879985 - to the extent that the panel no longer closes when using the search widget, at all. Which isn't ideal...

Comment on attachment 776473 [details] [diff] [review] Clicking on clear recent history in the history subview doesn't dismiss the panel, Review of attachment 776473 [details] [diff] [review]: ----------------------------------------------------------------- Yep, let's take it. r=me. Thanks Gijs!

Tested on :--- User Agent : Mozilla/5.0 (Windows NT 6.2; rv:28.0) Gecko/20100101 Firefox/28.0 Version : Firefox 28.0a1 Platform Tested : x86_64, Win 8 Status : Found Fixed.

Marking this verified fixed based on comment 15. Thanks for your help Ganesh.