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

VERIFIED FIXED in Firefox 43

Status

()

Core
Audio/Video: Playback
P2
normal
VERIFIED FIXED
2 years ago
2 years ago

People

(Reporter: Robert Killingsworth, Assigned: cpearce)

Tracking

40 Branch
mozilla44
x86
Windows XP
Points:
---
Bug Flags:
qe-verify +

Firefox Tracking Flags

(firefox43 verified, firefox44 verified, relnote-firefox 44+)

Details

Attachments

(1 attachment)

(Reporter)

Description

2 years ago
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.
(Reporter)

Updated

2 years ago
OS: Unspecified → Windows XP
Hardware: Unspecified → x86

Updated

2 years ago
Component: Untriaged → Audio/Video
Product: Firefox → Core
Blocks: 778617
Summary: Screensaver doesn't disable when watching fullscreen videos → Win XP Screensaver doesn't disable when watching fullscreen videos
Only affects the HTML player, not flashplayer, presumely.
Component: Audio/Video → Audio/Video: Playback
Priority: -- → P2
(Reporter)

Comment 2

2 years ago
Yes, it only affects the HTML player.
No longer blocks: 778617
Is it related to fullscreen? I think watching video itself should stop the system activating screensaver.
(Assignee)

Comment 4

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

Updated

2 years ago
Assignee: nobody → cpearce
(Assignee)

Comment 5

2 years ago
Created attachment 8677866 [details] [diff] [review]
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.
Attachment #8677866 - Flags: review?(jmathies)
(Assignee)

Updated

2 years ago
Summary: Win XP Screensaver doesn't disable when watching fullscreen videos → Win XP and Vista screensaver doesn't disable when watching videos

Comment 6

2 years ago
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?
Attachment #8677866 - Flags: review?(jmathies) → review+
(Assignee)

Comment 7

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

2 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/96eeff9fec80
https://hg.mozilla.org/mozilla-central/rev/96eeff9fec80
Status: UNCONFIRMED → RESOLVED
Last Resolved: 2 years ago
status-firefox44: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla44
(Assignee)

Comment 10

2 years ago
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.
Attachment #8677866 - Flags: approval-mozilla-beta?
status-firefox43: --- → affected
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.
Attachment #8677866 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Flags: qe-verify+
(Assignee)

Comment 12

2 years ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-beta/rev/fabe9ce33dc2
status-firefox43: affected → fixed
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.
Status: RESOLVED → VERIFIED
status-firefox43: fixed → verified
status-firefox44: fixed → verified
(Assignee)

Comment 14

2 years ago
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)]:
relnote-firefox: --- → ?
Added to Beta44 release notes.
relnote-firefox: ? → 44+
(Assignee)

Updated

2 years ago
Duplicate of this bug: 1210139
You need to log in before you can comment on or make changes to this bug.