1193610 - Win XP and Vista screensaver doesn't disable when watching videos

Status: VERIFIED FIXED

(Core :: Audio/Video: Playback, defect, P2)

Description

[Robert Killingsworth]

User Agent: Mozilla/5.0 (Windows NT 5.1; rv:40.0) Gecko/20100101 Firefox/40.0
Build ID: 20150807085045

Steps to reproduce:

1. Enable webm so you can get something better than 360p (step might be optional)
2. Watch Youtube video
3. Wait for screensaver activation.


Actual results:

Screensaver started.


Expected results:

Screensaver should not have started.

Comment 1

[Ralph Giles (:rillian)]

Only affects the HTML player, not flashplayer, presumely.

Comment 2

[Robert Killingsworth]

Yes, it only affects the HTML player.

Comment 3

[Xidorn Quan [:xidorn] UTC+10]

Is it related to fullscreen? I think watching video itself should stop the system activating screensaver.

Comment 4

[Chris Pearce [:cpearce (Not reading bugmail)]]

It's not related to fullscreen, but if you want to take this bug Xidorn, go ahead! ;)

We have a wakelocklistener here:
http://mxr.mozilla.org/mozilla-central/source/widget/windows/nsAppShell.cpp#47

That receives notifications when videos start and stop playing. Don't know why it's not working on WinXP.

Comment 5

[Chris Pearce [:cpearce (Not reading bugmail)]]

Attachment: Patch: Use SystemParametersInfo to block screen saver

Using SetThreadExecutionState(ES_DISPLAY_REQUIRED|ES_CONTINUOUS) isn't sufficient to block the screen saver on Win Vista and XP. So periodically get the screen saver timeout, and then set the screensaver timeout to the got value, which resets the OS's timer, and blocks the screen saver without changing anything.

Comment 6

[Jim Mathies [:jimm]]

Comment on attachment 8677866 [details] [diff] [review]
Patch: Use SystemParametersInfo to block screen saver

Review of attachment 8677866 [details] [diff] [review]:
-----------------------------------------------------------------

::: widget/windows/nsAppShell.cpp
@@ +61,5 @@
>      // "locked-foreground" notifications when multipe wake locks are held.
>      if (aState.EqualsASCII("locked-foreground")) {
> +      WAKE_LOCK_LOG("WinWakeLock: Blocking screen saver");
> +      // We block the screen saver by periodically resetting the screen
> +      // saver timeout. 

nit - whitespace at eol

@@ +122,5 @@
> +  // Resets the operating system's timeout for when to disable the screen.
> +  // Called periodically to keep the screensaver off.
> +  void ResetScreenSaverTimeout() {
> +    if (SystemParametersInfo(SPI_GETSCREENSAVETIMEOUT, 0, &mScreenSaverTimeout, 0)) {
> +      SystemParametersInfo(SPI_SETSCREENSAVETIMEOUT, mScreenSaverTimeout, NULL, 0);

Where did you find this trick?

Comment 7

[Chris Pearce [:cpearce (Not reading bugmail)]]

(In reply to Jim Mathies [:jimm] from comment #6)
> Comment on attachment 8677866 [details] [diff] [review]
> Patch: Use SystemParametersInfo to block screen saver

> @@ +122,5 @@
> > +  // Resets the operating system's timeout for when to disable the screen.
> > +  // Called periodically to keep the screensaver off.
> > +  void ResetScreenSaverTimeout() {
> > +    if (SystemParametersInfo(SPI_GETSCREENSAVETIMEOUT, 0, &mScreenSaverTimeout, 0)) {
> > +      SystemParametersInfo(SPI_SETSCREENSAVETIMEOUT, mScreenSaverTimeout, NULL, 0);
> 
> Where did you find this trick?

In the comments of the MSDN article on SetThreadExecutionState():
https://msdn.microsoft.com/en-us/library/windows/desktop/aa373208%28v=vs.85%29.aspx

Was mentioned in some stackoverflow threads too.

Comment 8

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/96eeff9fec80

Comment 9

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/96eeff9fec80

Comment 10

[Chris Pearce [:cpearce (Not reading bugmail)]]

Comment on attachment 8677866 [details] [diff] [review]
Patch: Use SystemParametersInfo to block screen saver

Approval Request Comment
[Feature/regressing bug #]: Video playback/YouTube on Win Vista and XP
[User impact if declined]: On WinXP and Vista, when video is playing the screen saver can engage, interrupting viewing pleasure; we currently disable the screen saver on other Windows versions.
[Describe test coverage new/current, TreeHerder]: We don't have a screen saver test; it's hard to test.
[Risks and why]: Low; we do an extra thing to what we already do, so shouldn't break existing screen saver disabling on other Windows versions.
[String/UUID change made/needed]: None.

Comment 11

[Liz Henry (:lizzard) (relman/hg->git project)]

Comment on attachment 8677866 [details] [diff] [review]
Patch: Use SystemParametersInfo to block screen saver

Good for user experience, let's take this in beta.
Not a lot of test coverage so we should verify the fix manually.

Comment 12

[Chris Pearce [:cpearce (Not reading bugmail)]]

https://hg.mozilla.org/releases/mozilla-beta/rev/fabe9ce33dc2

Comment 13

[Vasilica Tamas, QA [:vtamas]]

I was able to reproduce this issue on Firefox 42.0a1 (2015-08-11) using Windows Xp 64-bit.
Verified fixed on Firefox 44.0a2 (2015-11-16) and Firefox 43 beta 3 (20151112144305) under Windows Xp 64-bit and Windows Vista 32-bit.

Comment 14

[Chris Pearce [:cpearce (Not reading bugmail)]]

Release Note Request (optional, but appreciated)
[Why is this notable]: Fixes what is probably a very annoying behaviour for Firefox on WinXP/Vista.
[Suggested wording]: Screensaver is temporarily disabled on Windows XP and Vista when playing HTML5 video.
[Links (documentation, blog post, etc)]:

Comment 15

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Added to Beta44 release notes.