Review Request: Move screensaver and locking functionality to kwin from krunner

Alex Merry kde at randomguy3.me.uk
Tue Aug 9 12:53:26 UTC 2011 


> On July 15, 2011, 3:28 p.m., Martin Gräßlin wrote:
> > The code from the SoK project is now in an own branch: https://projects.kde.org/projects/kde/kdebase/kde-workspace/repository/revisions/2780fd91810bad353ac33422ce4b3eab291c4b47
> > 
> > Comparing the two I prefer the SoK commit as it doesn't add files to kwin's root directory (which I don't like any more and get's currently more and more changed)
> 
> Alex Merry wrote:
>     The issue with the SoK version is that KRunner still maintains the keyboard shortcut for locking the screen (Ctrl+Alt+L by default), and this means that the shortcut won't work when krunner is not running (such as when plasma is in netbook mode).
>     
>     The code I put in useractions.cpp should be transferred to that branch, I think.
> 
> Martin Gräßlin wrote:
>     Yes it should be transferred, but AFAIK it is not enough to just transfer the code. There is more "magic" required.
>     
>     
>     As a matter of fact I don't want it in useractions.cpp, if at all it would have to go to kwinbindings.cpp, but I would more like to have it completely in the context of screenlocker (Similar to what we just did to tabbox key handling).

OK, I've created another RR against that branch, following the same pattern as tabbox: https://git.reviewboard.kde.org/r/102261/

I'll discard this one.


- Alex


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/101943/#review4741
-----------------------------------------------------------


On July 13, 2011, 4:07 p.m., Alex Merry wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/101943/
> -----------------------------------------------------------
> 
> (Updated July 13, 2011, 4:07 p.m.)
> 
> 
> Review request for kwin, Plasma, Aaron J. Seigo, Martin Gräßlin, and Farhad Hedayati Fard.
> 
> 
> Summary
> -------
> 
> This transfers the screen locking and screensaver funcitonality wholesale from krunner to kwin.  This has been OK'd in principle by the relevant maintainers.
> 
> Most of the code is simply moved untouched.  Points of note:
> 
> I've introduced a KWIN_BUILD_SCREENSAVER option, to match the other kwin build options, like KWIN_BUILD_TABBOX.  I disabled it for Plasma active, but that may not be appropriate.
> 
> I put the screensaver stuff (creating the SaverEngine object and setting up the shortcut) in KWin::Workspace.  The shortcut stuff is actually in useractions.cpp, but this implements methods in KWin::Workspace.
> 
> I'm using the kglobalaccel D-Bus interface directly to steal the existing shortcut from KRunner.  I'm doing it like this because the KAction/KGlobalAccel interface is not rich enough to do this (probably wisely - this isn't something apps should generally be doing).  The shortcut deregistration happens in KWin rather than KRunner because I don't want to rely on the order in which KWin and KRunner are started (or even on KRunner being started at all).
> 
> 
> Diffs
> -----
> 
>   CMakeLists.txt 89d97cd 
>   kcontrol/screensaver/CMakeLists.txt e4dcc3a 
>   krunner/CMakeLists.txt 4455847 
>   krunner/README c8b9740 
>   krunner/config-xautolock.h.cmake eadb0a6 
>   krunner/dbus/org.freedesktop.ScreenSaver.xml 5efd943 
>   krunner/dbus/org.kde.screensaver.xml e700b88 
>   krunner/kcfg/kscreensaversettings.kcfg c8f76f3 
>   krunner/kcfg/kscreensaversettings.kcfgc af9133d 
>   krunner/krunnerapp.h 82db725 
>   krunner/krunnerapp.cpp c143be5 
>   krunner/lock/CMakeLists.txt cf9a67e 
>   krunner/lock/autologout.h 0c44405 
>   krunner/lock/autologout.cc c86e29a 
>   krunner/lock/config-krunner-lock.h.cmake 7bfdfd6 
>   krunner/lock/kscreenlocker.notifyrc 2955c5f 
>   krunner/lock/lockdlg.h f25e55f 
>   krunner/lock/lockdlg.cc 6367216 
>   krunner/lock/lockprocess.h 8b6d9a8 
>   krunner/lock/lockprocess.cc ecc632f 
>   krunner/lock/main.h 8a60353 
>   krunner/lock/main.cc 9f1c9b8 
>   krunner/main.cpp 84a547b 
>   krunner/screensaver/saverengine.h 3384d4a 
>   krunner/screensaver/saverengine.cpp 6c15be6 
>   krunner/screensaver/xautolock.h 3db3233 
>   krunner/screensaver/xautolock.cpp 7124215 
>   krunner/screensaver/xautolock_c.h 3b82f5c 
>   krunner/screensaver/xautolock_diy.c b9df2f8 
>   krunner/screensaver/xautolock_engine.c d6d0cf5 
>   kscreenlocker/CMakeLists.txt PRE-CREATION 
>   kscreenlocker/autologout.h PRE-CREATION 
>   kscreenlocker/autologout.cc PRE-CREATION 
>   kscreenlocker/config-kscreenlocker.h.cmake PRE-CREATION 
>   kscreenlocker/kscreenlocker.notifyrc PRE-CREATION 
>   kscreenlocker/lockdlg.h PRE-CREATION 
>   kscreenlocker/lockdlg.cc PRE-CREATION 
>   kscreenlocker/lockprocess.h PRE-CREATION 
>   kscreenlocker/lockprocess.cc PRE-CREATION 
>   kscreenlocker/main.h PRE-CREATION 
>   kscreenlocker/main.cc PRE-CREATION 
>   kwin/CMakeLists.txt 7d6ea52 
>   kwin/config-kwin.h.cmake a291859 
>   kwin/config-xautolock.h.cmake PRE-CREATION 
>   kwin/kscreensaversettings.kcfg PRE-CREATION 
>   kwin/kscreensaversettings.kcfgc PRE-CREATION 
>   kwin/screensaver/org.freedesktop.ScreenSaver.xml PRE-CREATION 
>   kwin/screensaver/org.kde.screensaver.xml PRE-CREATION 
>   kwin/screensaver/saverengine.h PRE-CREATION 
>   kwin/screensaver/saverengine.cpp PRE-CREATION 
>   kwin/screensaver/xautolock.h PRE-CREATION 
>   kwin/screensaver/xautolock.cpp PRE-CREATION 
>   kwin/screensaver/xautolock_c.h PRE-CREATION 
>   kwin/screensaver/xautolock_diy.c PRE-CREATION 
>   kwin/screensaver/xautolock_engine.c PRE-CREATION 
>   kwin/useractions.cpp 387e499 
>   kwin/workspace.h 66b9830 
>   kwin/workspace.cpp 8cf5299 
>   plasma/desktop/applets/kickoff/CMakeLists.txt bc5fa2e 
>   plasma/generic/applets/lock_logout/CMakeLists.txt 8381d46 
>   plasma/generic/containmentactions/contextmenu/CMakeLists.txt a1fc205 
>   plasma/generic/runners/sessions/CMakeLists.txt 1b8292c 
>   powerdevil/daemon/CMakeLists.txt bad3dae 
> 
> Diff: http://git.reviewboard.kde.org/r/101943/diff
> 
> 
> Testing
> -------
> 
> Allowing the screensaver to activate (both terminating the screensaver before it locks and after, with lock after 60 seconds set).
> 
> Using the lock screen action from krunner.
> 
> Stealing a non-default shortcut from KRunner (set the krunner Lock Session shortcut to another sequence, and ran KWin; KWin successfully deregistered krunner's Lock Session shortcut and assigned the key sequence to its own Lock Session shortcut).
> 
> Running KWin when no existing Lock Session shortcuts had been defined (either for krunner or kwin).  KWin successfully registered its Lock Session shortcut with the default key sequence (this is what would happen with a fresh user account).
> 
> 
> Thanks,
> 
> Alex
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20110809/4032626b/attachment-0001.html>

More information about the Plasma-devel mailing list