1184201 - Screen flickers black and white when enter/exit DOM Fullscreen on Windows7 Classic

Status: VERIFIED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Alice0775 White]

[Tracking Requested - why for this release]:

Visually very annoying.  Bug 1160014 should be backed out!!!

Steps to Reproduce;
1. Windows7 Classic
2. enter/exit DOM Fullscreen

Actual Results:
Whole screen flickers black and white

Expected Results:
no black and white. just browser should be resized


Regression range:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=cc263dfb7b13&tochange=b867bb9c50ee

Regressed by: Bug 1160014

Comment 1

[Jim Mathies [:jimm]]

feedback on nightly / win7 / aero / cinema display

1) the transition into dom fullscreen looks really good to me. 
2) the transition out via 'esc' looks bad, I see the desktop window positioned on the desktop at the upper left of the screen before the screen goes black, then the screen goes black and I see a fade back with the browser window in the right position.
3) unlike #2, the transition I see when pressing the Deny button looks good. I don't see the desktop window positioned to the upper left of the display before the fade in.
3) transition into f11 fullscreen looks bad, I see the browser window lose all its chrome and go black before the transition completes.
4) transitioning out of f11 fullscreen look good.

https://developer.mozilla.org/samples/domref/fullscreen.html

This needs some polish work before it rolls out, I think we should block on this bug.

Comment 2

[Jim Mathies [:jimm]]

Also Windows classic desktop looks worse than aero. We need to do some testing there and with aero basic.

Comment 3

[Alice0775 White]

Attachment: screen capture

Whole screen(incl. desctop) flickers black and white

Comment 4

[Alice0775 White]

s/desctop/desktop/

Comment 5

[Olli Pettay [:smaug][[e-mail]]]

yikes, that is bad.

Comment 6

[Xidorn Quan [:xidorn] UTC+10]

Errr, It seems the transition window suddently disappears when the main window changes state... Let me try a bit to see how can we workaround this issue.

Note that it is not necessary to backout bug 1160014. If we decide this should not be shipped yet, we can disable the transition anytime via setting pref "full-screen-api.transition-duration.{enter,leave}" to "0 0" by default.

Comment 7

[Jim Mathies [:jimm]]

(In reply to Xidorn Quan [:xidorn] (UTC+12) from comment #6)
> Note that it is not necessary to backout bug 1160014. If we decide this
> should not be shipped yet, we can disable the transition anytime via setting
> pref "full-screen-api.transition-duration.{enter,leave}" to "0 0" by default.

excellent, thanks! if you need any help testing try builds please ni me.

Comment 8

[Xidorn Quan [:xidorn] UTC+10]

I can also reproduce this on Windows XP.

Comment 9

[Xidorn Quan [:xidorn] UTC+10]

Some observation on Windows XP:

For entering fullscreen, the browser window jumps in after we finish the fullscreen change on the main thread (after MakeFullScreen() returns, and before PerformFullscreenTransition() is called for the second stage). And I also noticed that, if there is any animation on the browser window, it could dig a hole on the transition window.

For exiting fullscreen, when SetWindowPos() is called on the browser window to leave fullscreen state, the desktop jumps in, and the black is limited to the client area of the browser window (could also cover some part of the taskbar). And like entering fullscreen, the content of the browser window displays after MakeFullscreen() returns, and before the second stage of transition starts.

My feeling is that, there is some bug on Windows that zorder is sometimes not respected during painting. And I have no idea how to workaround it. Any advice?

Comment 10

[Xidorn Quan [:xidorn] UTC+10]

If... it finally figures out that it is infeasible to workaround this issue, we probably could have a runtime check and disable the transition dynamically according to the desktop config.

Comment 11

[Jim Mathies [:jimm]]

I'm trying to test this a bit but I keep getting crashes here - 

https://crash-stats.mozilla.com/report/index/154e308f-e02d-4743-a9b8-4dbef2150717

usually shortly after I get into dom fullscreen or try to exit.

https://developer.mozilla.org/samples/domref/fullscreen.html

Comment 12

[Jim Mathies [:jimm]]

This was in 42.0a1 (2015-07-15)

Comment 13

[Xidorn Quan [:xidorn] UTC+10]

(In reply to Jim Mathies [:jimm] from comment #11)
> I'm trying to test this a bit but I keep getting crashes here - 
> 
> https://crash-stats.mozilla.com/report/index/154e308f-e02d-4743-a9b8-
> 4dbef2150717

This has been fixed in bug 1183651 which was just merged into m-c.

Comment 14

[Jim Mathies [:jimm]]

One thing I noticed, there's no check of the return result on PerformFullscreenTransition in nsGlobalWindow. Probably not a big deal but you might want to file a bug and 
add that in case something unexpected happens when creating the overlay window.

I'm only looking at one of the transitions currently:

STR

1) open the url below in a normal, non-maximized window
https://developer.mozilla.org/samples/domref/fullscreen.html

2) click the fullscreen button

result: Everything looks great, the desktop fades to black, then the browser window shows with the allow/deny dialog and a black video screen behind it.  

3) type escape

result:

- the video window disappears suddenly along with the allow/deny dialog leaving a maximized browser with tabs on the desktop
- the desktop and browser window fade to black leaving only the taskbar visible. (weird! Looks like we're re-animating the black overlay back in again.)
- once the desktop fades to black, it then faded back to the desktop more slowly displaying the original normalized browser on the desktop

Comment 15

[Jim Mathies [:jimm]]

I get the same thing if I click allow on the dialog, and then hit escape.

There's two animations when we're restore the window for some reason. That indicates to me there are bugs in the implementation here or Windows is doing something you don't expect which is caused by that AnimateWindow call.

Comment 16

[Xidorn Quan [:xidorn] UTC+10]

(In reply to Jim Mathies [:jimm] from comment #14)
> 3) type escape
> 
> result:
> 
> - the video window disappears suddenly along with the allow/deny dialog
> leaving a maximized browser with tabs on the desktop
> - the desktop and browser window fade to black leaving only the taskbar
> visible. (weird! Looks like we're re-animating the black overlay back in
> again.)
> - once the desktop fades to black, it then faded back to the desktop more
> slowly displaying the original normalized browser on the desktop

This is bug 1184443 which is unrelated to the issue here.

Comment 17

[Jim Mathies [:jimm]]

(In reply to Xidorn Quan [:xidorn] (UTC+12) from comment #16)
> (In reply to Jim Mathies [:jimm] from comment #14)
> > 3) type escape
> > 
> > result:
> > 
> > - the video window disappears suddenly along with the allow/deny dialog
> > leaving a maximized browser with tabs on the desktop
> > - the desktop and browser window fade to black leaving only the taskbar
> > visible. (weird! Looks like we're re-animating the black overlay back in
> > again.)
> > - once the desktop fades to black, it then faded back to the desktop more
> > slowly displaying the original normalized browser on the desktop
> 
> This is bug 1184443 which is unrelated to the issue here.

Ok, let me update and go through testing this again.

Comment 18

[Jim Mathies [:jimm]]

All the previous issues appear to be fixed now. The remaining issue I see (win7 + aero basic): the browser window takes the foreground briefly when it transitions from normalized to fullscreen. The overlay window then takes the foreground and fades out, leaving the video screen. Is that what others are seeing?

Comment 19

[Xidorn Quan [:xidorn] UTC+10]

(In reply to Jim Mathies [:jimm] from comment #18)
> All the previous issues appear to be fixed now. The remaining issue I see
> (win7 + aero basic): the browser window takes the foreground briefly when it
> transitions from normalized to fullscreen. The overlay window then takes the
> foreground and fades out, leaving the video screen. Is that what others are
> seeing?

Yes, it seems to me your description here matches what I saw.

Comment 20

[Xidorn Quan [:xidorn] UTC+10]

Are you still working on this for a workaround?

If it isn't solvable, we would probably better disable the transition in some conditions. Could you suggest how can we detect those conditions?

Comment 21

[Jim Mathies [:jimm]]

(In reply to Xidorn Quan [:xidorn] (UTC+12) from comment #20)
> Are you still working on this for a workaround?
> 
> If it isn't solvable, we would probably better disable the transition in
> some conditions. Could you suggest how can we detect those conditions?

I wasn't really working on anything, I was just going to investigate. That said I haven't had a chance yet, will try to find some time tomorrow. If you don't have a fix in mind, we should probably just disable on xp for now and come back to this when we have some time.

Comment 22

[Xidorn Quan [:xidorn] UTC+10]

(In reply to Jim Mathies [:jimm] from comment #21)
> 
> I wasn't really working on anything, I was just going to investigate. That
> said I haven't had a chance yet, will try to find some time tomorrow. If you
> don't have a fix in mind, we should probably just disable on xp for now and
> come back to this when we have some time.

It seems not only xp has this issue. With some settings, Windows 7 may also be affected.

Comment 23

[Kate Glazko]

Tracking for 42 because regression.

Comment 24

[Xidorn Quan [:xidorn] UTC+10]

I guess we can use IsCompositionActive() [1] or IsThemeActive() [2] or IsAppThemed() [3] as the condition in nsWindow::PrepareForFullscreenTransition(). The only problem is that, these three functions are added since Windows Vista, hence I'm not quite sure how to use them in our code.


[1] https://msdn.microsoft.com/en-us/library/bb759811%28v=vs.85%29.aspx
[2] https://msdn.microsoft.com/en-us/library/bb759813%28v=vs.85%29.aspx
[3] https://msdn.microsoft.com/en-us/library/bb759809%28v=vs.85%29.aspx

Comment 25

[Xidorn Quan [:xidorn] UTC+10]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=0e36b99e9292

Comment 26

[Xidorn Quan [:xidorn] UTC+10]

Attachment: patch

This patch disables fullscreen transition when composition is not enabled. I tested the build on Windows XP, and the transition is disabled properly. Still need test on Windows 7 Classic, though.

Win32 build: http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/[e-mail]-2d8a97f2c1d4/try-win32/
Win64 build: http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/[e-mail]-2d8a97f2c1d4/try-win64/

Alice, could you help test whether this build fixes this issue?

Comment 27

[Alice0775 White]

(In reply to Xidorn Quan [:xidorn] (UTC+12) from comment #26)
> Created attachment 8644832 [details] [diff] [review]
> patch
> 
> This patch disables fullscreen transition when composition is not enabled. I
> tested the build on Windows XP, and the transition is disabled properly.
> Still need test on Windows 7 Classic, though.
> 
> Win32 build:
> http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/[e-mail]-
> 2d8a97f2c1d4/try-win32/
> Win64 build:
> http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/[e-mail]-
> 2d8a97f2c1d4/try-win64/
> 
> Alice, could you help test whether this build fixes this issue?

I cannot reproduce the problem on Windows7 Classic.
The try-server builds disabled the transition animation.

https://hg.mozilla.org/try/rev/2d8a97f2c1d4
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:42.0) Gecko/20100101 Firefox/42.0 ID:20150806231415

https://hg.mozilla.org/try/rev/2d8a97f2c1d4
Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:42.0) Gecko/20100101 Firefox/42.0 ID:20150806231415

Comment 28

[Xidorn Quan [:xidorn] UTC+10]

(In reply to Alice0775 White from comment #27)
> (In reply to Xidorn Quan [:xidorn] (UTC+12) from comment #26)
> > Created attachment 8644832 [details] [diff] [review]
> > patch
> > 
> > This patch disables fullscreen transition when composition is not enabled. I
> > tested the build on Windows XP, and the transition is disabled properly.
> > Still need test on Windows 7 Classic, though.
> > 
> > Win32 build:
> > http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/[e-mail]-
> > 2d8a97f2c1d4/try-win32/
> > Win64 build:
> > http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/[e-mail]-
> > 2d8a97f2c1d4/try-win64/
> > 
> > Alice, could you help test whether this build fixes this issue?
> 
> I cannot reproduce the problem on Windows7 Classic.
> The try-server builds disabled the transition animation.

That's great. It is the intended behavior.

Comment 29

[Xidorn Quan [:xidorn] UTC+10]

url:        https://hg.mozilla.org/integration/mozilla-inbound/rev/acf0df12b9e310ccd71f372c508a49bd7b55713b
changeset:  acf0df12b9e310ccd71f372c508a49bd7b55713b
user:       Xidorn Quan <[e-mail]>
date:       Mon Aug 10 23:20:03 2015 +1000
description:
Bug 1184201 - Disable fullscreen transition on Windows if composition is disabled. r=jimm

Comment 30

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/acf0df12b9e3

Comment 31

[Xidorn Quan [:xidorn] UTC+10]

Comment on attachment 8644832 [details] [diff] [review]
patch

Approval Request Comment
[Feature/regressing bug #]: bug 1160014 which introduced the transition
[User impact if declined]: some user in xp and win7 classic will see annoying flick when entering fullscreen
[Describe test coverage new/current, TreeHerder]: n/a
[Risks and why]: no risk, as it just conditionally disables the newly-added transition on some system
[String/UUID change made/needed]: n/a

Comment 32

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8644832 [details] [diff] [review]
patch

Recent regression, taking it

Comment 33

[(PTO June 19-July 7) Ryan VanderMeulen]

https://hg.mozilla.org/releases/mozilla-aurora/rev/881a6fc93a03

Comment 34

[Camelia Badau [:cbadau], Desktop Test Engineering]

Verified fixed on XP and Win7 Classic using Firefox 43 Beta 4 (buildID: 20151116155110) and Firefox 44.0a2 (buildID: 20151119004053).