Firefox fails over to maximized when trying to open a window in Lion fullscreen mode

NEW
Assigned to

Status

()

Core
Widget: Cocoa
5 years ago
5 years ago

People

(Reporter: Johan Larsson, Assigned: smichaud)

Tracking

14 Branch
x86
Mac OS X
Points:
---

Firefox Tracking Flags

(firefox14+ fixed, firefox15+ fixed)

Details

(Whiteboard: [leave open])

Attachments

(1 attachment)

(Reporter)

Description

5 years ago
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.
(Assignee)

Comment 1

5 years ago
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.
Status: UNCONFIRMED → NEW
Component: Untriaged → Widget: Cocoa
Ever confirmed: true
Product: Firefox → Core
QA Contact: untriaged → cocoa
Summary: Lion Fullscreen mode is not resumed when restarting Firefox → Firefox can't open a window in Lion fullscreen mode
Version: 15 Branch → 14 Branch
(Assignee)

Updated

5 years ago
Assignee: nobody → smichaud
(Assignee)

Comment 2

5 years ago
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-firefox14: --- → ?
tracking-firefox15: --- → ?

Comment 3

5 years ago
Tracking for FF14 and FF15 - we'll uplift depending on when a low risk patch is available.
tracking-firefox14: ? → +
tracking-firefox15: ? → +

Comment 4

5 years ago
(if a low risk fix isn't found, this wouldn't block FF14's release, or cause the new feature to be backed out)
(Assignee)

Updated

5 years ago
Summary: Firefox can't open a window in Lion fullscreen mode → Firefox fails over to maximized when trying to open a window in Lion fullscreen mode
(Assignee)

Comment 5

5 years ago
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.
(Assignee)

Comment 6

5 years ago
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.
(Assignee)

Updated

5 years ago
See Also: → bug 741304
(Assignee)

Comment 7

5 years ago
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.
(Assignee)

Comment 8

5 years ago
> 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.
(Assignee)

Comment 9

5 years ago
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.
Attachment #634562 - Flags: review?(bgirard)
Comment on attachment 634562 [details] [diff] [review]
Handle failure to enter fullscreen mode (partial fix)

LGTM
Attachment #634562 - Flags: review?(bgirard) → review+
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.
Whiteboard: [leave open]
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.
https://hg.mozilla.org/mozilla-central/rev/5a7d3fce7893
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.
Attachment #634562 - Flags: approval-mozilla-beta?
Attachment #634562 - Flags: approval-mozilla-aurora?
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.
Attachment #634562 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
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.
Attachment #634562 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
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
status-firefox14: --- → fixed
status-firefox15: --- → fixed
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.
You need to log in before you can comment on or make changes to this bug.