[prev in list] [next in list] [prev in thread] [next in thread]
List: kwin
Subject: Re: Review Request: Move screensaver and locking functionality to
From: "Alex Merry" <kde () randomguy3 ! me ! uk>
Date: 2011-08-09 12:53:26
Message-ID: 20110809125326.26511.2619 () vidsolbach ! de
[Download RAW message or body]
[Attachment #2 (multipart/alternative)]
> On July 15, 2011, 3:28 p.m., Martin Gr=C3=A4=C3=9Flin wrote:
> > The code from the SoK project is now in an own branch: https://projects=
.kde.org/projects/kde/kdebase/kde-workspace/repository/revisions/2780fd9181=
0bad353ac33422ce4b3eab291c4b47
> > =
> > Comparing the two I prefer the SoK commit as it doesn't add files to kw=
in's root directory (which I don't like any more and get's currently more a=
nd more changed)
> =
> Alex Merry wrote:
> The issue with the SoK version is that KRunner still maintains the ke=
yboard shortcut for locking the screen (Ctrl+Alt+L by default), and this me=
ans 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 branc=
h, I think.
> =
> Martin Gr=C3=A4=C3=9Flin wrote:
> Yes it should be transferred, but AFAIK it is not enough to just tran=
sfer 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 comp=
letely in the context of screenlocker (Similar to what we just did to tabbo=
x 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=C3=A4=C3=9Flin=
, 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 mai=
ntainers.
> =
> 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, bu=
t 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 use=
ractions.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/KGlobal=
Accel interface is not rich enough to do this (probably wisely - this isn't=
something apps should generally be doing). The shortcut deregistration ha=
ppens 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 be=
fore 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 Sessio=
n shortcut to another sequence, and ran KWin; KWin successfully deregistere=
d 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 (ei=
ther for krunner or kwin). KWin successfully registered its Lock Session s=
hortcut with the default key sequence (this is what would happen with a fre=
sh user account).
> =
> =
> Thanks,
> =
> Alex
> =
>
[Attachment #5 (text/html)]
<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>
_______________________________________________
kwin mailing list
kwin@kde.org
https://mail.kde.org/mailman/listinfo/kwin
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic