User-Agent: Mozilla/5.0 (X11; U; Linux x86_64; en; rv:1.8.1.11) Gecko/20071201 (Debian-1.8.1.11-1) Epiphany/2.20 Build Identifier: xflock4 only checks if a screensaver is installed, not if it's running. If both xscreensaver and gnome-screensaver are installed, but gnome-screensaver is running, xflock4 will try to lock using xscreensaver, wich will fail. xflock4 should test the running screensaver, not the installed one. Thanks for the work, and bye, -- Yves-Alexis Perez Reproducible: Always
*** Bug 3791 has been marked as a duplicate of this bug. ***
Created attachment 2080 xflock4 scripts that I have not modified In my system this seems to be fixed even if it doesn't seem to be fixed in svn :o (http://svn.xfce.org/svn/xfce/xfce-utils/trunk/scripts/xflock4). I wonder what are those square brackets doing in the script.
(In reply to comment #2) > Created an attachment (id=2080) [details] > xflock4 scripts that I have not modified yes, this is the debian one. I wonder why I didn't already submitted the patch. > I wonder what are those square brackets doing in the script. It's a ps/grep trick used not to have the grep command shown in the ps output.
(In reply to comment #3) > It's a ps/grep trick used not to have the grep command shown in the ps output. I have no idea how it works. Why don't you just enclose the first character of each process name by square brackets?
(In reply to comment #4) > (In reply to comment #3) > > It's a ps/grep trick used not to have the grep command shown in the ps output. > > I have no idea how it works. In fact, the command is run by the shell, which expands [s] to s. So what is grepped for is xscreensaver. But in ps output the run command is [s]creensaver which is not matched by the grep. Why don't you just enclose the first character of > each process name by square brackets? That would do the trick too
(In reply to comment #5) Thanks for the clarifications.
Created attachment 2132 Check the running screensaver Here's a new version, which checks the running screensaver and force the blank mode in xlock, because it's known to be insecure. I'll apply this one for 4.6 debian packages.
Any chance this can be included in 4.6? I see RC1 was set as target, nevertheless it's not yet applied.
The proposed patch fails in some unlikely cases such as when xscreensaver daemon is not running, but e.g. command "nano is_not_xscreensaver_running" is running.
Created attachment 3226 solution suggested by Guido Berhoerster
Does anything (like suspending/hibernating) rely on xflock4 always exiting with 0 exit status or is that just another quirk in the original script?
As for the solution of Guido Berhoerster, it does not check which screensaver is running, but maybe it is better not to check it since it it too hard.
(In reply to comment #12) > As for the solution of Guido Berhoerster, it does not check which screensaver > is running, but maybe it is better not to check it since it it too hard. This solution *does* check which screensaver is running, both xscreensaver-command and gnome-screensaver-command fail with exit code 1 if xscreensaver or respectively gnome-screensaver are not running.
Then it is better than the ps way, I think. I had both ubuntu-desktop and xubuntu-desktop installed on 10.04 sometime and I am not sure, if there were two power managers or even two screen saver daemons running.
(In reply to comment #14) > Then it is better than the ps way, I think. I had both ubuntu-desktop and > xubuntu-desktop installed on 10.04 sometime and I am not sure, if there were > two power managers or even two screen saver daemons running. No it isn't, the "ps way" is fundamentally flawed as somebodey already pointed out. The above script tries to lock the screen using the screensaver running in the current session, if there is none it tries to fall back to xlock and slock, if those are not installed it doesn't lock the screen. And this should cover all scenarios appropriately.
Guido Berhoerster, yes it is fundamentally better in checking whether xscreensaver daemon is running or not. If xscreensaver daemon is not running and gnome-screensaver is installed, it will start gnome-screensaver daemon and lock screen by its screensaver. I am not sure, if gnome-screensaver can be used properly with xubuntu anymore; I tried to switch user by it, and I couldn't switch back anymore. On the other hand, new login does not work by xscreensaver either in 10.04. As for slock and xlock, Bug 5732 remains.
Oh, and sorry, if I put something ubuntu specific here.
(In reply to comment #16) > Guido Berhoerster, yes it is fundamentally better in checking whether > xscreensaver daemon is running or not. If xscreensaver daemon is not running > and gnome-screensaver is installed, it will start gnome-screensaver daemon and > lock screen by its screensaver. I am not sure, if gnome-screensaver can be used The dbus activation of gnome-screensaver by gnome-screensaver-command is a bug only present in GNOME 2.30 and already fixed, see https://bugzilla.gnome.org/show_bug.cgi?id=629740
gnome-screensaver-command --lock starts gnome-screensaver daemon also in GNOME 2.28 (in ubuntu 10.04), but I think your script handles the bug relatively well. As for the remaining screen lockers their existence could be checked by "which" command.
(In reply to comment #19) > gnome-screensaver-command --lock starts gnome-screensaver daemon also in GNOME > 2.28 (in ubuntu 10.04), but I think your script handles the bug relatively > well. As for the remaining screen lockers their existence could be checked by > "which" command. No, that's the whole point of that bug. We need to check the running screensaver, not the existing screensavers.
xlock (and slock, if that is included) do not have any daemon running.
(In reply to comment #21) > xlock (and slock, if that is included) do not have any daemon running. Ok, I thought you were including xscreensaver in “the remaining” ones.
(In reply to comment #19) > gnome-screensaver-command --lock starts gnome-screensaver daemon also in GNOME > 2.28 (in ubuntu 10.04), but I think your script handles the bug relatively Well that is simply because Ubuntu 10.04 comes with GNOME 2.30 and not 2.28. Both Ubuntu 10.04 and 10.10 ship gnome-screensaver 2.30.0. The bug was introduced with commit 7bd27979d3e364efcbd6392edea9d233c1f19cad after 2.30.0 was tagged and fixed with commit a37f531e5ba968bea445e82dbc418d8b7ced219c. > well. As for the remaining screen lockers their existence could be checked by > "which" command. Why would you want to do that? If a command doesn't exist the for loop will simply move to the next one. (Besides, "which" is a bashism, the portable SUSv3 equivalent is command -v.)
(In reply to comment #23) > (In reply to comment #19) > > gnome-screensaver-command --lock starts gnome-screensaver daemon also in GNOME > > 2.28 (in ubuntu 10.04), but I think your script handles the bug relatively > > Well that is simply because Ubuntu 10.04 comes with GNOME 2.30 and not 2.28. > Both Ubuntu 10.04 and 10.10 ship gnome-screensaver 2.30.0. You are correct. I only checked the version of "gnome" package. > > well. As for the remaining screen lockers their existence could be checked by > > "which" command. > > Why would you want to do that? If a command doesn't exist the for loop will > simply move to the next one. (Besides, "which" is a bashism, the portable SUSv3 > equivalent is command -v.) If slock exists and is used, the proposed xflock4 script does not terminate until screen is unlocked. To make xflock4 terminate after it has locked screen, slock should be run in background by "slock &". Your script seems to work even with that kind of lock_cmd, but displays output you tried to prevent, if slock does not exist.
(In reply to comment #24) > slock should be run in background by "slock &". Your script seems to work even > with that kind of lock_cmd, but displays output you tried to prevent, if slock > does not exist. Oh it did on my command line, but now I tested it in the script, and it does not work. So I guess the existence of xlock and alike have to be checked separately, it there is more than one locker.
Created attachment 3232 check existence of screen lockers that do not have daemons If xscreensaver daemon is not running: If GNOME 2.30 is used, the script runs gnome-screensaver, if it is installed. In some other GNOME version it will run gnome-screensaver only, if its daemon is running. Checks existence of some screen lockers that do not have daemons. Do we want slock? Lets the script terminate after it has locked screen. Exits with code 1, if it notices it can not lock screen. Is this ok?
(In reply to comment #25) > (In reply to comment #24) > > > slock should be run in background by "slock &". Your script seems to work even > > with that kind of lock_cmd, but displays output you tried to prevent, if slock > > does not exist. > > Oh it did on my command line, but now I tested it in the script, and it does > not work. So I guess the existence of xlock and alike have to be checked Of course that cannot possibly work, you can either have an OR-list where the execution of the command on the right hand side depends on the exit status of the command on the left hand side or you start a command asynchronously with '&'. > separately, it there is more than one locker. Why do you actually want to execute xlock asynchronously? xflock4 itself is already executed asynchronously through g_spawn_command_line_async() (which is jusr a wrapper around fork()+exec())?
(In reply to comment #27) > Why do you actually want to execute xlock asynchronously? xflock4 itself is > already executed asynchronously through g_spawn_command_line_async() (which is > jusr a wrapper around fork()+exec())? User may run xflock4 script in his/her own script and expect it to terminate after it has locked screen or report it could not lock screen.
Created attachment 3233 alternative variant executing xlock, slock asynchronously (In reply to comment #28) > (In reply to comment #27) > > > Why do you actually want to execute xlock asynchronously? xflock4 itself is > > already executed asynchronously through g_spawn_command_line_async() (which is > > jusr a wrapper around fork()+exec())? > > User may run xflock4 script in his/her own script and expect it to terminate > after it has locked screen or report it could not lock screen. Hm, the current script doesn't do that either and I don't see why this should be treated as a public interface when something trivial as this can be implemented in a handful of lines in a shellscript. But anyway, here is a portable, SUSv3 compliant variant which should work how you want it (it avoids the 'which' bashism and the unecessary test and subshell).
(In reply to comment #29) > Hm, the current script doesn't do that either and I don't see why this should > be treated as a public interface when something trivial as this can be > implemented in a handful of lines in a shellscript. But anyway, here is a > portable, SUSv3 compliant variant which should work how you want it (it avoids > the 'which' bashism and the unecessary test and subshell). That is elegant except that there are no xxscreensaver-command or xgnome-screensaver-command. But in my system I think I don't need a screen locker that uses a daemon. (I miss ability to switch user and run a guest session so that I could return to the original session, though.) And isn't monitor power control done by xfce4-power-manager even without a screensaver?
Created attachment 3234 alternative variant executing xlock, slock asynchronously (In reply to comment #30) > (In reply to comment #29) > > > Hm, the current script doesn't do that either and I don't see why this should > > be treated as a public interface when something trivial as this can be > > implemented in a handful of lines in a shellscript. But anyway, here is a > > portable, SUSv3 compliant variant which should work how you want it (it avoids > > the 'which' bashism and the unecessary test and subshell). > > That is elegant except that there are no xxscreensaver-command or I've fixed the typo. > xgnome-screensaver-command. But in my system I think I don't need a screen > locker that uses a daemon. (I miss ability to switch user and run a guest Nobody forces you to run a screensaver with a daemon. > session so that I could return to the original session, though.) That is really beyond the scope of this bug. > And isn't monitor power control done by xfce4-power-manager even without a > screensaver? This script has nothing to do with power saving. It is only run by the xfsm-logout-plugin in order to lock the screen before suspending or hibernating which is done for security reasons. Either one of the above scripts addresses the bug in the original script which causes the wrong screensaver command to be run resulting in no locking.
(In reply to comment #31) > Nobody forces you to run a screensaver with a daemon. Maybe so, but such a screensaver daemon is launched in the default xinitrc, if one is installed, but there seems to be some tendency to allow configurability in Git: http://git.xfce.org/xfce/xfce-utils/tree/scripts/xinitrc.in.in?id=f9d3219f5d566f54c53b2b4f8864a5a744bd864d
(In reply to comment #29) > But anyway, here is a > portable, SUSv3 compliant variant which should work how you want it (it avoids > the 'which' bashism and the unecessary test and subshell). As for your efforts to avoid bashisms, I guess you would like to do something to xinitrc, too, since they replaced some "type" commands by "which" commands to avoid bashisms: http://git.xfce.org/xfce/xfce-utils/commit/?id=f9d3219f5d566f54c53b2b4f8864a5a744bd864d (Bug 5557)
Created attachment 3849 Slightly modified script based on Guido Berhoerster's attachment: xflock is preferred over the newcomer slock; display is turned off, when using one of them. Also comments added. This version fixes this bug and also bug 2653 and bug 5732. I changed copyright notice; if it is not accepted, I request that all authors of the script should be listed. Also note that nowadays screensaver can be specified by xfconf-query and that information can be used in xinitrc, but I think the safest bet is to try the daemons, since they may have crashed or something. (Also note that an administrator can override this script by writing xflock4 script in /usr/local/bin in xbuntu, at least.)
*** Bug 5732 has been marked as a duplicate of this bug. ***
Oki fixed in master, used the latest version.
Well, even the lastest version of the script does not check, if gnome-screensaver is running: running "gnome-screensaver-command --lock" will start gnome-screensaver daemon, if it is not running already, instead of exiting with an error. I think it would be better to use pid command to check, if the daemon is running. By the way, command xlock (or package xlockmore) is no longer available in repositories of Ubuntu 13.04 and later. I made a sibling script lxlock for LXDE or Lubuntu (but it is not in use officially, at least yet): https://launchpadlibrarian.net/157012750/lxlock (as a fix for a bug: https://bugs.launchpad.net/ubuntu/+source/lxsession/+bug/1205384) That script uses command pid to tell, if gnome-screensaver is running. Please note, that the script features i3lock as yet another alternative locker utility. i3lock can fork, so there is no need to run it in background by "&" after the command. It also can turn off screen using DPMS by option -d (or --dpms), so there is no need to use separate xset command with it. Finally, the script reiles on xscreensaver and assumes it is installed and uses it, if nothing else works, but alternatively it could exit with an error, like the current xflock4 does, if you don't want to depend on xscreensaver.
Created attachment 5287 Yet another solution that uses pidof to know if a daemon is running. Added light-locker and i3lock as locking methods. Support for light-locker has been requested by Xubuntu: https://bugs.launchpad.net/ubuntu/+source/xfce4-session/+bug/1254366 I also added "set -o errexit" so that the script will exit with error, if any command fails for any reason; tests should still work fine (and similarly I routinely added "set -o nounset" so that no one uses uninitialized variables in the script). "command -v" is used instead of "which" to tell, if a command is available, but I don't know which is more portable. "command" is a shell built-in, and thus more efficient, but that is not significant in this case. In Ubuntu "which" is provided by package debianutils on which e.g. Ubuntu's and Debian's default /bin/sh namely dash (shell) depends on, whereas "command -v" is supposed to work on "systems supporting the User Portability Utilities" and it can find even special built-in utilities and such things. (http://pubs.opengroup.org/onlinepubs/009695399/utilities/command.html)
Created attachment 5288 For completeness, require that any running daemon executable is on given PATH.
xflock4 could also support xautolock by something like if pidof "`command -v xautolock`" >/dev/null; then xautolock -locknow >/dev/null If it was checked as the first alternative, it would give an end user (or anyone that gets access to the computer) ability to run any command (that does not need sudo) by xflock4. If it was checked last, it would be arguably more secure, but some lockers could not be launched the way user wants, like "i3lock --image=/path/to/my/fancy/image.png --tiling". For another thing, "xset dpms force off" could be used after "slock &" and "xlock -mode blank &" by default (like the current xflock4 does), as they are blank lockers anyway.
One more thing about xautolock is that it does not tell in its exit code, if the configured low-level locker fails to start.
One good thing about xflock4 script is that a superuser can override it by adding a custom xflock4 script to /usr/local/bin (in Ubuntu at least).
Created attachment 5293 xflock4: Added xautolock and slimlock, and turn display off with regular blank lockers Also fixes bug https://bugzilla.xfce.org/show_bug.cgi?id=9063
I don't know how necessary it is to export a custom PATH for xflock4 (introduced in Guido's attachement), but for OpenBSD it needs updating: https://bugzilla.xfce.org/show_bug.cgi?id=8830
Anyone use these screensavers, so care to add them? The following bug report also tells gnome-screensaver is dead: https://bugzilla.xfce.org/show_bug.cgi?id=10217
Created attachment 5294 xflock4: Added support for mate-screensaver and cinnamon-screensaver and some dirs to PATH for OpenBSD
Created attachment 5295 xflock4: Updated comments, used output redirection uniformly (probably not significant), cleaned code Where do the standard output and standard error messages go, if not redirected to /dev/null, when xflock4 is called by e.g. xfce4-power-manager?
As an addition to comment https://bugzilla.xfce.org/show_bug.cgi?id=3770#c38 I found one more difference between "command -v" and "which": The latter checks that the argument is an executable file. But I guess we can assume in this case that the file is an executable, if one is found.
As a summary: The latest xflock4 I atteched is supposed to fix this bug properly and additionally fix: https://bugzilla.xfce.org/show_bug.cgi?id=9063 https://bugzilla.xfce.org/show_bug.cgi?id=10217 (but this does not remove support for gnome-screensaver) https://bugzilla.xfce.org/show_bug.cgi?id=8830 and https://bugs.launchpad.net/ubuntu/+source/xfce4-session/+bug/1254366