1749187 - linux screensaver inhibition is unconditional and can't be overridden, yet relies on untrusted data from the web

Status: RESOLVED MOVED

(Core :: Widget: Gtk, defect, P3)

Description

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Firefox/91.0

Steps to reproduce:

1. Run firefox with a normal selection of tabs open to a variety of sites (can't tell you which one, because I don't know which of many sites is triggering the problem, because of the way the inhibition is reported - see below). Probably youtube.

2. Kill gnome screensaver and xscreensaver.

3. Run xscreensaver -verbose

Actual results:

This set of messages is relayed by xsreensaver every 30 seconds:

xscreensaver-systemd: 21:26:54: uninhibited by "firefox-esr" with cookie 0FE73BCD
xscreensaver-systemd: 21:26:55: inhibit: unable to get pid of "firefox-esr": No data available
xscreensaver-systemd: 21:26:55: inhibited by "firefox-esr" with "video-playing" -> cookie 002B8F6E
xscreensaver-systemd: 21:27:25: uninhibited by "firefox-esr" with cookie 002B8F6E
xscreensaver-systemd: 21:27:26: inhibit: unable to get pid of "firefox-esr": No data available
xscreensaver-systemd: 21:27:26: inhibited by "firefox-esr" with "video-playing" -> cookie C5E68D51
xscreensaver-systemd: 21:27:56: inhibited by "firefox-esr" since Sun Jan  9 21:27:26 2022
xscreensaver-systemd: 21:27:56: exec: xscreensaver-command -verbose -deactivate
xscreensaver: ClientMessage DEACTIVATE received while inactive: resetting idle timer.
xscreensaver-command: not active: idle timer reset.

And the screensaver never is invoked. If the user forcefully invokes the screensaver to turn off their screen of a night, the screen is awakened again within 30 seconds. None of that output tells me why firefox considers a video is playing somewhere - like #1744641, the supposedly human-readable text doesn't tell what video is playing. What URL? What tab? None of my tabs have a video symbol on them. They're not searchable in my window alt-tab list. I'm guessing it's some silly ad somewhere on a page I don't care about in the first place. Either way, something, somewhere on the untrusted web is telling my machine that I'm not allowed to invoke the screensaver, that it is too important to be hidden.

Expected results:

1. The user should be given a choice, perhaps even hidden in about:config, to blanket ignore any requests to inhibit the screensaver (in my case, I never want firefox to inhibit the screensaver as I only watch videos in a real video player, but some people clearly want it).

2. The "video-playing" text should change to be something useful.

Comment 1

[BugBot [:suhaib / :marco/ :calixte]]

The Bugbug bot thinks this bug should belong to the 'Core::Widget: Gtk' component, and is moving the bug to that component. Please revert this change in case you think the bot is wrong.

Comment 2

[Vincent Bernat]

Hey!

I was bit by this "bug" as well. Any website can play a sound or a video to disable screen locking. This is not what I expect as it may randomly prevents screen locking depending on the tab the browser is left open. Only full screen videos should inhibit a lock. I would gladly prefer that Firefox does not inhibit video at all if it was the only solution.

Comment 3

[Sylvestre Ledru [:Sylvestre]]

Martin, does it ring a bell? thanks

Comment 4

[Martin Stránský [:stransky] (ni? me)]

I think it's more related to HTML video element than Gtk linux backend:
https://searchfox.org/mozilla-central/rev/3aaca0a12a2d1463da54933bdbdae2f06fead06f/dom/html/HTMLVideoElement.cpp#420

Gtk specific lock:
https://searchfox.org/mozilla-central/rev/3aaca0a12a2d1463da54933bdbdae2f06fead06f/widget/gtk/WakeLockListener.cpp#511

Windows lock:
https://searchfox.org/mozilla-central/rev/3aaca0a12a2d1463da54933bdbdae2f06fead06f/widget/windows/nsAppShell.cpp#175

We can adjust Linux lock to add 'isNonDisplayLock' from Windows but fullscreen/normal video playback is handled here:
https://searchfox.org/mozilla-central/rev/3aaca0a12a2d1463da54933bdbdae2f06fead06f/dom/html/HTMLVideoElement.cpp#400

and it's shared by all platforms.

Comment 5

[Martin Stránský [:stransky] (ni? me)]

(In reply to Tim Connors from comment #0)

1. The user should be given a choice, perhaps even hidden in about:config, to blanket ignore any requests to inhibit the screensaver.

I don't have any strong opinion here...feel free to create a patch for it and ask me for review.
Thanks.

Comment 6

[Vincent Bernat]

I am not the original reporter, but I would suggest instead to:

1. never inhibit on audio (no screen needed to play audio, dedicated audio players don't inhibit the screen saver either)
2. only inhibit on full screen video

I can try to do a patch for that, but as I am not a regular contributor, I would prefer to get a signal that this approach is likely to be accepted. If not, I can also implement the about:config option instead, but again, not before I get a signal that this is likely to be accepted.

Comment 7

[Martin Stránský [:stransky] (ni? me)]

(In reply to Vincent Bernat from comment #6)

I am not the original reporter, but I would suggest instead to:

1. never inhibit on audio (no screen needed to play audio, dedicated audio players don't inhibit the screen saver either)

Can be easily done on Linux, see Windows implementation:
https://searchfox.org/mozilla-central/rev/3aaca0a12a2d1463da54933bdbdae2f06fead06f/widget/windows/nsAppShell.cpp#184

2. only inhibit on full screen video

AFAIK this is set by html video element and needs a different bug to audio/video playback. Such change will affect all platforms and I don't expect layout folks will accept that.

I can try to do a patch for that, but as I am not a regular contributor, I would prefer to get a signal that this approach is likely to be accepted. If not, I can also implement the about:config option instead, but again, not before I get a signal that this is likely to be accepted.

I think it's OK to add widget.* pref to about:config for that.

Comment 8

[Martin Stránský [:stransky] (ni? me)]

Attachment: Bug 1749187 [Linux] Don't disable screensaver for audio playback r?emilio

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/199c0ffb9c3b
[Linux] Don't disable screensaver for audio playback r=emilio

Comment 10

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/199c0ffb9c3b

Comment 11

[Tim Connors]

Er, the fix here only deals with audio. The bug is about how the screensaver inhibition is unconditional on things like endless video advertisement loops on news websites. The bug has not yet been fixed so shouldn't be marked as fixed.

Comment 12

[Martin Stránský [:stransky] (ni? me)]

(In reply to Tim Connors from comment #11)

Er, the fix here only deals with audio. The bug is about how the screensaver inhibition is unconditional on things like endless video advertisement loops on news websites. The bug has not yet been fixed so shouldn't be marked as fixed.

If you want any specific screensaver tweaks regards media you need to file but against AudioVideo playback. Widget code doesn't have any info which media is played, we only know that something is playing and we should disable screensaver.

It may be possible to add preference and disable screensaver suppression completely for all media on widget level.

Comment 13

[[e-mail]]

I find really strange that the Mozilla/Firefox team didn't consider by itself the fact that choice should remain at the end in the hands of users. I have a virtual env with 9 desktops, each having multiple firefox each having multiple tabs, meaning hundreds of tabs opened. Now on page has decided to display a video, and now my screen is just burning as no screensaver is running (despite my configuration at system level).
For me it's close to a DOS. and can kill HW :-(

You should provide a way for a user to be pointed to the tab creating the issue (so I can shut it down and be back to normal) and more over to have at least an option to disable that disablement of screensaver.

I'm way too unfamiliar with firefox code to propose a patch here, sorry, but if user feedback is what drives that community they should here us :-)

Comment 14

[Tim Connors]

I was the or(In reply to Martin Stránský [:stransky] (ni? me) from comment #12)

(In reply to Tim Connors from comment #11)

Er, the fix here only deals with audio. The bug is about how the screensaver inhibition is unconditional on things like endless video advertisement loops on news websites. The bug has not yet been fixed so shouldn't be marked as fixed.

If you want any specific screensaver tweaks regards media you need to file but against AudioVideo playback. Widget code doesn't have any info which media is played, we only know that something is playing and we should disable screensaver.

It may be possible to add preference and disable screensaver suppression completely for all media on widget level.

I never responded to this, sorry. It wasn't I (the original submitter) who assigned this to Gtk though - looks like the bot got the wrong component. I am reassigning to AudioVideo per this comment.

Comment 15

[Masatoshi Kimura [:emk]]

I think we should create a new bug on Audio/Video: Playback component instead of taking over this bug because this bug already has a landed patch on a different component.

Comment 16

[Tim Connors]

Cloned to 1885268, so resetting these flags back to what they were.