1423401 - Remember fullscreen on MacOS

Status: RESOLVED DUPLICATE of bug 539597

(Core :: Widget: Cocoa, defect, P3)

Description

[Patrick Albrecht]

Firefox should remember if the user quit the app in fullscreen as Apple apps do it (e.g. Mail.app or Safari.app). (as https://discussions.apple.com/thread/4140495?tstart=0 shows, see the recommended answer)

I don't think that this is an issue, rather it's a change of an existing behavior.

Comment 1

[Stephen A Pohl [:spohl]]

Has this ever worked? Would you mind running mozregression[1] to find out?

[1] http://mozilla.github.io/mozregression/

Comment 3

[Patrick Albrecht]

As the other reporter already meant; I don't think that it's a recent regression and I would probably need to go back years when Firefox was probably not even compatible with MacOS High Sierra which means a lot of other issues could happen or prevent Firefox from running. Do you still want me to try to find out a regression?

Comment 4

[Stephen A Pohl [:spohl]]

No, this will do. Thanks!

Comment 5

[Gabriel Vîjială [:gabriel-v]]

I would like to work on this. It's annoying when using nightly and having to go back into fullscreen to after every update restart.

1. Is nsWidgetInitData the place to put the flag that remembers if the window was in full screen mode when it was closed?
2. Is nsCocoaWindow::Create the place to check that flag and call MakeFullScreen/MakeFullScreenWithNativeTransition?
3. Where would a test for this feature live?

Comment 6

[Markus Stange [:mstange]]

I'm not sure - do we already persist fullscreen mode on other platforms, or does this need to be implemented for all platforms?

Comment 7

[Xidorn Quan [:xidorn] UTC+11]

We explicitly avoid persisting fullscreen mode, because it's tricky to make it work properly. I forgot the details but iirc macOS is the trickiest one.

Comment 8

[Xidorn Quan [:xidorn] UTC+11]

See bug 1137009. There are two issues:
1. when exit from dom fullscreen (e.g. fullscreen video) we would enter fullscreen mode when restart
2. when in fullscreen (either fullscreen mode or dom fullscreen) open new window would be in fullscreen as well

I also kinda recall that we opening a fullscreen window on mac has some weird behavior, but couldn't find what it was from comments.

Comment 9

[Patrick Albrecht]

From Safari behaviour:
1. Maybe the Webkit code could be here an inspiration, I only tested Safari but I strongly assume Webkit also works "correct" (= exit with a fullscreen video opens the browser in non-fullscreen the next time)
2. I think this could be tolerated as Safari already opens a new window in full screen if the user is in full screen. The only thing is that they map command + t & command + n to "open a new tab" if you're in full screen. The user has to use option + command + n to explicitly open a new (full screen) window. Might be something to consider for Firefox?

Comment 10

[Xidorn Quan [:xidorn] UTC+11]

It is not impossible to distinguish fullscreen mode from dom fullscreen somehow, so it's not a real blocker, but it's only part of the problem.

I think the main problem was that Firefox cannot properly open in fullscreen (i.e. opening window in fullscreen can cause UI breakage). That was mentioned in bug 946768, bug 1197416, and bug 1207674. The underlying issue was explored in bug 946768 comment 5.

That was a problem mainly on macOS (but there were also some mentions of it on Windows, like bug 1204332 and bug 1207674 comment 2), and I believe it was one of the main reasons, actually probably the most important one, that made me decide to stop persisting fullscreen state as a wholesale solution to similar issues.

I'm not sure how everything works nowadays, but I pretty much suspect it still doesn't work well.

This doesn't really matter whether other browsers can do this. There are quite a bit of internals involving in Gecko's widget code and Firefox frontend code. They may need to get harmonized somehow in order to have it work properly. Simply persisting the fullscreen state is unlikely to just work.

Comment 11

[Xidorn Quan [:xidorn] UTC+11]

If you want to explore the way down, you can probably start with removing the persisting condition in nsXULWindow::SavePersistentAttributes[1] and see what will happen.

It may work, it may not, but it should be a good start point for fixing it.


[1] https://searchfox.org/mozilla-central/rev/e9d2dce0820fa2616174396459498bcb96ecf812/xpfe/appshell/nsXULWindow.cpp#1830

Comment 12

[Robin Whittleton]

This is a duplicate of bug #539597 I think (or vice versa).

Comment 13

[Stephen A Pohl [:spohl]]

(In reply to Robin Whittleton from comment #12)

This is a duplicate of bug #539597 I think (or vice versa).

Agreed.