The way non-browser windows get loaded is now a lot simpler without overlays, but I think there's still more that we could remove to make the loading process more straightforward and start paving the way to share this behavior with HTML windows.

Comment on attachment 8988347 [details] Bug 1471734 - Move OSX dock menu markup to hiddenWindow.xul; https://reviewboard.mozilla.org/r/253594/#review260338 ::: commit-message-9c7bb:4 (Diff revision 1) > +Bug 1471734 - Move OSX dock menu markup to hiddenWindow.xul;r=Gijs > + > +It's currently in macWindow.inc.xul which means it gets created for > +all non-browser windows, but it's only ever set up for the hidden window. Hrm. This took me some figuring out. Can we move the whole nonBrowserWindowStartup stuff out of browser.js into something more obviously correct / related to where this stuff lives? Doesn't need to happen here. ::: browser/base/content/hiddenWindow.xul:8 (Diff revision 1) > # > # This Source Code Form is subject to the terms of the Mozilla Public > # License, v. 2.0. If a copy of the MPL was not distributed with this > # file, You can obtain one at http://mozilla.org/MPL/2.0/. > > #ifdef XP_MACOSX Huh, why do we even ship this on non-mac?

Comment on attachment 8988348 [details] Bug 1471734 - Remove FULL_BROWSER_WINDOW preprocessor directive; https://reviewboard.mozilla.org/r/253596/#review260344 I don't think this is right. On current nightly, the menu *doesn't* appear in normal windows. It only appears if you close all the windows, or switch to a non-browser window like "About Firefox". Your patch makes it so it appears everywhere (also on non-mac) and disables it in the non-browser windows like the hidden window and "About Firefox".

Comment on attachment 8988349 [details] Bug 1471734 - Remove FULL_BROWSER_WINDOW preprocessor directive; https://reviewboard.mozilla.org/r/253598/#review260348 This seems like it's the right way around, though it'll need rebasing if we drop the middle cset...

(In reply to :Gijs (he/him) from comment #5) > Comment on attachment 8988348 [details] > Bug 1471734 - Remove MAC_NON_BROWSER_WINDOW preprocessor directive; > > https://reviewboard.mozilla.org/r/253596/#review260344 > > I don't think this is right. On current nightly, the menu *doesn't* appear > in normal windows. It only appears if you close all the windows, or switch > to a non-browser window like "About Firefox". > > Your patch makes it so it appears everywhere (also on non-mac) and disables > it in the non-browser windows like the hidden window and "About Firefox". Thank you - I misread the condition here. I think we should set it to hidden in the markup and then un-hide it for non-browser windows.

(In reply to :Gijs (he/him) from comment #4) > Comment on attachment 8988347 [details] > Bug 1471734 - Move OSX dock menu markup to hiddenWindow.xul; > > https://reviewboard.mozilla.org/r/253594/#review260338 > > ::: commit-message-9c7bb:4 > (Diff revision 1) > > +Bug 1471734 - Move OSX dock menu markup to hiddenWindow.xul;r=Gijs > > + > > +It's currently in macWindow.inc.xul which means it gets created for > > +all non-browser windows, but it's only ever set up for the hidden window. > > Hrm. This took me some figuring out. Can we move the whole > nonBrowserWindowStartup stuff out of browser.js into something more > obviously correct / related to where this stuff lives? Doesn't need to > happen here. For sure. I'm hoping to figure this out and simplify it as part of the work tracked in Bug 1453783. > ::: browser/base/content/hiddenWindow.xul:8 > (Diff revision 1) > > # > > # This Source Code Form is subject to the terms of the Mozilla Public > > # License, v. 2.0. If a copy of the MPL was not distributed with this > > # file, You can obtain one at http://mozilla.org/MPL/2.0/. > > > > #ifdef XP_MACOSX > > Huh, why do we even ship this on non-mac? It's not referenced on non-mac (https://searchfox.org/mozilla-central/rev/ed2763bea882619ccb48b0aecc54e523d2bdd2ae/xpfe/appshell/nsAppShellService.cpp#126-129) so the the preprocessor directive here seems removable. It looks like Bug 71895 (!) is opened to remove it from the build as per: https://searchfox.org/mozilla-central/rev/ed2763bea882619ccb48b0aecc54e523d2bdd2ae/browser/base/jar.mn#104.

Was thinking to add a test to make sure the openLocation menu item is hidden in browser windows and unhidden in non-browser windows but I couldn't think of a good place to put it. This is the type of thing I would have usually dumped in general/... maybe a new folder like 'windowing' or something? Or can you think of a good existing place to put it?

Moving the ni? over into Bug 1472751.

MozReview re-flagged you for review on the FULL_BROWSER_WINDOW part. That's just a rebase after part 2 was pulled out into another bug.

Comment on attachment 8988348 [details] Bug 1471734 - Remove FULL_BROWSER_WINDOW preprocessor directive; https://reviewboard.mozilla.org/r/253596/#review261122 r=me per comment that this is just a rebase. :-)

Pushed by bgrinstead@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/98c95a5cac9d Move OSX dock menu markup to hiddenWindow.xul;r=Gijs https://hg.mozilla.org/integration/autoland/rev/6899be9874e1 Remove FULL_BROWSER_WINDOW preprocessor directive;r=Gijs

https://hg.mozilla.org/mozilla-central/rev/98c95a5cac9d https://hg.mozilla.org/mozilla-central/rev/6899be9874e1