GNOME Bugzilla – Bug 669110
Totem legacy screensaver support is not needed and causes problems
Last modified: 2012-04-23 13:51:41 UTC
Created attachment 206517 [details] [review] removes legacy screensaver support from totem Totems legacy screensaver support uses XTest to tap a keycode, specifically alt and altgr every 30 seconds which causes issues with anything that uses Alt as a key binding. Firstly, totem should not be using XTest as its impossible for applications depending on X to tell the difference between synthesized events and real events when they come from XTest. Secondly, this legacy support is not needed any longer, the two methods that do exist (XScreensaver and the dbus api) work perfectly. Other media players such as mplayer and VLC use these API's on their own with zero complications. Attached is a patch that removes the legacy screensaver functionality
(In reply to comment #0) > Created an attachment (id=206517) [details] [review] > removes legacy screensaver support from totem > > Totems legacy screensaver support uses XTest to tap a keycode, specifically alt > and altgr every 30 seconds which causes issues with anything that uses Alt as a > key binding. Firstly, totem should not be using XTest as its impossible for > applications depending on X to tell the difference between synthesized events > and real events when they come from XTest. Totem only sends events when it's the application that has the focus. > Secondly, this legacy support is not > needed any longer, the two methods that do exist (XScreensaver and the dbus > api) work perfectly. Other media players such as mplayer and VLC use these > API's on their own with zero complications. Using the XScreensaver API can end up disabling the desktop support for the screensaver on some systems. I doubt they would like it. > Attached is a patch that removes the legacy screensaver functionality Unless this fixes _actual_ problems, I'm not removing it.
(In reply to comment #1) > (In reply to comment #0) > > Secondly, this legacy support is not > > needed any longer, the two methods that do exist (XScreensaver and the dbus > > api) work perfectly. Other media players such as mplayer and VLC use these > > API's on their own with zero complications. > > Using the XScreensaver API can end up disabling the desktop support for the > screensaver on some systems. I doubt they would like it. > totem already uses the XScreensaver API, along with the dbus api, this patch doesn't change codepaths, but merely removes one legacy codepath. > > Attached is a patch that removes the legacy screensaver functionality > > Unless this fixes _actual_ problems, I'm not removing it. It fixes a problem we have with Unity in Ubuntu, in that we have actions bound to alt, (it shows a shell interface). When totem is playing, as it sends an Alt key-press every 30 seconds, will trigger this interface to appear. As it uses the XTest api, there is no way to tell the difference between totem simulating an Alt keypress and a user pressing Alt
Would that patch also fix https://bugzilla.redhat.com/show_bug.cgi?id=571179 ?
(In reply to comment #3) > Would that patch also fix https://bugzilla.redhat.com/show_bug.cgi?id=571179 ? it should do, it removes the Alt pressing behavior entirely
(In reply to comment #3) > Would that patch also fix https://bugzilla.redhat.com/show_bug.cgi?id=571179 ? It's already fixed by: http://git.gnome.org/browse/totem/commit/lib/totem-scrsaver.c?id=5a4eee111fb6af3fdd215394feee721e59a8e1cb (In reply to comment #2) > > Unless this fixes _actual_ problems, I'm not removing it. > > It fixes a problem we have with Unity in Ubuntu, in that we have actions bound > to alt, (it shows a shell interface). When totem is playing, as it sends an Alt > key-press every 30 seconds, will trigger this interface to appear. As it uses > the XTest api, there is no way to tell the difference between totem simulating > an Alt keypress and a user pressing Alt And the D-Bus screensaver API doesn't work under Unity? How do you get Alt+M to get to the movie menu working if Unity is using that key? Also, using XI2 you can see which device is sending the key event, so your earlier statement that it's impossible isn't correct (we use it in gnome-settings-daemon to change the volume of usb headsets and speakers).
(In reply to comment #5) > (In reply to comment #3) > > Would that patch also fix https://bugzilla.redhat.com/show_bug.cgi?id=571179 ? > > It's already fixed by: > http://git.gnome.org/browse/totem/commit/lib/totem-scrsaver.c?id=5a4eee111fb6af3fdd215394feee721e59a8e1cb > > (In reply to comment #2) > > > Unless this fixes _actual_ problems, I'm not removing it. > > > > It fixes a problem we have with Unity in Ubuntu, in that we have actions bound > > to alt, (it shows a shell interface). When totem is playing, as it sends an Alt > > key-press every 30 seconds, will trigger this interface to appear. As it uses > > the XTest api, there is no way to tell the difference between totem simulating > > an Alt keypress and a user pressing Alt > > And the D-Bus screensaver API doesn't work under Unity? How do you get Alt+M to > get to the movie menu working if Unity is using that key? > The D-Bus screensaver *does* work under unity, that is not disabled with this patch. only tapping the alt key is disabled. Unity is only using the Alt keypress and does not block other UI's from having Alt based key bindings such as Alt+M > Also, using XI2 you can see which device is sending the key event, so your > earlier statement that it's impossible isn't correct (we use it in > gnome-settings-daemon to change the volume of usb headsets and speakers). We use autopilot for a complete test system, which uses XTest to simulate X events, so we can't tell the difference between totem and XTest.
(In reply to comment #6) <snip> > The D-Bus screensaver *does* work under unity, that is not disabled with this > patch. only tapping the alt key is disabled. Unity is only using the Alt > keypress and does not block other UI's from having Alt based key bindings such > as Alt+M The X11 codepath should only be exercised if the D-Bus path isn't available. So either you don't support the D-Bus screensaver API, or there's a bug in the totem-scrsaver code.
Comment on attachment 206517 [details] [review] removes legacy screensaver support from totem As per comment 7
(In reply to comment #3) > Would that patch also fix https://bugzilla.redhat.com/show_bug.cgi?id=571179 ? it should do, it removes the Alt pressing behavior entirely(In reply to comment #7) > (In reply to comment #6) > <snip> > The X11 codepath should only be exercised if the D-Bus path isn't available. So > either you don't support the D-Bus screensaver API, or there's a bug in the > totem-scrsaver code. Quick test shows that totem registers itself with the dbus method and is listed as an Inhibitor - but still exercises the alt pressing codepath. However, it is still bad behavior for a an application to simulate Alt keypress events using an API designed for testing.
As you asked for actual problems: You can't have totem playing a movie and use the browser Opera side-by-side. Opera is always interrupted and shows the file-menu.
I'd also like to add, that totem sumulates Alt keypresses when it is NOT in focus (as long as a movie is running).
Could you please make sure you're all testing this with Totem 3.2 or newer? I'm not interested in debugging old code.
I don't have the environment to test Totem 3.2 right now, but I'll check that out soon. This will still be a problem, though, for distros that will stick with older Totem versions because of the clutter dependency (like Ubuntu). So I assume whoever wants this fixed has to fix it downstream.
The old screensaver code was replaced by GtkApplication’s inhibit code in these two commits: • http://git.gnome.org/browse/totem/commit/?id=1c9970511b8691012b284a5ae02190f4405e332c • http://git.gnome.org/browse/totem/commit/?id=4b78319a380052e297f7f8cc82d82c024568aac4 so this is fixed.