<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 13th, 2011, 3:38 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;">before I do a proper review I have a few questions:
1. How does this align with the work our Season of KDE student, Farhad, is doing? To my knowledge he already did what this review request provides. Could you please add him to CC to ensure that we are not duplication work.
2. What are the further plans? Are you aware of the architecture and ideas outlined in http://community.kde.org/KWin/Screenlocker ?</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;">1. I wasn't aware of anyone else working on this - I can find no trace of what Farhad has done, so I don't know whether he has already started/finished what I just did. I've added him to this RR, though.
2. I had none. I wasn't aware of that, no, although this RR is the obvious first step for the stuff in that document. I just turned up on #plasma complaining about the fact that screen locking doesn't work in netbook mode in Plasma (due to the lack of krunner), and aseigo metioned that the consensus was that locking support should be in kwin. I volunteered to do it.</pre>
<br />
<p>- Alex</p>
<br />
<p>On July 13th, 2011, 3:54 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, 3:54 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>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>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/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>