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

VERIFIED FIXED in Firefox 42

Status

()

--
critical
VERIFIED FIXED
3 years ago
3 years ago

People

(Reporter: alice0775, Assigned: xidorn)

Tracking

({regression})

42 Branch
mozilla43
Unspecified
Windows
regression
Points:
---
Bug Flags:
qe-verify +

Firefox Tracking Flags

(firefox41 unaffected, firefox42+ verified, firefox43 verified)

Details

(URL)

Attachments

(2 attachments)

(Reporter)

Description

3 years ago
[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)
(Reporter)

Updated

3 years ago
OS: Unspecified → Windows 7

Comment 1

3 years ago
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)

Updated

3 years ago
Flags: needinfo?(quanxunzhen)

Comment 2

3 years ago
Also Windows classic desktop looks worse than aero. We need to do some testing there and with aero basic.
(Reporter)

Comment 3

3 years ago
Created attachment 8634273 [details]
screen capture

Whole screen(incl. desctop) flickers black and white
(Reporter)

Comment 4

3 years ago
s/desctop/desktop/

Comment 5

3 years ago
yikes, that is bad.
(Assignee)

Comment 6

3 years ago
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

3 years ago
(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.
(Assignee)

Comment 8

3 years ago
I can also reproduce this on Windows XP.
OS: Windows 7 → Windows
(Assignee)

Comment 9

3 years ago
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)
(Assignee)

Comment 10

3 years ago
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.
(Assignee)

Updated

3 years ago
Flags: needinfo?(quanxunzhen)

Comment 11

3 years ago
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

3 years ago
This was in 42.0a1 (2015-07-15)
Flags: needinfo?(jmathies)

Updated

3 years ago
Flags: needinfo?(jmathies)
(Assignee)

Comment 13

3 years ago
(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

3 years ago
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)

Comment 15

3 years ago
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.
(Assignee)

Comment 16

3 years ago
(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)

Comment 17

3 years ago
(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)

Updated

3 years ago
Flags: needinfo?(jmathies)

Comment 18

3 years ago
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)
(Assignee)

Comment 19

3 years ago
(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.
(Assignee)

Comment 20

3 years ago
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)

Comment 21

3 years ago
(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.
(Assignee)

Comment 22

3 years ago
(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

3 years ago
Tracking for 42 because regression.
tracking-firefox42: ? → +
(Assignee)

Comment 24

3 years ago
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
(Assignee)

Comment 26

3 years ago
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?
Assignee: nobody → quanxunzhen
Flags: needinfo?(jmathies) → needinfo?(alice0775)
Attachment #8644832 - Flags: review?(jmathies)
(Reporter)

Comment 27

3 years ago
(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)
(Assignee)

Comment 28

3 years ago
(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.

Updated

3 years ago
Attachment #8644832 - Flags: review?(jmathies) → review+
(Assignee)

Comment 29

3 years ago
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
Last Resolved: 3 years ago
status-firefox43: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla43
(Assignee)

Comment 31

3 years ago
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
status-firefox42: fixed → verified
status-firefox43: fixed → verified
You need to log in before you can comment on or make changes to this bug.