User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.7; rv:15.0) Gecko/15.0 Firefox/15.0a1 Build ID: 20120505040218 Steps to reproduce: I quit Firefox when in Lion Fullscreen mode. Actual results: When I restarted Firefox, it opened in windowed mode. Expected results: Firefox should start in Fullscreen mode if it was in Fullscreen mode when the user exited (or restarted) Firefox.
I can confirm this. In fact Firefox can't open a window in Lion fullscreen mode. You see this when you quit Firefox in fullscreen mode and restart it. You also see it when you close a fullscreen window and then open another window. In both cases the window is maximized but not fullscreen. Furthermore Firefox still *thinks* it's fullscreen. So you need to issue the fullscreen "command" twice to make the window fullscreen again. This is true only of the Lion fullscreen mode -- not of the fullscreen mode used on OS X 10.6 and 10.5.
This is a pretty bad bug, but I'm not sure it should block the first appearance of the Lion-specific fullscreen mode (currently scheduled for FF 14). What do you think, Alex?
Tracking for FF14 and FF15 - we'll uplift depending on when a low risk patch is available.
(if a low risk fix isn't found, this wouldn't block FF14's release, or cause the new feature to be backed out)
I've figured out the underlying cause of this bug: When opening a new fullscreen window, cross-platform code (nsGlobalWindow::SetFullScreen()) tries to make the window fullscreen before it's been made visible (before nsCocoaWindow::Show() has been called on it). Fixing this isn't going to be easy, or low risk. There's a hacky workaround (calling the undocumented [mWindow setVisible:] in nsCocoaWindow::MakeFullScreen() if mWindow isn't yet visible). But this triggers annoying glitches in the animated transition to fullscreen mode. So I'm afraid FF 14 is going to have to go out with this bug still in it.
Forgot to mention that the following error shows up in the Console when FF fails to open a window in fullscreen mode: *** Assertion failure in -[_NSWindowFullScreenTransition makeAndSetupOverlayWindow], /SourceCache/AppKit/AppKit-1138.32/AppKit.subproj/NSWindowFullScreenTransition.m:671 This is what happens when you try to use [NSWindow toggleFullScreen:] to put an invisible window in fullscreen mode.
Another thing: There's no way to tell that [mWindow toggleFullScreen:] has failed (the OS never calls [WindowDelegate windowDidFailToEnterFullScreen:]). So we don't have any way to reset mFullScreen appropriately when the failure happens.
> So we don't have any way to reset mFullScreen appropriately when the failure > happens. Actually I suppose we could just fail in nsCocoaWindow::MakeFullScreen() when mWindow is invisible. That would fix part of this bug. I'll submit a patch tomorrow.
Created attachment 634562 [details] [diff] [review] Handle failure to enter fullscreen mode (partial fix) This patch follows my suggestion from comment #8. It doesn't fix this bug. But it does get rid of the confusion that would otherwise result when Firefox fails to enter fullscreen mode: With this patch, you only need one "enter fullscreen mode" command to recover from the bug. The patch is very simple and should be low-risk.
Comment on attachment 634562 [details] [diff] [review] Handle failure to enter fullscreen mode (partial fix) LGTM
Landed on mozilla-inbound: http://hg.mozilla.org/integration/mozilla-inbound/rev/5a7d3fce7893 This is only a partial fix, so I've put [leave open] in the whiteboard.
I know I'm a bit behind here, but the work I started to do in the last patch in bug 714911 may help too. I was tracking calls to MakeFullScreen that came in too early, and then calling MakeFullScreen when the window was ready.
Comment on attachment 634562 [details] [diff] [review] Handle failure to enter fullscreen mode (partial fix) [Approval Request Comment] Bug caused by (feature/regressing bug #): MakeFullScreen called prematurely from cross-platform code User impact if declined: Users will have to click twice to re-enter fullscreen mode Testing completed (on m-c, etc.): By myself and a little on m-c Risk to taking this patch (and alternatives if risky): Low risk String or UUID changes made by this patch: none As noted above (in comment #9) this patch doesn't fix the bug, but it does mitigate it. With this patch, Firefox will no longer be confused about what state the window is in. It's not the end of the world if the window state confusion isn't fixed in Firefox 14. But I think it makes fullscreen mode look flaky -- even more so than the original bug. So I think we should try to get this into FF 14. The patch hasn't yet baked on mozilla-central. But it'll get much more exposure to user testing on aurora and beta. So if there are going to be problems, we'll be more likely to find them out before the FF 14 release.
Comment on attachment 634562 [details] [diff] [review] Handle failure to enter fullscreen mode (partial fix) Approving for Aurora then, let's get more user coverage here and then see if this makes sense to take on Beta.
Comment on attachment 634562 [details] [diff] [review] Handle failure to enter fullscreen mode (partial fix) Landed on aurora: http://hg.mozilla.org/releases/mozilla-aurora/rev/6d28ccd8dde5 Because this bug isn't yet fully fixed, I'm not changing the aurora status.
Comment on attachment 634562 [details] [diff] [review] Handle failure to enter fullscreen mode (partial fix) [Triage Comment] Let's also land for our next Beta, to make sure it makes it in.
Comment on attachment 634562 [details] [diff] [review] Handle failure to enter fullscreen mode (partial fix) Landed on mozilla-beta: http://hg.mozilla.org/releases/mozilla-beta/rev/324e72553ab4
Lukas: The original bug hasn't (yet) been fixed. My patch from comment #9 (attachment 634562 [details] [diff] [review]) just ameliorates its effects. That's why I've been holding off setting status-firefox14 and status-firefox15 to "fixed". Don't know if that messes up your tracking system, though.