Steps to reproduce (for me at least): 1. Open Thunderbird 2. Close the Thunderbird window (but do not exit the program) 3. Open a new Thunderbird window (e.g., click on the dock icon) 4. Click on Thunderbird->About Thunderbird At this point, one of two things happen: 1. The menu closes, but nothing else happens OR 2. Thunderbird crashes. If #1 happens, the program has always crashed after I clicked About Thunderbird a second time. Usually I will be asked to submit a crash report, but occasionally I will just get an OS error message saying Thunderbird closed unexpectedly. I've included a link to me latest crash report in the URL field. Maybe related, maybe a different bug: After closing the Thunderbird window (so no windows are open, but the program is still running), clicking About Thunderbird closes the menu, but otherwise does absolutely nothing.
Can you retrieve the crash report id? Instructions for Thunderbird at https://support.mozillamessaging.com/en-US/kb/Mozilla-Crash-Reporter#w_viewing-crash-reports
This is probably a dup of bug 670914 (reported for Thunderbird). It also looks related to bug 704866 (reported for Firefox). But the latter really does seem to be "fixed" -- there are very few of these crashes in Firefox since my patch for bug 704866 landed on the 11 branch. As I said at bug 670914 comment #5, it's really hard to make any progress on these crashes if we're not able to reproduce them. Can anyone else besides the reporter reproduce his STR from comment #0?
Hooray! I can repro with today's comm-central nightly! I'll get to work on this as soon as I can (this week or next).
> This is probably a dup of bug 670914 (reported for Thunderbird). Please don't dup this bug to bug 670914. Only this bug has true STR. So it's more convenient to work on it here.
I've been working on this bug for several days now. But it has many layers, and every time I peel off one layer I find another one. This bug is a recent regression -- the STR from comment #0 only starts working as of the following comm-central nightly: thunderbird-2011-21-21-03-00-22-comm-central The proximate cause of the crashes is that, as of this nightly, the "hidden window" no longer has a menubar. But I can't tell which of the patches in this range caused this to start happening, so I'll need to hg bisect.
Here's the patch that triggered this bug: Bug 644169 - Implement Tabs on Top for Thunderbird. ui-r=bwinton,r=sid0,sr=Standard8. author Mike Conley <firstname.lastname@example.org> Tue Dec 20 17:12:32 2011 -0500 (at Tue Dec 20 17:12:32 2011 -0500) http://hg.mozilla.org/comm-central/rev/3f507b7d7ee6 Mike, do you know how your patch stopped the "hidden window" from having a menubar?
Hey Steven, Hm. So we moved the menubar inside a toolbox named "navigation-toolbox" in mailWindowOverlay.xul. I'm not 100% sure which hidden window you're referring to. Is it defined somewhere in TB's XUL? -Mike
The "hidden window" is Mac-specific, I believe. I'm not entirely sure how it works (or what it's for). But Cocoa widget code assumes its existence, and assumes that it has a menubar. Here are two definitions for it -- one for Firefox and one for Thunderbird: ./mozilla/browser/base/content/hiddenWindow.xul ./mail/base/content/hiddenWindow.xul
Note that Firefox's "hidden window" still has a menubar (so that the STR from comment #0 doesn't work with current trunk Firefox).
Hm - so it looks like hiddenWindow.xul is hooking into mail-toolbox, which no longer has the menubar: http://mxr.mozilla.org/comm-central/source/mail/base/content/hiddenWindow.xul#106 It's possible that hiddenWindow.xul wants navigation-toolbox there instead. Does this resolve the crash?
> It's possible that hiddenWindow.xul wants navigation-toolbox there instead. > > Does this resolve the crash? I'll try this and let you know my results.
Created attachment 601372 [details] [diff] [review] Proposed fix (Following up comment #14) It worked! This change restores the hidden window's menubar, and gets rid of this bug's crashes. The reason for the crashes is that a static native "application menu" (sApplicationMenu in mozilla/widget/cocoa/nsMenuBarX.mm) is created whenever the hidden window's menubar is created, which in some sense "belongs" to the hidden window's menubar (it shares some resources with it). This application menu is "static" in the sense that it's created only once, but is passed around as needed among all the browser windows. So it needs to live as long as the application itself (i.e. as long as the hidden window). If the hidden window doesn't have a menubar, sApplicationMenu instead gets created when the first browser window's menubar gets created, and goes out of scope when that window (and its menubar) is destroyed -- causing crashes and other problems. In my brief tests this patch caused no problems. I paid particular attention to the "application menu" -- which is the leftmost menu after the Apple menu (the "Daily" menu in comm-central builds). But I don't know enough about Thunderbird (or about the hidden window in any application) to be sure I didn't miss something. Who should review my patch? Should I ask you to do it, Mike? I've started a tryserver build, which should be available in a few hours.
Sure, go ahead and r? me.
Thanks for working on this, by the way. :)
Comment on attachment 601372 [details] [diff] [review] Proposed fix > Thanks for working on this, by the way. :) You're quite welcome. Thank you for figuring out how to give the hidden window back its menubar.
Comment on attachment 601372 [details] [diff] [review] Proposed fix This looks good to me, and I can confirm that the crash is fixed.
6 years ago
6 years ago
6 years ago
Comment on attachment 601372 [details] [diff] [review] Proposed fix We're definitely going to want this for TB 11 and 12.
Comment on attachment 601372 [details] [diff] [review] Proposed fix Yep, please land it asap.
Committed to: comm-central as http://hg.mozilla.org/comm-central/rev/82295114c5a6 comm-aurora as http://hg.mozilla.org/releases/comm-aurora/rev/315ceda5ffe6 comm-beta as http://hg.mozilla.org/releases/comm-beta/rev/3a8256a08f2d Thanks for your work, Steven! You rock!
(In reply to Steven Michaud from comment #15) > If the hidden window doesn't have a menubar, sApplicationMenu instead > gets created when the first browser window's menubar gets created, and > goes out of scope when that window (and its menubar) is destroyed -- > causing crashes and other problems. Well, shouldn't we fail in a more obvious manner then? e.g. don't display any menu if the hidden window doesn't have a menu.
(In reply to comment #23) No, not really. All the "hiddenWindow" objects contain comments indicating it needs a menubar. Mike missed this in his patch (because he didn't actually change any of the hiddenMenu objects). This caused problems in Cocoa widget code that led to the bug being given to someone (me) who could figure it out from the Cocoa widget side. This would all still have happened if the symptom had been a missing menu. Though I admit it might have saved me a few hours figuring out the problem. This is yet another example (and there are hundreds) of an implicit "API" governing relations between different parts of the tree. As long as it isn't "public", it doesn't really need to be formalized. Though you do need comments explaining it -- which in fact we already have.
(In reply to Steven Michaud from comment #24) > (In reply to comment #23) > > No, not really. > > All the "hiddenWindow" objects contain comments indicating it needs a > menubar. Mike missed this in his patch (because he didn't actually > change any of the hiddenMenu objects). This caused problems in Cocoa > widget code that led to the bug being given to someone (me) who could > figure it out from the Cocoa widget side. > > This would all still have happened if the symptom had been a missing > menu. Though I admit it might have saved me a few hours figuring > out the problem. Or, well, some text in the menu bar saying "No menu found, check that your hiddenWindow has a menu". My point is that from the way you describe it, hiddenWindow without a menu is a thoroughly broken state anyway, so any attempts to fix it are only going to make it worse.
> My point is that from the way you describe it, hiddenWindow without > a menu is a thoroughly broken state anyway, so any attempts to fix > it are only going to make it worse. I don't understand. What do you mean by "any attempts to fix it are only going to make it worse"? Cocoa widget code assumes that a hidden window will always exist, and that it will have a menubar. I don't see anything wrong with this. Similar assumptions are made all over the tree.
Ah, I see. Never mind.