In 3.6.4, this bug manifests normal full sized window on the desktop. In the recent titlebar work we did, we fixed this so we actually go into full screen mode on restart. Widget sends a size mode event, but for some reason the browser down't pick up on it, so the UI is the normal browser. STR: 1) open 2) F11, go to fullscreen 3) close 4) restart To get out of this weird normal browser/fullscreen mode, hit F11 again.

These patches fix a few bugs and improve one issue related to full screen. Xul windows persist size modes, so when the user is in full screen mode and opens a new window, or if they shut down in fs and relaunch, the new window should be in fs mode. In 3.6 this doesn't work. On windows what you will get is a normal window that is sized to the size of the desktop. This same problem *was* in m-c builds until a patch for the new titlebar landed this weekend fixing the "big window" problem. This didn't fix things entirely though - widget currently isn't sending a size mode event when we enter full screen mode [bug #1, widget patch]. nsXULWindow also doesn't propagate fs size mode events to the dom via calls to global window [bug #2, xpfe + dom patches]. So there's no connection on startup between widget's size mode changes and browser.js. Also, browser.js isn't hooked up the "fullscreen" event soon enough [bug #3, browser patch] to receive it on initial startup. The last change, which is in the xpfe patch, makes it so browser windows with persisted fs mode opened on top of existing full screen windows open normally, which is a standard windows convention.

Comment on attachment 455080 [details] [diff] [review] appshell patch >+#ifdef XP_WIN Any reason this is Windows-specific? >+ // If the parent is currently fullscreen, tell the child to ignore persisted >+ // full screen states. This way new browser windows open on top of fullscreen >+ // windows normally. What if some other window opens a browser window? >+ reinterpret_cast<nsXULWindow*>(window.get())->IgnoreXULSizeMode(PR_TRUE); Doesn't nsWebShellWindow derive from nsXULWindow?

Comment on attachment 455081 [details] [diff] [review] dom patch This looks like something which should happen in widget level. MakeFullScreen should just do the right thing, no?

(In reply to comment #6) > (From update of attachment 455080 [details] [diff] [review]) > >+#ifdef XP_WIN > Any reason this is Windows-specific? > > >+ // If the parent is currently fullscreen, tell the child to ignore persisted > >+ // full screen states. This way new browser windows open on top of fullscreen > >+ // windows normally. > What if some other window opens a browser window? It works great, if you have a full screen browser and open a new with ctrl-n, it opens normal. Prior to this patch it would open full screen on top of the first full screen window.

(In reply to comment #7) > (From update of attachment 455081 [details] [diff] [review]) > This looks like something which should happen in widget level. > MakeFullScreen should just do the right thing, no? There's circular call risk here, I put this in to protect against that on other platforms. win32 widget handles this. So if we're sure widget -> send size mode to web shell web shell -> global window -> fullscreen global window -> widget -> makefullscreen Is safe, we don't need it.

- nix over zealous castings

(In reply to comment #8) >(In reply to comment #6) >>(From update of attachment 455080 [details] [diff] [review] [details]) >>>+#ifdef XP_WIN >>Any reason this is Windows-specific? No answer to this question? >>>+ // If the parent is currently fullscreen, tell the child to ignore persisted >>>+ // full screen states. This way new browser windows open on top of fullscreen >>>+ // windows normally. >>What if some other window opens a browser window? >It works great, if you have a full screen browser and open a new with ctrl-n No, I said if some *other* window opens a browser window...

(In reply to comment #11) > (In reply to comment #8) > >(In reply to comment #6) > >>(From update of attachment 455080 [details] [diff] [review] [details] [details]) > >>>+#ifdef XP_WIN > >>Any reason this is Windows-specific? > No answer to this question? Sorry, missed that. Actually I was kind of hoping that would get fleshed out through reviews or by asking around to see what the convention is on other platforms. > > >>>+ // If the parent is currently fullscreen, tell the child to ignore persisted > >>>+ // full screen states. This way new browser windows open on top of fullscreen > >>>+ // windows normally. > >>What if some other window opens a browser window? > >It works great, if you have a full screen browser and open a new with ctrl-n > No, I said if some *other* window opens a browser window... Hmm, good question. Bookmarks comes to mind. I'll test and see. Maybe I should walk across a list of all open top level windows looking for one that is full screen as a test.

Comment on attachment 455179 [details] [diff] [review] appshell patch v.2 I can improve on this slightly, but it'll likely be after the summit before I post a new rev.

Comment on attachment 457505 [details] [diff] [review] appshell patch v.3 Updated to walk the list of open windows.

I didn't realize the resulting window was unusable, we need to at least restore with a usable window.

Comment on attachment 457505 [details] [diff] [review] appshell patch v.3 >+ PRBool more; >+ do { >+ windowList->HasMoreElements(&more); >+ if (!more) >+ return PR_FALSE; This ... >+ nsIWidget* widget = nsnull; nsCOMPtr should work on nsIWidget, no? Saves manually releasing. >+ } while (more); ... means that this is always true. So you can just write for (;;) { PRBool more = PR_FALSE; etc. >+ window.get()->IgnoreXULSizeMode(PR_TRUE); Nit: .get() shouldn't be necessary I should be able to give this a whirl soon now that I've got a working build again.

Comment on attachment 455082 [details] [diff] [review] browser patch The event callback needs the height of the toolbox, which can change during delayedStartup.

gavin, ping?

Comment on attachment 460219 [details] [diff] [review] browser patch enterFS is a bit confusing, maybe use "inFullscreen"? I know it's not exactly accurate in the transition case, but I don't really think that's a problem.

(In reply to comment #21) > Comment on attachment 460219 [details] [diff] [review] > browser patch > > enterFS is a bit confusing, maybe use "inFullscreen"? I know it's not exactly > accurate in the transition case, but I don't really think that's a problem. What's confusing, "enter" or "FS"? I actually care about the accuracy, as the vague semantics of just "fullScreen" confused me when I wrote the patch.

"enterFS" seems to describe the transition as opposed to the end-state that the controls are being set up for, which I thought was kind of odd. But I don't feel strongly - just leave it as is if you want.

State names are traditionally gerunds or participles/adjectives (consider TCP closing and closed), not imperative-mood verbs like "enter". Sorry to be a nosy bystander but I'm wondering whether entering or entered is best. What's really going on here? For the ending, -FullScreen instead of -FS seems winning, but maybe the whole name is then too long. /be

The function runs both before the transition (in the normal "toggle full screen for a given window" case) or after (in the case where a window is restored as a fullscreen window on startup), so entered/entering can both be wrong for a given invocation. That's why I suggested "inFullScreen" - describes the final state that we're preparing the UI for.

The function runs in two different states. Does the distinction really not matter? /be

I'm not sure what you're asking. It either updates the UI to match the current state in the post-transition invocation, or updates the UI to match the future state in the pre-transition invocation. In both cases the state it's using to update the UI is reflected by the boolean.

current state : post-transition invocation :: future state : pre-transition It all makes sense but it's hard to name something that has two "tenses" :-|. /be

http://hg.mozilla.org/mozilla-central/rev/bb9d266da12c http://hg.mozilla.org/mozilla-central/rev/0b6d45b6c2e6 http://hg.mozilla.org/mozilla-central/rev/a179e301dbfd