512575 - Switching out of fullscreen mode and alt-tabbing puts me back in fullscreen mode

Status: RESOLVED FIXED

(Core :: Widget: Gtk, defect)

Description

[Daniel Holbert [:dholbert]]

STEPS TO REPRODUCE:
 1. Press F11 to enter full-screen mode
 2. Press F11 to exit full-screen mode
 3. Alt-tab to another window and then back to Firefox
    OR... drag Firefox around by its titlebar
    OR... resize your Firefox window

ACTUAL RESULTS:
 Firefox enters full-screen mode again.  (Also: it takes *two* F11 presses to leave the newly-entered full-screen mode.)

Regression window:
Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.3a1pre) Gecko/20090814 Minefield/3.7a1pre
Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.3a1pre) Gecko/20090815 Minefield/3.7a1pre
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=e5d56aa2b693&tochange=2c0a1af45de2

In that range, Bug 510546's checkin ("support for sizemode=fullscreen on gtk2.") stands out as the most likely cause.

Comment 1

[Daniel Holbert [:dholbert]]

This bug is SUPER annoying, as there's no obvious way to stop it from happening once it's started happening.  Every time I switch windows, Firefox force-resizes itself. Once this has started happening, it persists when I restart Firefox, too.

The only way I've found to recover normal behavior is to minimize & restore Firefox.

Comment 2

[BUGabundo]

not yet fixed
3.7~a1~hg20090825r31809+nobinonly-0ubuntu1~umd1
Rev 31809

Comment 3

[BUGabundo]

possibly related to bug 502133
https://bugzilla.mozilla.org/show_bug.cgi?id=502133

Comment 4

[Doug Turner (:dougt)]

Attachment: patch v.1

dholbert - sorry about the bustage.  do you want to try this?

Comment 6

[Daniel Holbert [:dholbert]]

Comment on attachment 396611 [details] [diff] [review]
patch v.1

Doug -- patch v.1 does indeed fix the problem in my debug build.  Thanks for the quick action on this! :)

Comment 7

[Daniel Holbert [:dholbert]]

Comment on attachment 396611 [details] [diff] [review]
patch v.1

>+    else {
>+        mSizeMode = nsSizeMode_Maximized;
>         gdk_window_unfullscreen (mShell->window);
>+    }

Discussed this with Doug briefly -- we should probably be restoring the *true* old mSizeMode here, rather than forcing nsSizeMode_Maximized.

Because of the forced "nsSizeMode_Maximized" in the existing patch, it causes the following bug:
  1. Open a "normal" (non-maximized) Firefox window with a few tabs
  2. F11 twice, to enter & exit full-screen mode
    (NOTE: we somehow restored the "normal" state correctly, so far)
  3. Quit and re-launch Firefox, saving session if prompted
   --> BUG: The newly launched Firefox process is full-screen, even though it's being restored from a "normal" size saved session.

Comment 8

[Daniel Holbert [:dholbert]]

(In reply to comment #7)
>    --> BUG: The newly launched Firefox process is full-screen, even though it's
> being restored from a "normal" size saved session.

Sorry -- I meant "the newly launched Firefox process is *maximized*" (not full-screen)

Comment 9

[Doug Turner (:dougt)]

Attachment: patch v.2

Comment 10

[Daniel Holbert [:dholbert]]

Comment on attachment 396807 [details] [diff] [review]
patch v.2

> #if GTK_CHECK_VERSION(2,2,0)
>     if (aFullScreen) {
>+        mLastSizeMode = mSizeMode;

Before the assignment, can we assert that mSizeMode != nsSizeMode_Fullscreen here?

>+    else {
>+        mSizeMode = mLastSizeMode;
>         gdk_window_unfullscreen (mShell->window);

And likewise here before the assignment, can we assert that mLastSizeMode != nsSizeMode_Fullscreen?

Except we can't actually assert that, because Firefox could call MakeFullScreen(PR_TRUE) twice, and that would end up making us store nsSizeMode_Fullscreen in mLastSizeMode.

So maybe we should add those assertions *and* a check to ensure that, if MakeFullScreen(PR_TRUE) is called when mSizeMode is already nsSizeMode_Fullscreen, we leave mLastSizeMode untouched.

r=dholbert with those changes.

Comment 11

[Doug Turner (:dougt)]

Attachment: patch v.3

Comment 12

[Daniel Holbert [:dholbert]]

Comment on attachment 396836 [details] [diff] [review]
patch v.3

>+    NS_ASSERTION(mLastSizeMode != nsSizeMode_Fullscreen, "mLastSizeMode should never be fullscreen");

Add a linebreak before the warning message, to bring that line < 80 char.

r=dholbert with that.

Comment 13

[Doug Turner (:dougt)]

http://hg.mozilla.org/mozilla-central/rev/8dc97b8f087f

Comment 14

[Doug Turner (:dougt)]

before this lands on 1.9.2, bug 510546 needs to land first.

Comment 15

[Dão Gottwald [:dao]]

Oh, right, I thought it already had.

Comment 16

[Doug Turner (:dougt)]

http://hg.mozilla.org/releases/mozilla-1.9.2/rev/20239c573bd8