This seems to be a new failure (at least since Variables View landed, probably more recent). Open a web page, with network logging enabled in the console, click one of the network entries. Panel opens, but unable to close it via the close button. Nothing in the Browser/Error Console. "Mozilla/5.0 (Macintosh; Intel Mac OS X 10.8; rv:23.0) Gecko/20130506 Firefox/23.0"

WFM on Windows 8

Happening to me on Mac as well, brand new nightly. Also, the window has no functioning minimize-to-dock button, and keeps leaping back onto the screen if I drag it out of my way.

WFM on Linux. Is this something you can always repro?

Moving this to Core::Widgets Cocoa

(In reply to comment #0) > with network logging enabled in the console What does this mean?

Last good nightly: 2013-05-01 First bad nightly: 2013-05-02 Pushlog: http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=02aa81c59df6&tochange=70e0955ccc87

Confirmed in local build that bug 851641 caused this.

When we create the widget for the panel we call BaseCreate in nsCocoaWindow::Create and pass it the parent nsIWidget. BaseCreate calls AddChild to add the newly created widget to the child list of the parent widget. The sibling widget then holds a nsCOMPtr to our new widget. The problem is that this pointer is never cleared, so the widget destructor never runs and the widget sticks around. Before bug 851641 we would hide the widget before it was Destroy()'ed so we didn't notice this problem, but I guess we just kept around the widget in memory? The place where it looks like it was intended to clear this sibling pointer is nsBaseWidget::Destroy, but that fails to do anything because GetParent returns null on nsCocoaWindow's because it is not implemented at all (nsBaseWidget's implementation just returns null). So this patch just implements GetParent.

Comment on attachment 747142 [details] [diff] [review] patch This looks fine to me, but I wonder if it's going to have side effects. All the other widget modules' nsIWidget::GetParent() implementations behave as you'd expect -- they return a (weak) reference to a parent if there is one. So does nsChildView::GetParent(). Only nsCocoaWindow doesn't override nsBaseWidget::GetParent(), which always returns NULL. But it *does* have a GetRealParent() method that behaves as you'd think nsCocoaWindow::GetParent() *should* behave. Which makes me wonder if there's some code, somewhere, that assumes that an nsCocoaWindow object never has a parent. If such code does exist, I don't know about it. It's entirely possible that it once existed and has now disappeared. Frankly I think we should just barge ahead with your patch, and see what happens.

By the way, I just checked, and it shouldn't be possible for your new nsCocoaWindow::GetParent() to return a stale pointer -- the nsCocoaWindow destructor already nulls out mParent in each of its children.

So this patch causes an extra assert running /dom/tests/mochitest/bugs/test_bug406375.html https://tbpl.mozilla.org/?tree=Try&rev=b3c30b4e75f5 the assert is ###!!! ASSERTION: Widget destroyed while running modal!: '!mModal', file ../../../widget/cocoa/nsCocoaWindow.mm, line 171 I'm guessing that the patch makes us actually drop the last reference to the window and so it gets destroyed whereas previously we just kept it alive when we shouldn't have.

So a XUL panel is a native window that's modal? If so that's seriously bad news. For complicated reasons, modal "windows" should only be implemented on the Mac as (window-modal, native) sheets or (tab modal, non-native) "HTML windows". Actual native modal windows cause a lot of trouble on the Mac unless they're app-modal (which these XUL panels probably aren't). See particularly bug 478073. I haven't seen many native modal windows lately. Are they something new? In any case, we need to make sure that, on the Mac, if they *are* modal they're implemented either as sheets or "HTML windows". If their current design is a recent decision, could someone tell me the bug where it was made?

(In reply to comment #12) That warning is intended to prevent consumers of the code from doing something that appears unwise. The code should be able to handle this "misuse" without trouble.

>> with network logging enabled in the console > > What does this mean? I just reproduced bug 869423 (which has been marked a duplicate of this bug), whose STR are very clear. It appears that bug's XUL panel *isn't* model. At least it's still possible to interact with other windows while that XUL panel is open. If that means XUL panels in general aren't modal, that's very good news. (Though I still have to figure out why the warning from comment #12 is getting displayed.) But I still need clear STR for this bug. Someone please provide them.

Oops, false alarm. XUL panels aren't modal (thank goodness), and there was no reason to think they are. Seeing the warning on dom/tests/mochitest/bugs/test_bug406375.html, I jumped to the conclusion that it must have something to do with XUL panels. It doesn't. Instead it contains a call to showModalWindow().

As for the assertion, I think it can safely be ignored. Should I open another bug to get rid of it?

Yeah let's get a bug to remove that assertion. It's good that the code handles the situation properly because I don't see any way of coding around it: the problem happens in code like this SetModal(true); run the event loop SetModal(false); So anything can happen before we SetModal(false).

(In reply to Steven Michaud from comment #10) > But it *does* have a > GetRealParent() method that behaves as you'd think > nsCocoaWindow::GetParent() *should* behave. I checked and GetRealParent was added by you in http://hg.mozilla.org/mozilla-central/rev/bf1f1adba3ad so if you don't know of a reason for not having a working GetParent I'm not sure who would. :)

(In reply to comment #19) Sigh :-( I'm usually better at recording my reasons for doing things. I assume I *did* have a reason not to mess with what was at the time existing behavior. But if I did, it's long past recall. All the more reason to land your patch :-)

Just looking over the uses of GetParent I found nsBaseWidget::GetTopLevelWidget, and it has a couple of uses where returning the popup window vs the main window could be an important difference. So if this turns out to be a problem we can just do mParent->RemoveChild(this) from nsCocoaWindow::Destroy instead.

Since this landed, we're having major problems with our popups panel, including keystrokes causing the entire browser to hide as well as the panel not hiding properly. It seems like any modification to the open panel causes the browser to hide. This can be seen with this add-on: https://addons.mozilla.org/en-US/firefox/addon/webde-mailcheck/

Can you file a new bug so we can investigate please? cc me of course.