513850 - ensure correct sizemode is used when leaving fullscreen

Status: RESOLVED WORKSFORME

(Core :: Widget: Win32, defect)

Description

[Doug Turner (:dougt)]

Attachment: patch v.1

when leaving fullscreen mode, we want to ensure that we restore the correct sizemode

Comment 1

[Brad Lassey [:blassey] (use needinfo?)]

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

>   mPopupType            = ePopupTypeAny;
>   mDisplayPanFeedback   = PR_FALSE;
>+  mLastSizeMode         = nsSizeMode_Normal;
and
>+  // Remember the last sizemode so that we can restore it when
>+  // leaving fullscreen
>+  nsSizeMode            mLastSizeMode;
>+
> #ifdef WINCE_WINDOWS_MOBILE
>   nsCOMPtr<nsIRegion>   mInvalidatedRegion; 
> #endif

It looks like we're only using this variable for windows mobile, so either ifdef around it or use it on desktop

Comment 2

[Doug Turner (:dougt)]

Attachment: patch v.2

Comment 3

[Brad Lassey [:blassey] (use needinfo?)]

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


>   else {
>+    mSizeMode = mLastSizeMode;

I think you need to test mSizeMode == nsSizeMode_Fullscreen here, otherwise we could change the sizemode when we don't really want to

Comment 4

[Doug Turner (:dougt)]

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

how about I add:

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

Comment 5

[Doug Turner (:dougt)]

09:28 < blassey> basically, we only want to change mSizemode in MakeFullScreen if we're 
                 transitioning into or out of fullscreen
09:29 < blassey> my window is mazimized, then I go fullscreen, then I go normal
09:29 < blassey> then someone calls MakeFullScreen(FALSE)
09:29 < blassey> mSizemode = normal, mLastSizemode = maximized going into the function
09:29 < blassey> and they shouldn't change
09:29 < blassey> but they will
09:30 < blassey> and you'll wind up maximized

Comment 6

[Doug Turner (:dougt)]

this also probably happens on gtk2.

Comment 7

[Doug Turner (:dougt)]

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

this patch isn't correct.

Comment 8

[Stuart Parmenter]

doug what is this effecting?

Comment 9

[Jim Mathies [:jimm]]

(In reply to comment #5)
> 09:28 < blassey> basically, we only want to change mSizemode in MakeFullScreen
> if we're 
>                  transitioning into or out of fullscreen
> 09:29 < blassey> my window is mazimized, then I go fullscreen, then I go normal
> 09:29 < blassey> then someone calls MakeFullScreen(FALSE)
> 09:29 < blassey> mSizemode = normal, mLastSizemode = maximized going into the
> function
> 09:29 < blassey> and they shouldn't change
> 09:29 < blassey> but they will
> 09:30 < blassey> and you'll wind up maximized

Either I can't reproduce this on a trunk build, or the patch that just landed for bug 520178 fixed this.

My steps to test:

1) start with a normalized or maximized main window.
2) hit f11
3) hit f11 again

the window always returns to the original state in step 1.

Comment 10

[Jim Mathies [:jimm]]

Win7 pinned window states work correctly as well.