<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 solid;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="http://git.reviewboard.kde.org/r/101943/">http://git.reviewboard.kde.org/r/101943/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On July 15th, 2011, 3:28 p.m., <b>Martin Gräßlin</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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)</pre>
</blockquote>
<p>On July 17th, 2011, 4:41 p.m., <b>Alex Merry</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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.</pre>
</blockquote>
<p>On July 17th, 2011, 6:10 p.m., <b>Martin Gräßlin</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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).</pre>
</blockquote>
</blockquote>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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.</pre>
<br />
<p>- Alex</p>
<br />
<p>On July 13th, 2011, 4:07 p.m., Alex Merry wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://git.reviewboard.kde.org/media/rb/images/review_request_box_top_bg.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for kwin, Plasma, Aaron J. Seigo, Martin Gräßlin, and Farhad Hedayati Fard.</div>
<div>By Alex Merry.</div>
<p style="color: grey;"><i>Updated July 13, 2011, 4:07 p.m.</i></p>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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).</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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).</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>CMakeLists.txt <span style="color: grey">(89d97cd)</span></li>
<li>kcontrol/screensaver/CMakeLists.txt <span style="color: grey">(e4dcc3a)</span></li>
<li>krunner/CMakeLists.txt <span style="color: grey">(4455847)</span></li>
<li>krunner/README <span style="color: grey">(c8b9740)</span></li>
<li>krunner/config-xautolock.h.cmake <span style="color: grey">(eadb0a6)</span></li>
<li>krunner/dbus/org.freedesktop.ScreenSaver.xml <span style="color: grey">(5efd943)</span></li>
<li>krunner/dbus/org.kde.screensaver.xml <span style="color: grey">(e700b88)</span></li>
<li>krunner/kcfg/kscreensaversettings.kcfg <span style="color: grey">(c8f76f3)</span></li>
<li>krunner/kcfg/kscreensaversettings.kcfgc <span style="color: grey">(af9133d)</span></li>
<li>krunner/krunnerapp.h <span style="color: grey">(82db725)</span></li>
<li>krunner/krunnerapp.cpp <span style="color: grey">(c143be5)</span></li>
<li>krunner/lock/CMakeLists.txt <span style="color: grey">(cf9a67e)</span></li>
<li>krunner/lock/autologout.h <span style="color: grey">(0c44405)</span></li>
<li>krunner/lock/autologout.cc <span style="color: grey">(c86e29a)</span></li>
<li>krunner/lock/config-krunner-lock.h.cmake <span style="color: grey">(7bfdfd6)</span></li>
<li>krunner/lock/kscreenlocker.notifyrc <span style="color: grey">(2955c5f)</span></li>
<li>krunner/lock/lockdlg.h <span style="color: grey">(f25e55f)</span></li>
<li>krunner/lock/lockdlg.cc <span style="color: grey">(6367216)</span></li>
<li>krunner/lock/lockprocess.h <span style="color: grey">(8b6d9a8)</span></li>
<li>krunner/lock/lockprocess.cc <span style="color: grey">(ecc632f)</span></li>
<li>krunner/lock/main.h <span style="color: grey">(8a60353)</span></li>
<li>krunner/lock/main.cc <span style="color: grey">(9f1c9b8)</span></li>
<li>krunner/main.cpp <span style="color: grey">(84a547b)</span></li>
<li>krunner/screensaver/saverengine.h <span style="color: grey">(3384d4a)</span></li>
<li>krunner/screensaver/saverengine.cpp <span style="color: grey">(6c15be6)</span></li>
<li>krunner/screensaver/xautolock.h <span style="color: grey">(3db3233)</span></li>
<li>krunner/screensaver/xautolock.cpp <span style="color: grey">(7124215)</span></li>
<li>krunner/screensaver/xautolock_c.h <span style="color: grey">(3b82f5c)</span></li>
<li>krunner/screensaver/xautolock_diy.c <span style="color: grey">(b9df2f8)</span></li>
<li>krunner/screensaver/xautolock_engine.c <span style="color: grey">(d6d0cf5)</span></li>
<li>kscreenlocker/CMakeLists.txt <span style="color: grey">(PRE-CREATION)</span></li>
<li>kscreenlocker/autologout.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>kscreenlocker/autologout.cc <span style="color: grey">(PRE-CREATION)</span></li>
<li>kscreenlocker/config-kscreenlocker.h.cmake <span style="color: grey">(PRE-CREATION)</span></li>
<li>kscreenlocker/kscreenlocker.notifyrc <span style="color: grey">(PRE-CREATION)</span></li>
<li>kscreenlocker/lockdlg.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>kscreenlocker/lockdlg.cc <span style="color: grey">(PRE-CREATION)</span></li>
<li>kscreenlocker/lockprocess.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>kscreenlocker/lockprocess.cc <span style="color: grey">(PRE-CREATION)</span></li>
<li>kscreenlocker/main.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>kscreenlocker/main.cc <span style="color: grey">(PRE-CREATION)</span></li>
<li>kwin/CMakeLists.txt <span style="color: grey">(7d6ea52)</span></li>
<li>kwin/config-kwin.h.cmake <span style="color: grey">(a291859)</span></li>
<li>kwin/config-xautolock.h.cmake <span style="color: grey">(PRE-CREATION)</span></li>
<li>kwin/kscreensaversettings.kcfg <span style="color: grey">(PRE-CREATION)</span></li>
<li>kwin/kscreensaversettings.kcfgc <span style="color: grey">(PRE-CREATION)</span></li>
<li>kwin/screensaver/org.freedesktop.ScreenSaver.xml <span style="color: grey">(PRE-CREATION)</span></li>
<li>kwin/screensaver/org.kde.screensaver.xml <span style="color: grey">(PRE-CREATION)</span></li>
<li>kwin/screensaver/saverengine.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>kwin/screensaver/saverengine.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>kwin/screensaver/xautolock.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>kwin/screensaver/xautolock.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>kwin/screensaver/xautolock_c.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>kwin/screensaver/xautolock_diy.c <span style="color: grey">(PRE-CREATION)</span></li>
<li>kwin/screensaver/xautolock_engine.c <span style="color: grey">(PRE-CREATION)</span></li>
<li>kwin/useractions.cpp <span style="color: grey">(387e499)</span></li>
<li>kwin/workspace.h <span style="color: grey">(66b9830)</span></li>
<li>kwin/workspace.cpp <span style="color: grey">(8cf5299)</span></li>
<li>plasma/desktop/applets/kickoff/CMakeLists.txt <span style="color: grey">(bc5fa2e)</span></li>
<li>plasma/generic/applets/lock_logout/CMakeLists.txt <span style="color: grey">(8381d46)</span></li>
<li>plasma/generic/containmentactions/contextmenu/CMakeLists.txt <span style="color: grey">(a1fc205)</span></li>
<li>plasma/generic/runners/sessions/CMakeLists.txt <span style="color: grey">(1b8292c)</span></li>
<li>powerdevil/daemon/CMakeLists.txt <span style="color: grey">(bad3dae)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/101943/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>