Open Bug 539597 Opened 15 years ago Updated 2 years ago

Make SessionStore handle full screen mode

Categories

(Firefox :: Session Restore, defect, P3)

defect

Tracking

()

People

(Reporter: zpao, Unassigned)

References

(Blocks 1 open bug)

Details

Private browsing needs SessionStore (see bug 538715 and bug 500150) to handle full screen mode.

I've been working on this & it's mostly an easy change. However I've run into some sizemode issues (at least on m-c & OSX). I'll be filing that bug and it should probably block this.
Blocks: 538715, 500150
Any updates here?
No updates. I haven't gone back to finish the OSX widget bug (bug 539601).
Assignee: paul → nobody
i have this too (mac os 10.9 / firefox 38.0.5)
+1

With 10.11 releasing even more improvements to full-screen (e.g. side by side fullscreen apps), I feel that it is very important to handle this. Apple markets this feature enormously. http://www.apple.com/osx/whats-new/

Add to the fact that Firefox has to be the only app installed on my computer that doesn't resume fullscreen. Super annoying!
This is probably the issue in Fx that annoys me the most, and I’d love to help diagnose / fix it. I’m really not a CPP coder and I don’t know the Fx codebase well at all, but I was trying to work out what might be going on and found a potential logic bug. In nsXULWindow.cpp there’s a line that seems to be checking for whether the sizemode should be persisted:

https://dxr.mozilla.org/mozilla-central/source/xpfe/appshell/nsXULWindow.cpp#1704

However shouldPersist is earlier only set to true if the owner document isn’t fullscreen:

https://dxr.mozilla.org/mozilla-central/source/xpfe/appshell/nsXULWindow.cpp#1652

If this is where the problem is then presumably line 1704 could be changed to check for (ownerXULDoc && persistString.Find("sizemode") >= 0) ?

I'm going to close some bugs as duplicates of this one, while trying to capture any helpful info as comments here:

From bug 1423401 comment 10:

(In reply to Xidorn Quan [:xidorn] UTC+10 from comment #10)

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.

From bug 1423401 comment 11:

(In reply to Xidorn Quan [:xidorn] UTC+10 from comment #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

Is there any plan to fix it? I mean, it's a bug that lasts for more than half the life of Firefox.

Severity: normal → S3

The severity field for this bug is relatively low, S3. However, the bug has 7 duplicates and 26 votes.
:dao, could you consider increasing the bug severity?

For more information, please visit auto_nag documentation.

Flags: needinfo?(dao+bmo)

The last needinfo from me was triggered in error by recent activity on the bug. I'm clearing the needinfo since this is a very old bug and I don't know if it's still relevant.

Flags: needinfo?(dao+bmo)
Priority: -- → P3
You need to log in before you can comment on or make changes to this bug.