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

VERIFIED FIXED in Firefox 42

Status

()

defect
--
critical
VERIFIED FIXED
4 years ago
4 months ago

People

(Reporter: alice0775, Assigned: xidorn)

Tracking

({regression})

42 Branch
mozilla43
Unspecified
Windows
Points:
---
Dependency tree / graph
Bug Flags:
qe-verify +

Firefox Tracking Flags

(firefox41 unaffected, firefox42+ verified, firefox43 verified)

Details

()

Attachments

(2 attachments)

[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
Flags: needinfo?(quanxunzhen)
Flags: needinfo?(jmathies)
OS: Unspecified → Windows 7
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.
Flags: needinfo?(quanxunzhen)
Flags: needinfo?(jmathies)
Flags: needinfo?(quanxunzhen)
Also Windows classic desktop looks worse than aero. We need to do some testing there and with aero basic.
Posted video screen capture
Whole screen(incl. desctop) flickers black and white
s/desctop/desktop/
yikes, that is bad.
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.
(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.
I can also reproduce this on Windows XP.
OS: Windows 7 → Windows
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?
Flags: needinfo?(jmathies)
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.
Flags: needinfo?(quanxunzhen)
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
This was in 42.0a1 (2015-07-15)
Flags: needinfo?(jmathies)
Flags: needinfo?(jmathies)
(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.
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
Flags: needinfo?(jmathies)
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.
(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.
Flags: needinfo?(jmathies)
(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.
Flags: needinfo?(jmathies)
Flags: needinfo?(jmathies)
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?
Flags: needinfo?(jmathies)
(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.
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?
Flags: needinfo?(jmathies)
(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.
(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.
Tracking for 42 because regression.
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
Posted patch patchSplinter Review
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/xquan@mozilla.com-2d8a97f2c1d4/try-win32/
Win64 build: http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/xquan@mozilla.com-2d8a97f2c1d4/try-win64/

Alice, could you help test whether this build fixes this issue?
Assignee: nobody → quanxunzhen
Flags: needinfo?(jmathies) → needinfo?(alice0775)
Attachment #8644832 - Flags: review?(jmathies)
(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/xquan@mozilla.com-
> 2d8a97f2c1d4/try-win32/
> Win64 build:
> http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/xquan@mozilla.com-
> 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
Flags: needinfo?(alice0775)
(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/xquan@mozilla.com-
> > 2d8a97f2c1d4/try-win32/
> > Win64 build:
> > http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/xquan@mozilla.com-
> > 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.
Attachment #8644832 - Flags: review?(jmathies) → review+
url:        https://hg.mozilla.org/integration/mozilla-inbound/rev/acf0df12b9e310ccd71f372c508a49bd7b55713b
changeset:  acf0df12b9e310ccd71f372c508a49bd7b55713b
user:       Xidorn Quan <quanxunzhen@gmail.com>
date:       Mon Aug 10 23:20:03 2015 +1000
description:
Bug 1184201 - Disable fullscreen transition on Windows if composition is disabled. r=jimm
https://hg.mozilla.org/mozilla-central/rev/acf0df12b9e3
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla43
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
Attachment #8644832 - Flags: approval-mozilla-aurora?
Comment on attachment 8644832 [details] [diff] [review]
patch

Recent regression, taking it
Attachment #8644832 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Flags: qe-verify+
Verified fixed on XP and Win7 Classic using Firefox 43 Beta 4 (buildID: 20151116155110) and Firefox 44.0a2 (buildID: 20151119004053).
Status: RESOLVED → VERIFIED
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.