> On July 13, 2011, 12:54 p.m., Aaron J. Seigo wrote: > > "I disabled it for Plasma active, but that may not be appropriate." > > > > we still need screen locking in Active, so this probably isn't entirely > > correct. what we probably want, however, is a replacement for the actual > > lock process which isn't appropriate for Active. right now we have a QML > > based thing that doesn't use passwords, and it would be very nice to use > > that as a full screen app in replacement of the desktop-appropriate lock > > app. this would also neatly remove the desktopy screen savers from Active, > > and that's just fine. > > > > in the patch i don't see where the lock process file get added to.. just > > lots and lots of code removals :) i assume this was a problem with the > > post-review script or some such and that you used `git mv` to move the > > files somewhere appropriate. i actulaly think the lock process should sit > > in the top level of kde-workspace, not hidden under whatever app launches > > it. and then we can provide both a password-based one for the desktop in > > there as well as a touch focused one; perhaps as build options, even. > > > > one other thing that probably needs to be done: the screensaver/lock window > > should be identifitied as such to the window manager, just as we now do > > with the desktop's Dashboard windows. dasbhoard marking is done with a > > simple: > > > > > > #ifdef Q_WS_X11 > > XClassHint classHint; > > classHint.res_name = DASHBOARD_WIN_NAME; > > classHint.res_class = DASHBOARD_WIN_CLASS; > > XSetClassHint(QX11Info::display(), windowId, &classHint); > > #endif > > > > the name and class are defined in the file as simply "dashboard". we could > > do the same with the lock window and then when it is showing KWin can do > > clever things like not paint any other windows. > > > > i like this patch so far, however, other than a few comments that follow > > inline. Martin also needs to comment on it, of course.
Yes, I moved the lock process to the kwin directory using git mv... clearly git diff didn't show it up properly. I can move it up to the top directory, maybe as "kscreenlocker", since that's the name of the executable. The window hinting is a good idea, but should probably go in as a separate patch. I'll upload a new version addressing the points below this evening. > On July 13, 2011, 12:54 p.m., Aaron J. Seigo wrote: > > kwin/workspace.cpp, lines 132-134 > > <http://git.reviewboard.kde.org/r/101943/diff/1/?file=26943#file26943line132> > > > > this is reduendant to line 236 and doesn't provide any useful > > initialization. the new should either be moved here, this line have a 0 > > added to it, or be removed. Oh, that was supposed to have a 0 in it, in keeping with the other initializers in that class. > On July 13, 2011, 12:54 p.m., Aaron J. Seigo wrote: > > kwin/workspace.cpp, line 236 > > <http://git.reviewboard.kde.org/r/101943/diff/1/?file=26943#file26943line236> > > > > should check KWIN_BUILD_SCREENSAVER? Yes. > On July 13, 2011, 12:54 p.m., Aaron J. Seigo wrote: > > kwin/workspace.cpp, line 516 > > <http://git.reviewboard.kde.org/r/101943/diff/1/?file=26943#file26943line516> > > > > should check KWIN_BUILD_SCREENSAVER? Yes. - Alex ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/101943/#review4680 ----------------------------------------------------------- On July 13, 2011, 11:28 a.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, 11:28 a.m.) > > > Review request for kwin, Plasma, Aaron J. Seigo, and Martin Gräßlin. > > > 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 > ----- > > 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 > kwin/CMakeLists.txt 7d6ea52 > kwin/config-kwin.h.cmake a291859 > 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 > >
_______________________________________________ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
