Re: When will akonadi work with Gmail?
See https://bugs.kde.org/show_bug.cgi?id=404990 . In short, it's not a bug in kmail or akonadi, but rather a "political" problem because Google themselves blocked access. Doesn't mean it shouldn't get fixed from the KDE side though, rather the opposite. But me personally don't really know what the remaining problems are. Maybe somebody else here can step up and finally help to solve it? Kind Regards, Wolfgang
Re: Maintenance status of KMid?
Am Donnerstag, 30. Mai 2019, 00:02:08 schrieb Shinjo Park: > I'd llike to ask whether KMid is maintained or not: It uses drumstick as backend AFAIK, and I think the drumstick maintainer talked about porting kmid to Qt5 somewhere. Can't find it currently though... Anyway, maybe ask at http://drumstick.sourceforge.net/ (the "Drumstick Karaoke" link actually points to kmid2 sources) Kind Regards, Wolfgang
Re: KDiff3 1.8 release.
The latest change (https://cgit.kde.org/kdiff3.git/commit/?id=638bd5a02893dde4a1927abd0c8a611b3b3ab6a1) unfortunately breaks the build here: /usr/lib/gcc/i586-suse-linux/8/../../../../i586-suse-linux/bin/ld: CMakeFiles/kdiff3part.dir/pdiff.cpp.o: in function `debugLineCheck(Diff3LineList&, int, e_SrcSelector)': /home/abuild/rpmbuild/BUILD/kdiff3-1.7.95git/src/pdiff.cpp:82: undefined reference to `kdeMain()' /usr/lib/gcc/i586-suse-linux/8/../../../../i586-suse-linux/bin/ld: /home/abuild/rpmbuild/BUILD/kdiff3-1.7.95git/src/pdiff.cpp:96: undefined reference to `kdeMain()' ... and so on. Kind Regards, Wolfgang
Re: Kdiff3 in kdereview
Am Freitag, 14. September 2018, 16:36:10 schrieb Michael Reeves: > Can some do a clean install and see if right clicking on a file brings up > the kdiff3 context menu? You mean right-clicking on a file in dolphin? Seems to work fine here with latest git master, the kdiff3 menu does show up, and seems to work as expected. That's openSUSE 42.3 with latest KF5 and Qt5 (not a clean install though). Kind Regards, Wolfgang
Re: kdiff3 status
Am Freitag, 23. Februar 2018, 17:03:37 CET schrieben Sie: > Fixed now thanks. Thank you. I can confirm that with latest master it is indeed being built here (without libkde4-devel or "patching" it), and it does work fine as well... ;-) Kind Regards, Wolfgang
Re: kdiff3 status
Am Dienstag, 20. Februar 2018, 01:47:26 schrieben Sie: > I'll take a look thanks. Is it given error out out or just not building? I should have been more detailed in the first place, sorry. It is not being built at all. From the build log: -- kabstractfileitemactionplugin.h found... NO --=> kdiff3fileitemactionplugin (KDiff3 contextmenu plugin for Konqueror/Dolphin, KDE>4.6) will not be built.) -- No automatic fallback provided current configuration is not supported for plugin... As I found out meanwhile, the problem is the check in CMakeLists.txt. The file kabstractfileitemactionplugin.h is not found, unless I install libkde4-devel as well (which of course shouldn't be necessary). If I add an unconditional "add_subdirectory(kdiff3fileitemactionplugin)" instead, it is being built and builds successfully. Kind Regards, Wolfgang
Re: kdiff3 status
Am Samstag, 17. Februar 2018, 05:03:52 schrieb Wolfgang Bauer: > While it builds fine in general, I couldn't get the kfileitemaction plugin > to build, so that it shows up in dolphin's context menu e.g. I think I found the reason meanwhile: if I additionally install the *KDE4* development files (package libkde4-devel in openSUSE) it is being built. That's a bug I think though. Kind Regards, Wolfgang
Re: kdiff3 status
I haven't looked at the code at all, I'll leave that up to others. But one thing: While it builds fine in general, I couldn't get the kfileitemaction plugin to build, so that it shows up in dolphin's context menu e.g. OTOH, the version from git://anongit.kde.org/scratch/thomasfischer/kdiff3.git (kf5 branch) works fine in this regard. Maybe you should add this commit? https://cgit.kde.org/scratch/thomasfischer/kdiff3.git/commit/?h=kf5=08f309ef1154232e356515ace4846c932c570940 (untested though, i.e. I'm not sure that this alone will help) Thanks. Kind Regards, Wolfgang
Re: Review Request 128600: Support newer hunspell versions in FindHUNSPELL.cmake
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/128600/ --- (Updated Oct. 20, 2016, 1:52 p.m.) Status -- This change has been marked as submitted. Review request for kdelibs. Changes --- Submitted with commit 2ab2745eb01f73355c490ac8d5d1837dec84fd6c by Albert Astals Cid on behalf of Wolfgang Bauer to branch KDE/4.14. Repository: kdelibs Description --- This is a "backport" of https://quickgit.kde.org/?p=lokalize.git=commitdiff=7aa9dafeb457ca3bc48a61c5dfa2e69b0ecdad6c in lokalize. I have no idea if anything actually still uses this (lokalize has been ported to KF5 and comes with its own copy, the sonnet hunspell plugin in kdelibs is disabled), but it sure doesn't hurt to "fix" it anyway I think. ;-) Diffs - cmake/modules/FindHUNSPELL.cmake 26942b5 Diff: https://git.reviewboard.kde.org/r/128600/diff/ Testing --- Built lokalize 14.12.3 (still KDE4 based) with hunspell 1.4 successfully. Before, the build aborted because a suitable hunspell couldn't be found. It also still builds fine with hunspell 1.3. Thanks, Wolfgang Bauer
Re: Review Request 128910: [kio_trash] Fill in UDS_LOCAL_PATH in UDSEntry
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/128910/#review99637 --- Ping? This can really cause bad problems and should be fixed ASAP IMHO, even if the problems exist since years. - Wolfgang Bauer On Sept. 14, 2016, 8:02 nachm., Wolfgang Bauer wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/128910/ > --- > > (Updated Sept. 14, 2016, 8:02 nachm.) > > > Review request for KDE Runtime, KDE Frameworks and David Faure. > > > Bugs: 208625, 272249, 329155, and 368104 > https://bugs.kde.org/show_bug.cgi?id=208625 > https://bugs.kde.org/show_bug.cgi?id=272249 > https://bugs.kde.org/show_bug.cgi?id=329155 > https://bugs.kde.org/show_bug.cgi?id=368104 > > > Repository: kio > > > Description > --- > > Not doing so results in PreviewJob assuming the files/folders are remote and > copying them to /tmp when generating previews. > This is especially annoying and dangerous if there are large folders in the > trash. > > KDE4's kio_trash in kde-runtime has the same problem, as the code is > basically the same, the same patch fixes it too. > > > Diffs > - > > src/ioslaves/trash/kio_trash.cpp 3810941 > > Diff: https://git.reviewboard.kde.org/r/128910/diff/ > > > Testing > --- > > Trash some files/folders, open dolphin, navigate to trash:/, and hover over > them. > Previews are still generated (if enabled), and the files/folders are not > copied to /tmp any more. > > Also tried with files on an USB stick, where a folder .Trash-XXX is used as > trash. > > > Thanks, > > Wolfgang Bauer > >
Review Request 128910: [kio_trash] Fill in UDS_LOCAL_PATH in UDSEntry
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/128910/ --- Review request for KDE Runtime, KDE Frameworks and David Faure. Bugs: 208625, 272249, 329155, and 368104 https://bugs.kde.org/show_bug.cgi?id=208625 https://bugs.kde.org/show_bug.cgi?id=272249 https://bugs.kde.org/show_bug.cgi?id=329155 https://bugs.kde.org/show_bug.cgi?id=368104 Repository: kio Description --- Not doing so results in PreviewJob assuming the files/folders are remote and copying them to /tmp when generating previews. This is especially annoying and dangerous if there are large folders in the trash. KDE4's kio_trash in kde-runtime has the same problem, as the code is basically the same, the same patch fixes it too. Diffs - src/ioslaves/trash/kio_trash.cpp 3810941 Diff: https://git.reviewboard.kde.org/r/128910/diff/ Testing --- Trash some files/folders, open dolphin, navigate to trash:/, and hover over them. Previews are still generated (if enabled), and the files/folders are not copied to /tmp any more. Also tried with files on an USB stick, where a folder .Trash-XXX is used as trash. Thanks, Wolfgang Bauer
Review Request 126659: [kio_ftp] fix display of file/directory modification time/date
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/126659/ --- Review request for KDE Frameworks, kdelibs and David Faure. Bugs: 354597 https://bugs.kde.org/show_bug.cgi?id=354597 Repository: kio Description --- - QDate() treats the year literally (i.e. 90 is really year 90, not 1990), so subtracting 1900 is wrong. - in the case when no year is mentioned in the server's reply (the year is implicit), it wasn't set to the current year, so the result was either 0 or -1. Diffs - src/ioslaves/ftp/ftp.cpp 2179179 Diff: https://git.reviewboard.kde.org/r/126659/diff/ Testing --- Connected to an FTP server with dolphin (15.12.0). The modification times/dates are now shown correctly. Thanks, Wolfgang Bauer
Re: Review Request 126659: [kio_ftp] fix display of file/directory modification time/date
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/126659/ --- (Updated Jan. 7, 2016, 12:46 p.m.) Status -- This change has been marked as submitted. Review request for KDE Frameworks, kdelibs and David Faure. Changes --- Submitted with commit 68af1d7e89b7fed136d4cc62b76c1c6ded2d94eb by Wolfgang Bauer to branch master. Bugs: 354597 https://bugs.kde.org/show_bug.cgi?id=354597 Repository: kio Description --- - QDate() treats the year literally (i.e. 90 is really year 90, not 1990), so subtracting 1900 is wrong. - Use QDate::currentDate() instead of QDateTime::currentDateTime(), we only need the current date anyway - Initialize day, month, and year to the current date instead of 0. In the case when no year is mentioned in the server's reply (the year is implicit), it wasn't set to the current year at all, so the result was either 0 or -1. Diffs - src/ioslaves/ftp/ftp.cpp 2179179 Diff: https://git.reviewboard.kde.org/r/126659/diff/ Testing --- Connected to an FTP server with dolphin (15.12.0). The modification times/dates are now shown correctly. Thanks, Wolfgang Bauer
Re: Review Request 119663: libkscreensaver: blank mouse cursor in startup code
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119663/ --- (Updated Sept. 30, 2015, 4:02 nachm.) Status -- This change has been discarded. Review request for kde-workspace. Bugs: 334762 http://bugs.kde.org/show_bug.cgi?id=334762 Repository: kde-workspace Description --- Qt's QGLWidget apparently explicitely sets a standard mouse pointer for the screensaver widget. This patch prevents the mouse cursor from being shown for KDE's OpenGL screensavers by setting a blank override cursor for the screensaver application when not run in demo mode. Diffs - kscreensaver/libkscreensaver/main.cpp 561205a Diff: https://git.reviewboard.kde.org/r/119663/diff/ Testing --- Set one of KDE's OpenGL screensavers in systemsettings->Display and Monitor->Screen Locker, like Euphoria or Solarwinds. Wait for the screensaver to kick in. Notice that the mouse pointer is blanked now, whereas it was shown without this patch. If a password is required, the mouse cursor does correctly re-appear for the greeter as it should if you move the mouse/press a key to get rid of the screensaver. I'm using the patch on my systems for over two months now and haven't noticed any unexpected results. Thanks, Wolfgang Bauer
Re: Review Request 121755: Fix input focus for KDM's dialogs when GrabInput is not active
On Juli 31, 2015, 10:24 vorm., Oswald Buddenhagen wrote: the description makes a good commit message, so please make sure you use that (apart from adding the BUG lines). Yes, of course. It's not my first commit to kde-workspace and I always handled it like that, i.e. using my description as commit message and adding the BUG lines. (I'm actually using the kdelibs template for my git commits...) ;-) - Wolfgang --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/121755/#review83215 --- On Juli 29, 2015, 11:39 vorm., Wolfgang Bauer wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/121755/ --- (Updated Juli 29, 2015, 11:39 vorm.) Review request for kde-workspace, Thomas Lübking and Oswald Buddenhagen. Bugs: 268988 and 338018 http://bugs.kde.org/show_bug.cgi?id=268988 http://bugs.kde.org/show_bug.cgi?id=338018 Repository: kde-workspace Description --- [Commit d03df616](https://projects.kde.org/projects/kde/kde-workspace/repository/revisions/d03df6169ecb291318e87099a346488c961fe1d6) made input grabbing optional in KDM. But without it, input dialogs do not correctly get focus and keyboard shortcuts don't work. KDM does call activateWindow() on opened dialogs, but this doesn't seem to have the desired effect without a window manager running. And if you hover the mouse over a widget, it visually looks like it has focus, but often it doesn't accept input anyway. This patch sets the input focus via XSetInputFocus() instead, this also has the positive side-effect that a widget retains the focus if you move the mouse away. Diffs - kdm/kfrontend/kfdialog.cpp 3f6fa84 Diff: https://git.reviewboard.kde.org/r/121755/diff/ Testing --- Tried all things mentioned in the bug reports, keyboard input and shortcuts work now in all cases. I also tested with onboard keyboards (xvkbd and kvkbd), both work fine. Before, kvkbd didn't work at all (the text input widget lost focus as soon as you moved the mouse to the OSK) and xvkbd only works if you forced the focus to the text input widget via its Focus button (from which this patch was inspired actually ;-) ). Other openSUSE users have tested this as well, and the patch is even part of openSUSE's official package since January. See also https://bugzilla.opensuse.org/show_bug.cgi?id=772344 Thanks, Wolfgang Bauer
Re: Review Request 121755: Fix input focus for KDM's dialogs when GrabInput is not active
On Juli 30, 2015, 12:05 nachm., Thomas Lübking wrote: This is Qt4, right? ::activateWindow() should be equivalent to XSetInputFocus(display, winId(), XRevertToParent, X11-time); unless a) _NET_ACTIVE_WINDOW is listed in _NET_SUPPORTED on the root window (supposed to be set and withdrawn by window managers, crash on exit?) AND b) Qt::X11BypassWindowManagerHint is NOT set on the toplevel window OR c) the window is not mapped/waiting for a mapping notification = If the problem occurs, login aside (VT1 or ssh) and export DISPLAY=:0 xprop -root | grep _NET_ACTIVE_WINDOW If that's emtpy, focus setting fails on either qt_widget_private(tlw)-topData()-waitingForMapNotify (qt bug in event handling? missing XSync(dpy, false)? events are being processed after show) or X11-time being superseded by a more recent/future XSetInputFocus call (fixed by passing CurrentTime) If the feature /is/ listed, that's a bug caused by a crashing WM = setting Qt::X11BypassWindowManagerHint will likely be sufficient. Wolfgang Bauer wrote: This is Qt4, right? Yes. = If the problem occurs, login aside (VT1 or ssh) and export DISPLAY=:0 xprop -root | grep _NET_ACTIVE_WINDOW Hm, I don't seem to be able to get the properties of kdm's root window. After opening a new login session: ``` # export DISPLAY=:1 # xprop -root No protocol specified. xprop: unable to open display ':1' ``` On fresh boot: ``` # export DISPLAY=:0 # xprop -root Invalid MIT-MAGIC-COOKIE-1 keyxprop: unable to open display ':0' ``` If I kill kwin in a running KDE4 session, the feature is listed. But that's to be expected I suppose, if I understand you correctly. And focus changes when you move the mouse, which I think is undesirable at the login screen anyway (there are not really multiple windows, just modal dialogs). If the feature /is/ listed, that's a bug caused by a crashing WM = setting Qt::X11BypassWindowManagerHint will likely be sufficient. I will try if that helps. But just to avoid a misunderstanding here: this is about kdm's login greeter. There's no window manager running at all. Thomas Lübking wrote: There's no window manager running at all. Yeah, got that ;-) xprop: unable to open display ':1' Ah, that's likely because the superuser has the X11 authority - try from a su login. And focus changes when you move the mouse That's the default X11 behavior, it will always behave like that (while unmanaged; and iirc it's suppressable by a config key/X switch) There's no window manager running at all. Yeah, got that ;-) I thought so, but I still wanted to mention it because you were talking about a crashing WM... ;-) xprop: unable to open display ':1' Ah, that's likely because the superuser has the X11 authority - try from a su login. Well, that was in fact a su login. But meanwhile I modified kdm to run xprop (at the same place that this patch modifies...). _NET_ACTIVE_WINDOW is *not* set, and _NET_SUPPORTED(ATOM) was not even mentioned in xprop's output. And focus changes when you move the mouse That's the default X11 behavior, it will always behave like that (while unmanaged; and iirc it's suppressable by a config key/X switch) I know, I even mentioned that in the openSUSE bugreport (I have to admit that I used wrong terms though...) So, is this patch ok for you? I think it should be, as Qt itself just calls XSetInputFocus(). The actual bug seems to be in Qt4, but that's unlikely to be fixed any more, isn't? - Wolfgang --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/121755/#review83168 --- On Juli 29, 2015, 11:39 vorm., Wolfgang Bauer wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/121755/ --- (Updated Juli 29, 2015, 11:39 vorm.) Review request for kde-workspace, Thomas Lübking and Oswald Buddenhagen. Bugs: 268988 and 338018 http://bugs.kde.org/show_bug.cgi?id=268988 http://bugs.kde.org/show_bug.cgi?id=338018 Repository: kde-workspace Description --- [Commit d03df616](https://projects.kde.org/projects/kde/kde-workspace/repository/revisions/d03df6169ecb291318e87099a346488c961fe1d6) made input grabbing optional in KDM. But without it, input dialogs do not correctly get focus and keyboard shortcuts don't work. KDM does call activateWindow() on opened dialogs, but this doesn't seem to have the desired effect without a window manager
Re: Review Request 121755: Fix input focus for KDM's dialogs when GrabInput is not active
On Juli 30, 2015, 12:05 nachm., Thomas Lübking wrote: This is Qt4, right? ::activateWindow() should be equivalent to XSetInputFocus(display, winId(), XRevertToParent, X11-time); unless a) _NET_ACTIVE_WINDOW is listed in _NET_SUPPORTED on the root window (supposed to be set and withdrawn by window managers, crash on exit?) AND b) Qt::X11BypassWindowManagerHint is NOT set on the toplevel window OR c) the window is not mapped/waiting for a mapping notification = If the problem occurs, login aside (VT1 or ssh) and export DISPLAY=:0 xprop -root | grep _NET_ACTIVE_WINDOW If that's emtpy, focus setting fails on either qt_widget_private(tlw)-topData()-waitingForMapNotify (qt bug in event handling? missing XSync(dpy, false)? events are being processed after show) or X11-time being superseded by a more recent/future XSetInputFocus call (fixed by passing CurrentTime) If the feature /is/ listed, that's a bug caused by a crashing WM = setting Qt::X11BypassWindowManagerHint will likely be sufficient. Wolfgang Bauer wrote: This is Qt4, right? Yes. = If the problem occurs, login aside (VT1 or ssh) and export DISPLAY=:0 xprop -root | grep _NET_ACTIVE_WINDOW Hm, I don't seem to be able to get the properties of kdm's root window. After opening a new login session: ``` # export DISPLAY=:1 # xprop -root No protocol specified. xprop: unable to open display ':1' ``` On fresh boot: ``` # export DISPLAY=:0 # xprop -root Invalid MIT-MAGIC-COOKIE-1 keyxprop: unable to open display ':0' ``` If I kill kwin in a running KDE4 session, the feature is listed. But that's to be expected I suppose, if I understand you correctly. And focus changes when you move the mouse, which I think is undesirable at the login screen anyway (there are not really multiple windows, just modal dialogs). If the feature /is/ listed, that's a bug caused by a crashing WM = setting Qt::X11BypassWindowManagerHint will likely be sufficient. I will try if that helps. But just to avoid a misunderstanding here: this is about kdm's login greeter. There's no window manager running at all. Thomas Lübking wrote: There's no window manager running at all. Yeah, got that ;-) xprop: unable to open display ':1' Ah, that's likely because the superuser has the X11 authority - try from a su login. And focus changes when you move the mouse That's the default X11 behavior, it will always behave like that (while unmanaged; and iirc it's suppressable by a config key/X switch) Wolfgang Bauer wrote: There's no window manager running at all. Yeah, got that ;-) I thought so, but I still wanted to mention it because you were talking about a crashing WM... ;-) xprop: unable to open display ':1' Ah, that's likely because the superuser has the X11 authority - try from a su login. Well, that was in fact a su login. But meanwhile I modified kdm to run xprop (at the same place that this patch modifies...). _NET_ACTIVE_WINDOW is *not* set, and _NET_SUPPORTED(ATOM) was not even mentioned in xprop's output. And focus changes when you move the mouse That's the default X11 behavior, it will always behave like that (while unmanaged; and iirc it's suppressable by a config key/X switch) I know, I even mentioned that in the openSUSE bugreport (I have to admit that I used wrong terms though...) So, is this patch ok for you? I think it should be, as Qt itself just calls XSetInputFocus(). The actual bug seems to be in Qt4, but that's unlikely to be fixed any more, isn't? Thomas Lübking wrote: The patch is certainly correct since it does the right thing, I'm rather worried *why* it's required in the first place, ie. why the Qt code doesn't work as expected here. Either the mapping condition flag is broken or there's trouble with timestamp handling. If you want to do another test on the causes, check whether you patch still works with QX11Info::appTime() instead of CurrentTime (but that's just for understanding, there's no problem with the patch itself, esp. not on local X11 servers) If you want to do another test on the causes, check whether you patch still works with QX11Info::appTime() instead of CurrentTime Yes, that seems to work. - Wolfgang --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/121755/#review83168 --- On Juli 29, 2015, 11:39 vorm., Wolfgang Bauer wrote
Re: Review Request 121755: Fix input focus for KDM's dialogs when GrabInput is not active
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/121755/ --- (Updated Aug. 4, 2015, 5:53 p.m.) Status -- This change has been marked as submitted. Review request for kde-workspace, Thomas Lübking and Oswald Buddenhagen. Changes --- Submitted with commit 46f1055ffdcd2e068296576a7824012f42e9b9a8 by Wolfgang Bauer to branch KDE/4.11. Bugs: 268988 and 338018 http://bugs.kde.org/show_bug.cgi?id=268988 http://bugs.kde.org/show_bug.cgi?id=338018 Repository: kde-workspace Description --- [Commit d03df616](https://projects.kde.org/projects/kde/kde-workspace/repository/revisions/d03df6169ecb291318e87099a346488c961fe1d6) made input grabbing optional in KDM. But without it, input dialogs do not correctly get focus and keyboard shortcuts don't work. KDM does call activateWindow() on opened dialogs, but this doesn't seem to have the desired effect without a window manager running. And if you hover the mouse over a widget, it visually looks like it has focus, but often it doesn't accept input anyway. This patch sets the input focus via XSetInputFocus() instead, this also has the positive side-effect that a widget retains the focus if you move the mouse away. Diffs - kdm/kfrontend/kfdialog.cpp 3f6fa84 Diff: https://git.reviewboard.kde.org/r/121755/diff/ Testing --- Tried all things mentioned in the bug reports, keyboard input and shortcuts work now in all cases. I also tested with onboard keyboards (xvkbd and kvkbd), both work fine. Before, kvkbd didn't work at all (the text input widget lost focus as soon as you moved the mouse to the OSK) and xvkbd only works if you forced the focus to the text input widget via its Focus button (from which this patch was inspired actually ;-) ). Other openSUSE users have tested this as well, and the patch is even part of openSUSE's official package since January. See also https://bugzilla.opensuse.org/show_bug.cgi?id=772344 Thanks, Wolfgang Bauer
Re: Review Request 121755: Fix input focus for KDM's dialogs when GrabInput is not active
On Juli 30, 2015, 12:05 nachm., Thomas Lübking wrote: This is Qt4, right? ::activateWindow() should be equivalent to XSetInputFocus(display, winId(), XRevertToParent, X11-time); unless a) _NET_ACTIVE_WINDOW is listed in _NET_SUPPORTED on the root window (supposed to be set and withdrawn by window managers, crash on exit?) AND b) Qt::X11BypassWindowManagerHint is NOT set on the toplevel window OR c) the window is not mapped/waiting for a mapping notification = If the problem occurs, login aside (VT1 or ssh) and export DISPLAY=:0 xprop -root | grep _NET_ACTIVE_WINDOW If that's emtpy, focus setting fails on either qt_widget_private(tlw)-topData()-waitingForMapNotify (qt bug in event handling? missing XSync(dpy, false)? events are being processed after show) or X11-time being superseded by a more recent/future XSetInputFocus call (fixed by passing CurrentTime) If the feature /is/ listed, that's a bug caused by a crashing WM = setting Qt::X11BypassWindowManagerHint will likely be sufficient. Wolfgang Bauer wrote: This is Qt4, right? Yes. = If the problem occurs, login aside (VT1 or ssh) and export DISPLAY=:0 xprop -root | grep _NET_ACTIVE_WINDOW Hm, I don't seem to be able to get the properties of kdm's root window. After opening a new login session: ``` # export DISPLAY=:1 # xprop -root No protocol specified. xprop: unable to open display ':1' ``` On fresh boot: ``` # export DISPLAY=:0 # xprop -root Invalid MIT-MAGIC-COOKIE-1 keyxprop: unable to open display ':0' ``` If I kill kwin in a running KDE4 session, the feature is listed. But that's to be expected I suppose, if I understand you correctly. And focus changes when you move the mouse, which I think is undesirable at the login screen anyway (there are not really multiple windows, just modal dialogs). If the feature /is/ listed, that's a bug caused by a crashing WM = setting Qt::X11BypassWindowManagerHint will likely be sufficient. I will try if that helps. But just to avoid a misunderstanding here: this is about kdm's login greeter. There's no window manager running at all. Thomas Lübking wrote: There's no window manager running at all. Yeah, got that ;-) xprop: unable to open display ':1' Ah, that's likely because the superuser has the X11 authority - try from a su login. And focus changes when you move the mouse That's the default X11 behavior, it will always behave like that (while unmanaged; and iirc it's suppressable by a config key/X switch) Wolfgang Bauer wrote: There's no window manager running at all. Yeah, got that ;-) I thought so, but I still wanted to mention it because you were talking about a crashing WM... ;-) xprop: unable to open display ':1' Ah, that's likely because the superuser has the X11 authority - try from a su login. Well, that was in fact a su login. But meanwhile I modified kdm to run xprop (at the same place that this patch modifies...). _NET_ACTIVE_WINDOW is *not* set, and _NET_SUPPORTED(ATOM) was not even mentioned in xprop's output. And focus changes when you move the mouse That's the default X11 behavior, it will always behave like that (while unmanaged; and iirc it's suppressable by a config key/X switch) I know, I even mentioned that in the openSUSE bugreport (I have to admit that I used wrong terms though...) So, is this patch ok for you? I think it should be, as Qt itself just calls XSetInputFocus(). The actual bug seems to be in Qt4, but that's unlikely to be fixed any more, isn't? Thomas Lübking wrote: The patch is certainly correct since it does the right thing, I'm rather worried *why* it's required in the first place, ie. why the Qt code doesn't work as expected here. Either the mapping condition flag is broken or there's trouble with timestamp handling. If you want to do another test on the causes, check whether you patch still works with QX11Info::appTime() instead of CurrentTime (but that's just for understanding, there's no problem with the patch itself, esp. not on local X11 servers) Wolfgang Bauer wrote: If you want to do another test on the causes, check whether you patch still works with QX11Info::appTime() instead of CurrentTime Yes, that seems to work. Thomas Lübking wrote: Ok, then it seems QWidget believes it's not mapped while it is - or it's only mapped by a sync triggered by XSetInputFocus (what then might become a problem when/if kdm is ported to xcb) Wolfgang Bauer wrote: I think that's becoming theoretical now. Who's going to port
Re: Review Request 121755: Fix input focus for KDM's dialogs when GrabInput is not active
On Juli 30, 2015, 12:05 nachm., Thomas Lübking wrote: This is Qt4, right? ::activateWindow() should be equivalent to XSetInputFocus(display, winId(), XRevertToParent, X11-time); unless a) _NET_ACTIVE_WINDOW is listed in _NET_SUPPORTED on the root window (supposed to be set and withdrawn by window managers, crash on exit?) AND b) Qt::X11BypassWindowManagerHint is NOT set on the toplevel window OR c) the window is not mapped/waiting for a mapping notification = If the problem occurs, login aside (VT1 or ssh) and export DISPLAY=:0 xprop -root | grep _NET_ACTIVE_WINDOW If that's emtpy, focus setting fails on either qt_widget_private(tlw)-topData()-waitingForMapNotify (qt bug in event handling? missing XSync(dpy, false)? events are being processed after show) or X11-time being superseded by a more recent/future XSetInputFocus call (fixed by passing CurrentTime) If the feature /is/ listed, that's a bug caused by a crashing WM = setting Qt::X11BypassWindowManagerHint will likely be sufficient. This is Qt4, right? Yes. = If the problem occurs, login aside (VT1 or ssh) and export DISPLAY=:0 xprop -root | grep _NET_ACTIVE_WINDOW Hm, I don't seem to be able to get the properties of kdm's root window. After opening a new login session: ``` # export DISPLAY=:1 # xprop -root No protocol specified. xprop: unable to open display ':1' ``` On fresh boot: ``` # export DISPLAY=:0 # xprop -root Invalid MIT-MAGIC-COOKIE-1 keyxprop: unable to open display ':0' ``` If I kill kwin in a running KDE4 session, the feature is listed. But that's to be expected I suppose, if I understand you correctly. And focus changes when you move the mouse, which I think is undesirable at the login screen anyway (there are not really multiple windows, just modal dialogs). If the feature /is/ listed, that's a bug caused by a crashing WM = setting Qt::X11BypassWindowManagerHint will likely be sufficient. I will try if that helps. But just to avoid a misunderstanding here: this is about kdm's login greeter. There's no window manager running at all. - Wolfgang --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/121755/#review83168 --- On Juli 29, 2015, 11:39 vorm., Wolfgang Bauer wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/121755/ --- (Updated Juli 29, 2015, 11:39 vorm.) Review request for kde-workspace, Thomas Lübking and Oswald Buddenhagen. Bugs: 268988 and 338018 http://bugs.kde.org/show_bug.cgi?id=268988 http://bugs.kde.org/show_bug.cgi?id=338018 Repository: kde-workspace Description --- [Commit d03df616](https://projects.kde.org/projects/kde/kde-workspace/repository/revisions/d03df6169ecb291318e87099a346488c961fe1d6) made input grabbing optional in KDM. But without it, input dialogs do not correctly get focus and keyboard shortcuts don't work. KDM does call activateWindow() on opened dialogs, but this doesn't seem to have the desired effect without a window manager running. And if you hover the mouse over a widget, it visually looks like it has focus, but often it doesn't accept input anyway. This patch sets the input focus via XSetInputFocus() instead, this also has the positive side-effect that a widget retains the focus if you move the mouse away. Diffs - kdm/kfrontend/kfdialog.cpp 3f6fa84 Diff: https://git.reviewboard.kde.org/r/121755/diff/ Testing --- Tried all things mentioned in the bug reports, keyboard input and shortcuts work now in all cases. I also tested with onboard keyboards (xvkbd and kvkbd), both work fine. Before, kvkbd didn't work at all (the text input widget lost focus as soon as you moved the mouse to the OSK) and xvkbd only works if you forced the focus to the text input widget via its Focus button (from which this patch was inspired actually ;-) ). Other openSUSE users have tested this as well, and the patch is even part of openSUSE's official package since January. See also https://bugzilla.opensuse.org/show_bug.cgi?id=772344 Thanks, Wolfgang Bauer
Review Request 121755: Fix input focus for KDM's dialogs when GrabInput is not active
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/121755/ --- Review request for kde-workspace, Thomas Lübking and Oswald Buddenhagen. Bugs: 268988 and 338018 http://bugs.kde.org/show_bug.cgi?id=268988 http://bugs.kde.org/show_bug.cgi?id=338018 Repository: kde-workspace Description --- [Commit d03df616](https://projects.kde.org/projects/kde/kde-workspace/repository/revisions/d03df6169ecb291318e87099a346488c961fe1d6) made input grabbing optional in KDM. But without it, input dialogs do not correctly get focus and keyboard shortcuts don't work. KDM does call activateWindow() on opened dialogs, but this doesn't seem to have the desired effect without a window manager running. And if you hover the mouse over a widget, it visually looks like it has focus, but often it doesn't accept input anyway. This patch sets the input focus via XSetInputFocus() instead, this also has the positive side-effect that a widget retains the focus if you move the mouse away. Diffs - kdm/kfrontend/kfdialog.cpp 3f6fa84 Diff: https://git.reviewboard.kde.org/r/121755/diff/ Testing --- Tried all things mentioned in the bug reports, keyboard input and shortcuts work now in all cases. I also tested with onboard keyboards (xvkbd and kvkbd), both work fine. Before, kvkbd didn't work at all (the text input widget lost focus as soon as you moved the mouse to the OSK) and xvkbd only works if you forced the focus to the text input widget via its Focus button (from which this patch was inspired actually ;-) ). Other openSUSE users have tested this as well, and the patch is even part of openSUSE's official package since January. See also https://bugzilla.opensuse.org/show_bug.cgi?id=772344 Thanks, Wolfgang Bauer
Re: Review Request 119663: libkscreensaver: blank mouse cursor in startup code
On Aug. 8, 2014, 5:35 nachm., Thomas Lübking wrote: Would it be sufficient to set the cursor on target instead of an application wide override? setOverrideCursor has this nasty stack behavior which could be troublesome in a library and I could eg. think of screensavers implementing a little game (ie. run kpatience or so ;-) In case they should have nice control over the cursor w/o having to rewind a stack (or using an override on top ...) Would it be sufficient to set the cursor on target instead of an application wide override? No, unfortunately not. That was the first thing I tried, but it had no effect at all (no matter whether it is done before target-show() or after). The only other way I see is explicitely set a blank mouse cursor in all of the affected screen savers (I tried this with Euphoria and it worked), but that would be much more work of course. setOverrideCursor has this nasty stack behavior which could be troublesome in a library and I could eg. think of screensavers implementing a little game (ie. run kpatience or so ;-) Does this actually make sense? When used as screensaver, it would immediately disappear anyway when you move the mouse, no? A side-note: xscreensaver's julia screensaver does something like this: it allows you to choose a new starting point for its animation by clicking with the mouse. But when used as screensaver, the xscreensaver daemon forces a blank cursor somehow (I haven't looked at the source code how exactly, but I would guess via XGrabPointer()), so even this one does not show a cursor then. When used with KDE's screen locker, the cursor does show (that's of course not changed by this patch, as that screensaver obviously doesn't use libkscreensaver), still you can do that only in demo mode/in the systemsettings preview because otherwise the screensaver disappears on every input event. - Wolfgang --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119663/#review64072 --- On Aug. 8, 2014, 2:37 nachm., Wolfgang Bauer wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119663/ --- (Updated Aug. 8, 2014, 2:37 nachm.) Review request for kde-workspace. Bugs: 334762 http://bugs.kde.org/show_bug.cgi?id=334762 Repository: kde-workspace Description --- Qt's QGLWidget apparently explicitely sets a standard mouse pointer for the screensaver widget. This patch prevents the mouse cursor from being shown for KDE's OpenGL screensavers by setting a blank override cursor for the screensaver application when not run in demo mode. Diffs - kscreensaver/libkscreensaver/main.cpp 561205a Diff: https://git.reviewboard.kde.org/r/119663/diff/ Testing --- Set one of KDE's OpenGL screensavers in systemsettings-Display and Monitor-Screen Locker, like Euphoria or Solarwinds. Wait for the screensaver to kick in. Notice that the mouse pointer is blanked now, whereas it was shown without this patch. If a password is required, the mouse cursor does correctly re-appear for the greeter as it should if you move the mouse/press a key to get rid of the screensaver. I'm using the patch on my systems for over two months now and haven't noticed any unexpected results. Thanks, Wolfgang Bauer
Review Request 119663: libkscreensaver: blank mouse cursor in startup code
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119663/ --- Review request for kde-workspace. Bugs: 334762 http://bugs.kde.org/show_bug.cgi?id=334762 Repository: kde-workspace Description --- Qt's QGLWidget apparently explicitely sets a standard mouse pointer for the screensaver widget. This patch prevents the mouse cursor from being shown for KDE's OpenGL screensavers by setting a blank override cursor for the screensaver application when not run in demo mode. Diffs - kscreensaver/libkscreensaver/main.cpp 561205a Diff: https://git.reviewboard.kde.org/r/119663/diff/ Testing --- Set one of KDE's OpenGL screensavers in systemsettings-Display and Monitor-Screen Locker, like Euphoria or Solarwinds. Wait for the screensaver to kick in. Notice that the mouse pointer is blanked now, whereas it was shown without this patch. If a password is required, the mouse cursor does correctly re-appear for the greeter as it should if you move the mouse/press a key to get rid of the screensaver. I'm using the patch on my systems for over two months now and haven't noticed any unexpected results. Thanks, Wolfgang Bauer
Re: Review Request 118947: KJS: treat specified time correctly as localtime when passed to the Date() constructor
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118947/ --- (Updated June 26, 2014, 12:03 p.m.) Status -- This change has been marked as submitted. Review request for kdelibs. Bugs: 335556 http://bugs.kde.org/show_bug.cgi?id=335556 Repository: kdelibs Description --- The commit https://projects.kde.org/projects/kde/kdelibs/repository/revisions/48dd1fc50277b861b49613c5f46b6f8b10cc932d introduced a bug: It factors out the code to convert a time specification to an actual time value into the function makeTimeFromList(). But in that function makeTime(t, ms, true) is called, whereas the original code called makeTime(t, ms, false) for the constructor. This patch fixes it by passing the utc parameter to makeTime() instead of true. Please note that this bug is also present in Frameworks5. Diffs - kjs/date_object.cpp c8d776c Diff: https://git.reviewboard.kde.org/r/118947/diff/ Testing --- Loaded the test case from the bug report into Konqueror/KHTML. The correct time is shown now like in other browsers, including Konqueror/WebKit. Thanks, Wolfgang Bauer
Re: Review Request 118898: KGamma: Apply user setting at login/startup
On June 24, 2014, 12:41 a.m., Christoph Feck wrote: Not sure why you added Marcel to the list of reviewers... Anyway, if the current patch is all that is needed to restore sanity as in KDE 3, the I am all for getting it fixed. Christoph Feck wrote: (And if the same issue was the cause for the KRandR regression, then I will eat my hat). Not sure why you added Marcel to the list of reviewers... Because he is listed as KDE Project Manager at https://projects.kde.org/projects/kde/kdegraphics/kgamma . Anyway, if the current patch is all that is needed to restore sanity as in KDE 3, the I am all for getting it fixed. It works fine here... (And if the same issue was the cause for the KRandR regression, then I will eat my hat). I don't think so. KRandR didn't use kcminit at all as far as I can see by having a quick glance at the source code. Instead it contains a script krandrstartup to apply the config, which is (still) sourced by startkde. - Wolfgang --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118898/#review60866 --- On June 23, 2014, 7:01 p.m., Wolfgang Bauer wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118898/ --- (Updated June 23, 2014, 7:01 p.m.) Review request for kde-workspace, Plasma and Marcel Wiesweg. Bugs: 218668 http://bugs.kde.org/show_bug.cgi?id=218668 Repository: kgamma Description --- KGamma's saved user settings are not applied on startup/login. The user has to enter the KCM to apply them. This makes it rather useless, as not even saving the settings system-wide really works any more. (this requires an xorg.conf which normally doesn't exist nowadays) This patch uses kcminit to apply these settings again on login. Apparently this has been forgotten when moving/porting kgamma to KDE4. PS: As there seems to be no kgamma group and this is desktop-related, I decided to add the kde-workspace and plasma groups for review. I hope that's ok... ;) Diffs - kcmkgamma/kgamma.cpp 890ba99 kcmkgamma/kgamma.desktop 3d87513 Diff: https://git.reviewboard.kde.org/r/118898/diff/ Testing --- Set a gamma value in the KGamma KCM, logout/login (or reboot), Gamma value gets set correctly. If there's no kgammarc file (or it contains no actual gamma settings), the Gamma value is not changed. It stays at what is configured for X (or its default). Thanks, Wolfgang Bauer
Re: Review Request 118898: KGamma: Apply user setting at login/startup
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118898/ --- (Updated June 24, 2014, 10:04 a.m.) Review request for kde-workspace, KDE Graphics, Plasma, and Marcel Wiesweg. Changes --- Added kdegraphics to the review groups as kgamma is part of kdegraphics Bugs: 218668 http://bugs.kde.org/show_bug.cgi?id=218668 Repository: kgamma Description --- KGamma's saved user settings are not applied on startup/login. The user has to enter the KCM to apply them. This makes it rather useless, as not even saving the settings system-wide really works any more. (this requires an xorg.conf which normally doesn't exist nowadays) This patch uses kcminit to apply these settings again on login. Apparently this has been forgotten when moving/porting kgamma to KDE4. PS: As there seems to be no kgamma group and this is desktop-related, I decided to add the kde-workspace and plasma groups for review. I hope that's ok... ;) Diffs - kcmkgamma/kgamma.cpp 890ba99 kcmkgamma/kgamma.desktop 3d87513 Diff: https://git.reviewboard.kde.org/r/118898/diff/ Testing --- Set a gamma value in the KGamma KCM, logout/login (or reboot), Gamma value gets set correctly. If there's no kgammarc file (or it contains no actual gamma settings), the Gamma value is not changed. It stays at what is configured for X (or its default). Thanks, Wolfgang Bauer
Re: Review Request 118898: KGamma: Apply user setting at login/startup
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118898/ --- (Updated June 24, 2014, 10:58 a.m.) Status -- This change has been marked as submitted. Review request for kde-workspace, KDE Graphics, Plasma, and Marcel Wiesweg. Bugs: 218668 http://bugs.kde.org/show_bug.cgi?id=218668 Repository: kgamma Description --- KGamma's saved user settings are not applied on startup/login. The user has to enter the KCM to apply them. This makes it rather useless, as not even saving the settings system-wide really works any more. (this requires an xorg.conf which normally doesn't exist nowadays) This patch uses kcminit to apply these settings again on login. Apparently this has been forgotten when moving/porting kgamma to KDE4. PS: As there seems to be no kgamma group and this is desktop-related, I decided to add the kde-workspace and plasma groups for review. I hope that's ok... ;) Diffs - kcmkgamma/kgamma.cpp 890ba99 kcmkgamma/kgamma.desktop 3d87513 Diff: https://git.reviewboard.kde.org/r/118898/diff/ Testing --- Set a gamma value in the KGamma KCM, logout/login (or reboot), Gamma value gets set correctly. If there's no kgammarc file (or it contains no actual gamma settings), the Gamma value is not changed. It stays at what is configured for X (or its default). Thanks, Wolfgang Bauer
Review Request 118898: KGamma: Apply user setting at login/startup
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/118898/
---
Review request for kde-workspace, Plasma and Marcel Wiesweg.
Bugs: 218668
http://bugs.kde.org/show_bug.cgi?id=218668
Repository: kgamma
Description
---
KGamma's saved user settings are not applied on startup/login. The user has to
enter the KCM to apply them.
This makes it rather useless, as not even saving the settings system-wide
really works any more. (this requires an xorg.conf which normally doesn't exist
nowadays)
This patch factors out the function init_kgamma() into its own source file
(init_kgamma.cpp), adds a small executable (applykgammasettings) that just
applies those settings by calling that function, and installs
applykgammasettings.desktop to ${AUTOSTART_INSTALL_DIR} that runs
applykgammasettings on login.
PS: As there seems to be no kgamma group and this is desktop-related, I decided
to add the kde-workspace and plasma groups for review. I hope that's ok... ;)
Diffs
-
kcmkgamma/CMakeLists.txt 3980023
kcmkgamma/applykgammasettings.cpp PRE-CREATION
kcmkgamma/applykgammasettings.desktop PRE-CREATION
kcmkgamma/init_kgamma.cpp PRE-CREATION
kcmkgamma/kgamma.cpp 890ba99
Diff: https://git.reviewboard.kde.org/r/118898/diff/
Testing
---
Set a gamma value in the KGamma KCM, logout/login (or reboot), Gamma value
gets set correctly.
If there's no kgammarc file (or it contains no actual gamma settings), the
Gamma value is not changed. It stays at what is configured for X (or its
default).
Thanks,
Wolfgang Bauer
Re: Review Request 118898: KGamma: Apply user setting at login/startup
On June 23, 2014, 3:21 p.m., Sebastian Kügler wrote:
I think there's a couple of problems with this approach:
- This slows down startup for everybody, not just those who changed the
setting. I'm not super-familiar with this, but isn't kcminit for this
use-case?
- Changing startup procedure this late in the game (Plasma 4.x has been on
LTS for almost a year, feature frozen for much longer)
- This particular KCM is dead in Plasma 5 (not really a reason to not fix
it in 4.x, but the feature will be lost again
Again, not super-privvy of the whole picture, but isn't color correction
the correct solution here?
applykgammasettings is only called on login for people actually installing
kgamma (it's a separate tarball and separate package on openSUSE at least).
There is no change to plasma's own startup procedure, and no change at all when
kgamma is not installed.
kcminit is actually used by kgamma right now (it calls the same function), but
that only applies when the user enters the KCM. But the point of this setting
is to be applied automatically on login/startup.
- Wolfgang
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/118898/#review60795
---
On June 23, 2014, 3:06 p.m., Wolfgang Bauer wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/118898/
---
(Updated June 23, 2014, 3:06 p.m.)
Review request for kde-workspace, Plasma and Marcel Wiesweg.
Bugs: 218668
http://bugs.kde.org/show_bug.cgi?id=218668
Repository: kgamma
Description
---
KGamma's saved user settings are not applied on startup/login. The user has
to enter the KCM to apply them.
This makes it rather useless, as not even saving the settings system-wide
really works any more. (this requires an xorg.conf which normally doesn't
exist nowadays)
This patch factors out the function init_kgamma() into its own source file
(init_kgamma.cpp), adds a small executable (applykgammasettings) that just
applies those settings by calling that function, and installs
applykgammasettings.desktop to ${AUTOSTART_INSTALL_DIR} that runs
applykgammasettings on login.
PS: As there seems to be no kgamma group and this is desktop-related, I
decided to add the kde-workspace and plasma groups for review. I hope that's
ok... ;)
Diffs
-
kcmkgamma/CMakeLists.txt 3980023
kcmkgamma/applykgammasettings.cpp PRE-CREATION
kcmkgamma/applykgammasettings.desktop PRE-CREATION
kcmkgamma/init_kgamma.cpp PRE-CREATION
kcmkgamma/kgamma.cpp 890ba99
Diff: https://git.reviewboard.kde.org/r/118898/diff/
Testing
---
Set a gamma value in the KGamma KCM, logout/login (or reboot), Gamma value
gets set correctly.
If there's no kgammarc file (or it contains no actual gamma settings), the
Gamma value is not changed. It stays at what is configured for X (or its
default).
Thanks,
Wolfgang Bauer
Re: Review Request 118898: KGamma: Apply user setting at login/startup
On June 23, 2014, 3:21 p.m., Sebastian Kügler wrote:
I think there's a couple of problems with this approach:
- This slows down startup for everybody, not just those who changed the
setting. I'm not super-familiar with this, but isn't kcminit for this
use-case?
- Changing startup procedure this late in the game (Plasma 4.x has been on
LTS for almost a year, feature frozen for much longer)
- This particular KCM is dead in Plasma 5 (not really a reason to not fix
it in 4.x, but the feature will be lost again
Again, not super-privvy of the whole picture, but isn't color correction
the correct solution here?
Wolfgang Bauer wrote:
applykgammasettings is only called on login for people actually
installing kgamma (it's a separate tarball and separate package on openSUSE
at least).
There is no change to plasma's own startup procedure, and no change at
all when kgamma is not installed.
kcminit is actually used by kgamma right now (it calls the same
function), but that only applies when the user enters the KCM. But the point
of this setting is to be applied automatically on login/startup.
PS: Sorry, kcminit does seem to be able to apply settings on login. I seem to
have confused it with something else...
I will have a look at this then.
- Wolfgang
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/118898/#review60795
---
On June 23, 2014, 3:06 p.m., Wolfgang Bauer wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/118898/
---
(Updated June 23, 2014, 3:06 p.m.)
Review request for kde-workspace, Plasma and Marcel Wiesweg.
Bugs: 218668
http://bugs.kde.org/show_bug.cgi?id=218668
Repository: kgamma
Description
---
KGamma's saved user settings are not applied on startup/login. The user has
to enter the KCM to apply them.
This makes it rather useless, as not even saving the settings system-wide
really works any more. (this requires an xorg.conf which normally doesn't
exist nowadays)
This patch factors out the function init_kgamma() into its own source file
(init_kgamma.cpp), adds a small executable (applykgammasettings) that just
applies those settings by calling that function, and installs
applykgammasettings.desktop to ${AUTOSTART_INSTALL_DIR} that runs
applykgammasettings on login.
PS: As there seems to be no kgamma group and this is desktop-related, I
decided to add the kde-workspace and plasma groups for review. I hope that's
ok... ;)
Diffs
-
kcmkgamma/CMakeLists.txt 3980023
kcmkgamma/applykgammasettings.cpp PRE-CREATION
kcmkgamma/applykgammasettings.desktop PRE-CREATION
kcmkgamma/init_kgamma.cpp PRE-CREATION
kcmkgamma/kgamma.cpp 890ba99
Diff: https://git.reviewboard.kde.org/r/118898/diff/
Testing
---
Set a gamma value in the KGamma KCM, logout/login (or reboot), Gamma value
gets set correctly.
If there's no kgammarc file (or it contains no actual gamma settings), the
Gamma value is not changed. It stays at what is configured for X (or its
default).
Thanks,
Wolfgang Bauer
Re: Review Request 118898: KGamma: Apply user setting at login/startup
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118898/ --- (Updated June 23, 2014, 7:01 p.m.) Review request for kde-workspace, Plasma and Marcel Wiesweg. Changes --- Use kcminit instead of creating a separate executable and starting it via an autostart .desktop file Bugs: 218668 http://bugs.kde.org/show_bug.cgi?id=218668 Repository: kgamma Description (updated) --- KGamma's saved user settings are not applied on startup/login. The user has to enter the KCM to apply them. This makes it rather useless, as not even saving the settings system-wide really works any more. (this requires an xorg.conf which normally doesn't exist nowadays) This patch uses kcminit to apply these settings again on login. Apparently this has been forgotten when moving/porting kgamma to KDE4. PS: As there seems to be no kgamma group and this is desktop-related, I decided to add the kde-workspace and plasma groups for review. I hope that's ok... ;) Diffs (updated) - kcmkgamma/kgamma.cpp 890ba99 kcmkgamma/kgamma.desktop 3d87513 Diff: https://git.reviewboard.kde.org/r/118898/diff/ Testing --- Set a gamma value in the KGamma KCM, logout/login (or reboot), Gamma value gets set correctly. If there's no kgammarc file (or it contains no actual gamma settings), the Gamma value is not changed. It stays at what is configured for X (or its default). Thanks, Wolfgang Bauer
Re: Review Request 118898: KGamma: Apply user setting at login/startup
On June 23, 2014, 3:21 p.m., Sebastian Kügler wrote: I think there's a couple of problems with this approach: - This slows down startup for everybody, not just those who changed the setting. I'm not super-familiar with this, but isn't kcminit for this use-case? - Changing startup procedure this late in the game (Plasma 4.x has been on LTS for almost a year, feature frozen for much longer) - This particular KCM is dead in Plasma 5 (not really a reason to not fix it in 4.x, but the feature will be lost again Again, not super-privvy of the whole picture, but isn't color correction the correct solution here? Wolfgang Bauer wrote: applykgammasettings is only called on login for people actually installing kgamma (it's a separate tarball and separate package on openSUSE at least). There is no change to plasma's own startup procedure, and no change at all when kgamma is not installed. kcminit is actually used by kgamma right now (it calls the same function), but that only applies when the user enters the KCM. But the point of this setting is to be applied automatically on login/startup. Wolfgang Bauer wrote: PS: Sorry, kcminit does seem to be able to apply settings on login. I seem to have confused it with something else... I will have a look at this then. Thanks for the hint! Kcminit works just fine. I just had to add some stuff to the .desktop file and rename/export the init function, so the patch is much simpler now. Apparently it worked the way it is in KDE3 and has been forgotten to be changed when porting/moving kgamma to KDE4? - Wolfgang --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118898/#review60795 --- On June 23, 2014, 7:01 p.m., Wolfgang Bauer wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118898/ --- (Updated June 23, 2014, 7:01 p.m.) Review request for kde-workspace, Plasma and Marcel Wiesweg. Bugs: 218668 http://bugs.kde.org/show_bug.cgi?id=218668 Repository: kgamma Description --- KGamma's saved user settings are not applied on startup/login. The user has to enter the KCM to apply them. This makes it rather useless, as not even saving the settings system-wide really works any more. (this requires an xorg.conf which normally doesn't exist nowadays) This patch uses kcminit to apply these settings again on login. Apparently this has been forgotten when moving/porting kgamma to KDE4. PS: As there seems to be no kgamma group and this is desktop-related, I decided to add the kde-workspace and plasma groups for review. I hope that's ok... ;) Diffs - kcmkgamma/kgamma.cpp 890ba99 kcmkgamma/kgamma.desktop 3d87513 Diff: https://git.reviewboard.kde.org/r/118898/diff/ Testing --- Set a gamma value in the KGamma KCM, logout/login (or reboot), Gamma value gets set correctly. If there's no kgammarc file (or it contains no actual gamma settings), the Gamma value is not changed. It stays at what is configured for X (or its default). Thanks, Wolfgang Bauer
Re: Review Request 118898: KGamma: Apply user setting at login/startup
On June 23, 2014, 5:13 p.m., Martin Gräßlin wrote: kcmkgamma/init_kgamma.cpp, lines 1-8 https://git.reviewboard.kde.org/r/118898/diff/1/?file=283881#file283881line1 please use a copyright header as in http://techbase.kde.org/Policies/Licensing_Policy#GPL_Header I just copied the license from the original source file (kgamma.cpp). But the new source file (init_kgamma.cpp) doesn't exist any more in the updated patch. On June 23, 2014, 5:13 p.m., Martin Gräßlin wrote: kcmkgamma/init_kgamma.cpp, line 39 https://git.reviewboard.kde.org/r/118898/diff/1/?file=283881#file283881line39 why delete config? I would just use a KSharedConfig::openConfig I just copy/pasted the original code. That is reverted as well now. Should I change this anyway? But that's not really related to this bugfix, I'd say... - Wolfgang --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118898/#review60809 --- On June 23, 2014, 7:01 p.m., Wolfgang Bauer wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118898/ --- (Updated June 23, 2014, 7:01 p.m.) Review request for kde-workspace, Plasma and Marcel Wiesweg. Bugs: 218668 http://bugs.kde.org/show_bug.cgi?id=218668 Repository: kgamma Description --- KGamma's saved user settings are not applied on startup/login. The user has to enter the KCM to apply them. This makes it rather useless, as not even saving the settings system-wide really works any more. (this requires an xorg.conf which normally doesn't exist nowadays) This patch uses kcminit to apply these settings again on login. Apparently this has been forgotten when moving/porting kgamma to KDE4. PS: As there seems to be no kgamma group and this is desktop-related, I decided to add the kde-workspace and plasma groups for review. I hope that's ok... ;) Diffs - kcmkgamma/kgamma.cpp 890ba99 kcmkgamma/kgamma.desktop 3d87513 Diff: https://git.reviewboard.kde.org/r/118898/diff/ Testing --- Set a gamma value in the KGamma KCM, logout/login (or reboot), Gamma value gets set correctly. If there's no kgammarc file (or it contains no actual gamma settings), the Gamma value is not changed. It stays at what is configured for X (or its default). Thanks, Wolfgang Bauer
Re: Review Request 118898: KGamma: Apply user setting at login/startup
On June 23, 2014, 5:13 p.m., Martin Gräßlin wrote: kcmkgamma/init_kgamma.cpp, line 39 https://git.reviewboard.kde.org/r/118898/diff/1/?file=283881#file283881line39 why delete config? I would just use a KSharedConfig::openConfig Wolfgang Bauer wrote: I just copy/pasted the original code. That is reverted as well now. Should I change this anyway? But that's not really related to this bugfix, I'd say... Martin Gräßlin wrote: But that's not really related to this bugfix, I'd say... no, it isn't. It just highlights how bad reviewboard is for copied content OK. I dropped this issue then. - Wolfgang --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118898/#review60809 --- On June 23, 2014, 7:01 p.m., Wolfgang Bauer wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118898/ --- (Updated June 23, 2014, 7:01 p.m.) Review request for kde-workspace, Plasma and Marcel Wiesweg. Bugs: 218668 http://bugs.kde.org/show_bug.cgi?id=218668 Repository: kgamma Description --- KGamma's saved user settings are not applied on startup/login. The user has to enter the KCM to apply them. This makes it rather useless, as not even saving the settings system-wide really works any more. (this requires an xorg.conf which normally doesn't exist nowadays) This patch uses kcminit to apply these settings again on login. Apparently this has been forgotten when moving/porting kgamma to KDE4. PS: As there seems to be no kgamma group and this is desktop-related, I decided to add the kde-workspace and plasma groups for review. I hope that's ok... ;) Diffs - kcmkgamma/kgamma.cpp 890ba99 kcmkgamma/kgamma.desktop 3d87513 Diff: https://git.reviewboard.kde.org/r/118898/diff/ Testing --- Set a gamma value in the KGamma KCM, logout/login (or reboot), Gamma value gets set correctly. If there's no kgammarc file (or it contains no actual gamma settings), the Gamma value is not changed. It stays at what is configured for X (or its default). Thanks, Wolfgang Bauer
Review Request 118563: kscreenlocker_greet: use SA_RESTART for signal handler
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118563/ --- Review request for kde-workspace. Repository: kde-workspace Description --- As discussed in https://git.reviewboard.kde.org/r/117091/. Not using the SA_RESTART flag might (in theory) cause the greeter to be aborted (because certain syscalls may be interrupted and fail with EINTR). SA_RESTART seems to be the BSD default and is used by legacy signal() by default in glibc 2 and later as well, anyway. Diffs - ksmserver/screenlocker/greeter/main.cpp 4cac94c Diff: https://git.reviewboard.kde.org/r/118563/diff/ Testing --- I'm using this myself for one month without any problems. Thanks, Wolfgang Bauer
Re: Review Request 118563: kscreenlocker_greet: use SA_RESTART for signal handler
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118563/ --- (Updated June 5, 2014, 4:20 p.m.) Status -- This change has been marked as submitted. Review request for kde-workspace. Repository: kde-workspace Description --- As discussed in https://git.reviewboard.kde.org/r/117091/. Not using the SA_RESTART flag might (in theory) cause the greeter to be aborted (because certain syscalls may be interrupted and fail with EINTR). SA_RESTART seems to be the BSD default and is used by legacy signal() by default in glibc 2 and later as well, anyway. Diffs - ksmserver/screenlocker/greeter/main.cpp 4cac94c Diff: https://git.reviewboard.kde.org/r/118563/diff/ Testing --- I'm using this myself for one month without any problems. Thanks, Wolfgang Bauer
Re: Review Request 117091: Force the screen locker's greeter to show the password input field in case of immediateLock
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117091/ --- (Updated April 24, 2014, 10:52 a.m.) Review request for kde-workspace, Plasma and Aaron J. Seigo. Changes --- use the global qApp* pointer for calling the method from the signal handler. Bugs: 327947 and 329076 http://bugs.kde.org/show_bug.cgi?id=327947 http://bugs.kde.org/show_bug.cgi?id=329076 Repository: kde-workspace Description --- If the screen locker is set to not require a password to unlock, it will not show the password input field even when the powermanagement settings suspend the system and are set to require a password after resume (when it was already running at that point). This locks people out of their system. This patch adds a signal handler for SIGUSR1 that switches the running greeter to immediateLock mode. The locker sends that signal to make sure the greeter shows the password input field when necessary. Diffs (updated) - ksmserver/screenlocker/greeter/greeterapp.h 8b79188 ksmserver/screenlocker/greeter/greeterapp.cpp c5e2f85 ksmserver/screenlocker/greeter/main.cpp d898734 ksmserver/screenlocker/ksldapp.cpp 3dfcc9e Diff: https://git.reviewboard.kde.org/r/117091/diff/ Testing --- Disable Require password after in the screen locker settings (the default), set it to start after 1 min. (for easier testing). Enable Suspend session after and set it to 2 minutes. (set the action to Suspend, Hibernate, or Lock Screen, doesn't matter) Make sure Lock screen on resume is enabled in the powermanagements Advanced Options (it is by default). After 1 minute the screen locker kicks in, and doesn't require a password. After 2 minutes the session gets suspended, hibernated or locked, and requires a password to resume. Without this patch no password dialog is shown, the user cannot resume the session by entering the password. With this patch this works: there is a password input field, the session is unlocked when the user enters the password. Thanks, Wolfgang Bauer
Re: Review Request 117091: Force the screen locker's greeter to show the password input field in case of immediateLock
On April 23, 2014, 5:43 p.m., Martin Gräßlin wrote: ksmserver/screenlocker/greeter/main.cpp, line 34 https://git.reviewboard.kde.org/r/117091/diff/5/?file=267770#file267770line34 I'm wondering about the variable naming. It's m_ so one would assume it's a member variable, but that looks more like a static variable. So better make it static and use the s_ prefix Wolfgang Bauer wrote: You're right. It is no member variable. No idea why I chose that name. It is a pointer to the actual UnlockApp object, so that the signalhandler can call its methods. Martin Gräßlin wrote: thinking about it: you don't need the pointer at all. You have the qApp* static pointer which you can just static_cast to UnlockApp* Oh. I wasn't aware of this. That's even better of course... ;) - Wolfgang --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117091/#review56294 --- On April 24, 2014, 10:52 a.m., Wolfgang Bauer wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117091/ --- (Updated April 24, 2014, 10:52 a.m.) Review request for kde-workspace, Plasma and Aaron J. Seigo. Bugs: 327947 and 329076 http://bugs.kde.org/show_bug.cgi?id=327947 http://bugs.kde.org/show_bug.cgi?id=329076 Repository: kde-workspace Description --- If the screen locker is set to not require a password to unlock, it will not show the password input field even when the powermanagement settings suspend the system and are set to require a password after resume (when it was already running at that point). This locks people out of their system. This patch adds a signal handler for SIGUSR1 that switches the running greeter to immediateLock mode. The locker sends that signal to make sure the greeter shows the password input field when necessary. Diffs - ksmserver/screenlocker/greeter/greeterapp.h 8b79188 ksmserver/screenlocker/greeter/greeterapp.cpp c5e2f85 ksmserver/screenlocker/greeter/main.cpp d898734 ksmserver/screenlocker/ksldapp.cpp 3dfcc9e Diff: https://git.reviewboard.kde.org/r/117091/diff/ Testing --- Disable Require password after in the screen locker settings (the default), set it to start after 1 min. (for easier testing). Enable Suspend session after and set it to 2 minutes. (set the action to Suspend, Hibernate, or Lock Screen, doesn't matter) Make sure Lock screen on resume is enabled in the powermanagements Advanced Options (it is by default). After 1 minute the screen locker kicks in, and doesn't require a password. After 2 minutes the session gets suspended, hibernated or locked, and requires a password to resume. Without this patch no password dialog is shown, the user cannot resume the session by entering the password. With this patch this works: there is a password input field, the session is unlocked when the user enters the password. Thanks, Wolfgang Bauer
Re: Review Request 117644: screenlocker: don't leave behind screensaver processes
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117644/ --- (Updated April 24, 2014, 3:42 p.m.) Review request for kde-workspace and Plasma. Changes --- Another update to reflect the changes in RR#117091 Bugs: 224200 http://bugs.kde.org/show_bug.cgi?id=224200 Repository: kde-workspace Description --- Currently the screen locker just kills the greeter (kscreenlocker_greet) when the screen is unlocked by the user during the grace time. But apparently this can leave behind running screensaver processes launched by the greeter, see the bug report (which has the highest number of votes of all open bugs AFAICS). This patch changes this to only terminate the greeter, and adds a signal handler to the greeter to exit gracefully in this case. The signal handler exits with return code 1, so that it is not possible to circumvent the password input by just sending a SIGTERM. (the screen locker restarts the greeter in case it doesn't quit with exit code 0) Diffs (updated) - ksmserver/screenlocker/greeter/main.cpp d898734 ksmserver/screenlocker/ksldapp.cpp 3dfcc9e Diff: https://git.reviewboard.kde.org/r/117644/diff/ Testing --- Configure a legacy screensaver in Systemsettings-Display and Monitor-Screen Locker, be sure to leave Require Password after disabled. Wait for the screen locker to kick in. Unlock the screen by moving the mouse or pressing a key. Check the process list. Without this patch at least kswarm.kss and kblankscreen.kss reliably kept running after unlocking the screen on my system. With this patch they quit themselves. I'm using this patch for over two weeks now, and I haven't seen any left-over screen saver processes any more (and I even set the timeout to 1 minute). I also tried to terminate kscreenlocker_greet manually by running killall kscreenlocker_greet from a text console in case of a password required, and the locker didn't quit, you still have to enter the password. Thanks, Wolfgang Bauer
Re: Review Request 117091: Force the screen locker's greeter to show the password input field in case of immediateLock
On April 24, 2014, 10:01 p.m., Thomas Lübking wrote: ksmserver/screenlocker/greeter/main.cpp, line 94 https://git.reviewboard.kde.org/r/117091/diff/7/?file=268053#file268053line94 was there a problem with SA_RESTART? --- KDE Workspaces 4.11.9 tagging: Fr 25. Apr 01:59:00 CEST 2014 Would you be ok to give Martin until Fr 25. Apr 00:01:00 CEST 2014 and then push it (on my responsibility)? Wolfgang Bauer wrote: Haven't tried SA_RESTART. I will do so now. It's ok for me to push at Fr 25. Apr 00:01:00 CEST 2014, so I will at least wait until then. Changed it to SA_RESTART now, let the system automatically suspend/hibernate a few times, and also started kscreenlocker_greet manually and sent it SIGUSR1 a few times. This worked as expected too, I haven't experienced any problems. So I suppose SA_RESTART is ok as well. - Wolfgang --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117091/#review56415 --- On April 24, 2014, 10:52 a.m., Wolfgang Bauer wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117091/ --- (Updated April 24, 2014, 10:52 a.m.) Review request for kde-workspace, Plasma and Aaron J. Seigo. Bugs: 327947 and 329076 http://bugs.kde.org/show_bug.cgi?id=327947 http://bugs.kde.org/show_bug.cgi?id=329076 Repository: kde-workspace Description --- If the screen locker is set to not require a password to unlock, it will not show the password input field even when the powermanagement settings suspend the system and are set to require a password after resume (when it was already running at that point). This locks people out of their system. This patch adds a signal handler for SIGUSR1 that switches the running greeter to immediateLock mode. The locker sends that signal to make sure the greeter shows the password input field when necessary. Diffs - ksmserver/screenlocker/greeter/greeterapp.h 8b79188 ksmserver/screenlocker/greeter/greeterapp.cpp c5e2f85 ksmserver/screenlocker/greeter/main.cpp d898734 ksmserver/screenlocker/ksldapp.cpp 3dfcc9e Diff: https://git.reviewboard.kde.org/r/117091/diff/ Testing --- Disable Require password after in the screen locker settings (the default), set it to start after 1 min. (for easier testing). Enable Suspend session after and set it to 2 minutes. (set the action to Suspend, Hibernate, or Lock Screen, doesn't matter) Make sure Lock screen on resume is enabled in the powermanagements Advanced Options (it is by default). After 1 minute the screen locker kicks in, and doesn't require a password. After 2 minutes the session gets suspended, hibernated or locked, and requires a password to resume. Without this patch no password dialog is shown, the user cannot resume the session by entering the password. With this patch this works: there is a password input field, the session is unlocked when the user enters the password. Thanks, Wolfgang Bauer
Re: Review Request 117091: Force the screen locker's greeter to show the password input field in case of immediateLock
On April 24, 2014, 10:01 p.m., Thomas Lübking wrote: ksmserver/screenlocker/greeter/main.cpp, line 94 https://git.reviewboard.kde.org/r/117091/diff/7/?file=268053#file268053line94 was there a problem with SA_RESTART? --- KDE Workspaces 4.11.9 tagging: Fr 25. Apr 01:59:00 CEST 2014 Would you be ok to give Martin until Fr 25. Apr 00:01:00 CEST 2014 and then push it (on my responsibility)? Wolfgang Bauer wrote: Haven't tried SA_RESTART. I will do so now. It's ok for me to push at Fr 25. Apr 00:01:00 CEST 2014, so I will at least wait until then. Wolfgang Bauer wrote: Changed it to SA_RESTART now, let the system automatically suspend/hibernate a few times, and also started kscreenlocker_greet manually and sent it SIGUSR1 a few times. This worked as expected too, I haven't experienced any problems. So I suppose SA_RESTART is ok as well. Thomas Lübking wrote: SA_RESTART seems the BSD default and used by legacy signal() Thus I was actually rather concerned that w/o SA_RESTART some EINTR could lead into an abort - though even in that case we'd just end up with a restarted greeter. Since you've been running it w/o so far, I'd sugest to push that version and keep testing SA_RESTART afterwards to change the behavior in master and eventual 4.11.10 to use SA_RESTART then. Ie. keep 0 for now as that's the better tested variant. OK. I'm more comfortable with that as well, anyway. - Wolfgang --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117091/#review56415 --- On April 24, 2014, 10:52 a.m., Wolfgang Bauer wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117091/ --- (Updated April 24, 2014, 10:52 a.m.) Review request for kde-workspace, Plasma and Aaron J. Seigo. Bugs: 327947 and 329076 http://bugs.kde.org/show_bug.cgi?id=327947 http://bugs.kde.org/show_bug.cgi?id=329076 Repository: kde-workspace Description --- If the screen locker is set to not require a password to unlock, it will not show the password input field even when the powermanagement settings suspend the system and are set to require a password after resume (when it was already running at that point). This locks people out of their system. This patch adds a signal handler for SIGUSR1 that switches the running greeter to immediateLock mode. The locker sends that signal to make sure the greeter shows the password input field when necessary. Diffs - ksmserver/screenlocker/greeter/greeterapp.h 8b79188 ksmserver/screenlocker/greeter/greeterapp.cpp c5e2f85 ksmserver/screenlocker/greeter/main.cpp d898734 ksmserver/screenlocker/ksldapp.cpp 3dfcc9e Diff: https://git.reviewboard.kde.org/r/117091/diff/ Testing --- Disable Require password after in the screen locker settings (the default), set it to start after 1 min. (for easier testing). Enable Suspend session after and set it to 2 minutes. (set the action to Suspend, Hibernate, or Lock Screen, doesn't matter) Make sure Lock screen on resume is enabled in the powermanagements Advanced Options (it is by default). After 1 minute the screen locker kicks in, and doesn't require a password. After 2 minutes the session gets suspended, hibernated or locked, and requires a password to resume. Without this patch no password dialog is shown, the user cannot resume the session by entering the password. With this patch this works: there is a password input field, the session is unlocked when the user enters the password. Thanks, Wolfgang Bauer
Re: Review Request 117091: Force the screen locker's greeter to show the password input field in case of immediateLock
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117091/ --- (Updated April 24, 2014, 10:20 p.m.) Status -- This change has been marked as submitted. Review request for kde-workspace, Plasma and Aaron J. Seigo. Bugs: 327947 and 329076 http://bugs.kde.org/show_bug.cgi?id=327947 http://bugs.kde.org/show_bug.cgi?id=329076 Repository: kde-workspace Description --- If the screen locker is set to not require a password to unlock, it will not show the password input field even when the powermanagement settings suspend the system and are set to require a password after resume (when it was already running at that point). This locks people out of their system. This patch adds a signal handler for SIGUSR1 that switches the running greeter to immediateLock mode. The locker sends that signal to make sure the greeter shows the password input field when necessary. Diffs - ksmserver/screenlocker/greeter/greeterapp.h 8b79188 ksmserver/screenlocker/greeter/greeterapp.cpp c5e2f85 ksmserver/screenlocker/greeter/main.cpp d898734 ksmserver/screenlocker/ksldapp.cpp 3dfcc9e Diff: https://git.reviewboard.kde.org/r/117091/diff/ Testing --- Disable Require password after in the screen locker settings (the default), set it to start after 1 min. (for easier testing). Enable Suspend session after and set it to 2 minutes. (set the action to Suspend, Hibernate, or Lock Screen, doesn't matter) Make sure Lock screen on resume is enabled in the powermanagements Advanced Options (it is by default). After 1 minute the screen locker kicks in, and doesn't require a password. After 2 minutes the session gets suspended, hibernated or locked, and requires a password to resume. Without this patch no password dialog is shown, the user cannot resume the session by entering the password. With this patch this works: there is a password input field, the session is unlocked when the user enters the password. Thanks, Wolfgang Bauer
Re: Review Request 117644: screenlocker: don't leave behind screensaver processes
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117644/ --- (Updated April 24, 2014, 10:28 p.m.) Status -- This change has been marked as submitted. Review request for kde-workspace and Plasma. Bugs: 224200 http://bugs.kde.org/show_bug.cgi?id=224200 Repository: kde-workspace Description --- Currently the screen locker just kills the greeter (kscreenlocker_greet) when the screen is unlocked by the user during the grace time. But apparently this can leave behind running screensaver processes launched by the greeter, see the bug report (which has the highest number of votes of all open bugs AFAICS). This patch changes this to only terminate the greeter, and adds a signal handler to the greeter to exit gracefully in this case. The signal handler exits with return code 1, so that it is not possible to circumvent the password input by just sending a SIGTERM. (the screen locker restarts the greeter in case it doesn't quit with exit code 0) Diffs - ksmserver/screenlocker/greeter/main.cpp d898734 ksmserver/screenlocker/ksldapp.cpp 3dfcc9e Diff: https://git.reviewboard.kde.org/r/117644/diff/ Testing --- Configure a legacy screensaver in Systemsettings-Display and Monitor-Screen Locker, be sure to leave Require Password after disabled. Wait for the screen locker to kick in. Unlock the screen by moving the mouse or pressing a key. Check the process list. Without this patch at least kswarm.kss and kblankscreen.kss reliably kept running after unlocking the screen on my system. With this patch they quit themselves. I'm using this patch for over two weeks now, and I haven't seen any left-over screen saver processes any more (and I even set the timeout to 1 minute). I also tried to terminate kscreenlocker_greet manually by running killall kscreenlocker_greet from a text console in case of a password required, and the locker didn't quit, you still have to enter the password. Thanks, Wolfgang Bauer
Re: Review Request 117091: Force the screen locker's greeter to show the password input field in case of immediateLock
On April 23, 2014, 7:41 a.m., Martin Gräßlin wrote: Is that only relevant for the legacy (XSS) locker or also for the new locker? I'm just wondering whether it needs to be ported to master Yes. I just tried, and the screen locker in master does have the same problem. I wasn't able yet to get powerdevil working with plasma next, but when I call the Lock() method via DBUS while the locker is already running and still in the grace period (I manually created a ~/.config/kscreensaverrc for that), the password input field is missing there as well, so you cannot unlock. - Wolfgang --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117091/#review56241 --- On April 22, 2014, 10:34 p.m., Wolfgang Bauer wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117091/ --- (Updated April 22, 2014, 10:34 p.m.) Review request for kde-workspace, Plasma and Aaron J. Seigo. Bugs: 327947 and 329076 http://bugs.kde.org/show_bug.cgi?id=327947 http://bugs.kde.org/show_bug.cgi?id=329076 Repository: kde-workspace Description --- If the screen locker is set to not require a password to unlock, it will not show the password input field even when the powermanagement settings suspend the system and are set to require a password after resume (when it was already running at that point). This locks people out of their system. This patch adds a signal handler for SIGUSR1 that switches the running greeter to immediateLock mode. The locker sends that signal to make sure the greeter shows the password input field when necessary. Diffs - ksmserver/screenlocker/greeter/greeterapp.cpp c5e2f85 ksmserver/screenlocker/greeter/main.cpp d898734 ksmserver/screenlocker/greeter/greeterapp.h 8b79188 ksmserver/screenlocker/ksldapp.cpp 3dfcc9e Diff: https://git.reviewboard.kde.org/r/117091/diff/ Testing --- Disable Require password after in the screen locker settings (the default), set it to start after 1 min. (for easier testing). Enable Suspend session after and set it to 2 minutes. (set the action to Suspend, Hibernate, or Lock Screen, doesn't matter) Make sure Lock screen on resume is enabled in the powermanagements Advanced Options (it is by default). After 1 minute the screen locker kicks in, and doesn't require a password. After 2 minutes the session gets suspended, hibernated or locked, and requires a password to resume. Without this patch no password dialog is shown, the user cannot resume the session by entering the password. With this patch this works: there is a password input field, the session is unlocked when the user enters the password. Thanks, Wolfgang Bauer
Re: Review Request 117644: screenlocker: don't leave behind screensaver processes
On April 23, 2014, 7:36 a.m., Martin Gräßlin wrote: would you please also adapt that for plasma-workspace repo (new master)? Yes, I will. Should I create a new review request for that, or should I just submit it? - Wolfgang --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117644/#review56240 --- On April 22, 2014, 10:41 p.m., Wolfgang Bauer wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117644/ --- (Updated April 22, 2014, 10:41 p.m.) Review request for kde-workspace and Plasma. Bugs: 224200 http://bugs.kde.org/show_bug.cgi?id=224200 Repository: kde-workspace Description --- Currently the screen locker just kills the greeter (kscreenlocker_greet) when the screen is unlocked by the user during the grace time. But apparently this can leave behind running screensaver processes launched by the greeter, see the bug report (which has the highest number of votes of all open bugs AFAICS). This patch changes this to only terminate the greeter, and adds a signal handler to the greeter to exit gracefully in this case. The signal handler exits with return code 1, so that it is not possible to circumvent the password input by just sending a SIGTERM. (the screen locker restarts the greeter in case it doesn't quit with exit code 0) Diffs - ksmserver/screenlocker/greeter/main.cpp d898734 ksmserver/screenlocker/ksldapp.cpp 3dfcc9e Diff: https://git.reviewboard.kde.org/r/117644/diff/ Testing --- Configure a legacy screensaver in Systemsettings-Display and Monitor-Screen Locker, be sure to leave Require Password after disabled. Wait for the screen locker to kick in. Unlock the screen by moving the mouse or pressing a key. Check the process list. Without this patch at least kswarm.kss and kblankscreen.kss reliably kept running after unlocking the screen on my system. With this patch they quit themselves. I'm using this patch for over two weeks now, and I haven't seen any left-over screen saver processes any more (and I even set the timeout to 1 minute). I also tried to terminate kscreenlocker_greet manually by running killall kscreenlocker_greet from a text console in case of a password required, and the locker didn't quit, you still have to enter the password. Thanks, Wolfgang Bauer
Re: Review Request 117091: Force the screen locker's greeter to show the password input field in case of immediateLock
On April 23, 2014, 7:41 a.m., Martin Gräßlin wrote: Is that only relevant for the legacy (XSS) locker or also for the new locker? I'm just wondering whether it needs to be ported to master Wolfgang Bauer wrote: Yes. I just tried, and the screen locker in master does have the same problem. I wasn't able yet to get powerdevil working with plasma next, but when I call the Lock() method via DBUS while the locker is already running and still in the grace period (I manually created a ~/.config/kscreensaverrc for that), the password input field is missing there as well, so you cannot unlock. I have adapted the patch to master as well: https://build.opensuse.org/package/view_file/home:wolfi323:branches:KDE:Unstable:Frameworks/plasma-workspace5/screenlocker-always-show-password-dialog-when-needed.patch?expand=0 I tested it and it fixes the problem there too. Should I create a new review request for this? There are no drastic differences to the KDE4 patch anyway. - Wolfgang --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117091/#review56241 --- On April 22, 2014, 10:34 p.m., Wolfgang Bauer wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117091/ --- (Updated April 22, 2014, 10:34 p.m.) Review request for kde-workspace, Plasma and Aaron J. Seigo. Bugs: 327947 and 329076 http://bugs.kde.org/show_bug.cgi?id=327947 http://bugs.kde.org/show_bug.cgi?id=329076 Repository: kde-workspace Description --- If the screen locker is set to not require a password to unlock, it will not show the password input field even when the powermanagement settings suspend the system and are set to require a password after resume (when it was already running at that point). This locks people out of their system. This patch adds a signal handler for SIGUSR1 that switches the running greeter to immediateLock mode. The locker sends that signal to make sure the greeter shows the password input field when necessary. Diffs - ksmserver/screenlocker/greeter/greeterapp.cpp c5e2f85 ksmserver/screenlocker/greeter/main.cpp d898734 ksmserver/screenlocker/greeter/greeterapp.h 8b79188 ksmserver/screenlocker/ksldapp.cpp 3dfcc9e Diff: https://git.reviewboard.kde.org/r/117091/diff/ Testing --- Disable Require password after in the screen locker settings (the default), set it to start after 1 min. (for easier testing). Enable Suspend session after and set it to 2 minutes. (set the action to Suspend, Hibernate, or Lock Screen, doesn't matter) Make sure Lock screen on resume is enabled in the powermanagements Advanced Options (it is by default). After 1 minute the screen locker kicks in, and doesn't require a password. After 2 minutes the session gets suspended, hibernated or locked, and requires a password to resume. Without this patch no password dialog is shown, the user cannot resume the session by entering the password. With this patch this works: there is a password input field, the session is unlocked when the user enters the password. Thanks, Wolfgang Bauer
Re: Review Request 117091: Force the screen locker's greeter to show the password input field in case of immediateLock
On April 23, 2014, 5:43 p.m., Martin Gräßlin wrote: ksmserver/screenlocker/greeter/main.cpp, line 34 https://git.reviewboard.kde.org/r/117091/diff/5/?file=267770#file267770line34 I'm wondering about the variable naming. It's m_ so one would assume it's a member variable, but that looks more like a static variable. So better make it static and use the s_ prefix You're right. It is no member variable. No idea why I chose that name. It is a pointer to the actual UnlockApp object, so that the signalhandler can call its methods. - Wolfgang --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117091/#review56294 --- On April 23, 2014, 11:26 p.m., Wolfgang Bauer wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117091/ --- (Updated April 23, 2014, 11:26 p.m.) Review request for kde-workspace, Plasma and Aaron J. Seigo. Bugs: 327947 and 329076 http://bugs.kde.org/show_bug.cgi?id=327947 http://bugs.kde.org/show_bug.cgi?id=329076 Repository: kde-workspace Description --- If the screen locker is set to not require a password to unlock, it will not show the password input field even when the powermanagement settings suspend the system and are set to require a password after resume (when it was already running at that point). This locks people out of their system. This patch adds a signal handler for SIGUSR1 that switches the running greeter to immediateLock mode. The locker sends that signal to make sure the greeter shows the password input field when necessary. Diffs - ksmserver/screenlocker/greeter/greeterapp.h 8b79188 ksmserver/screenlocker/greeter/greeterapp.cpp c5e2f85 ksmserver/screenlocker/greeter/main.cpp d898734 ksmserver/screenlocker/ksldapp.cpp 3dfcc9e Diff: https://git.reviewboard.kde.org/r/117091/diff/ Testing --- Disable Require password after in the screen locker settings (the default), set it to start after 1 min. (for easier testing). Enable Suspend session after and set it to 2 minutes. (set the action to Suspend, Hibernate, or Lock Screen, doesn't matter) Make sure Lock screen on resume is enabled in the powermanagements Advanced Options (it is by default). After 1 minute the screen locker kicks in, and doesn't require a password. After 2 minutes the session gets suspended, hibernated or locked, and requires a password to resume. Without this patch no password dialog is shown, the user cannot resume the session by entering the password. With this patch this works: there is a password input field, the session is unlocked when the user enters the password. Thanks, Wolfgang Bauer
Re: Review Request 117091: Force the screen locker's greeter to show the password input field in case of immediateLock
On March 26, 2014, 11:07 p.m., Thomas Lübking wrote:
you could sighup or sigusr the greeter process and have it
setImmediateLock(true);
desktopResized();
in return
Wolfgang Bauer wrote:
I agree, this would be a bit nicer...
But I tried it and cannot get it to work.
My signal handler is called, and both setImmediateLock(true) and
desktopResized() are called, (I verified this with debug output statements)
but the password dialog still doesn't show.
Wolfgang Bauer wrote:
Oops, sorry. I just noticed that the signal handler is apparently only
called when I run kscreenlocker_greet manually (and do kill -USR1), but not
when the locker runs it.
Will have to investigate this.
Wolfgang Bauer wrote:
Forget my previous comment.
The signal handler was actually called all the time.
But setImmediateLock(true); followed by desktopResized(); DOES NOT
have any (visual) effect.
I have yet to find out what else to call to make that dialog appear. Any
idea maybe?
Calling exit(1) does work, but that's not much different to killing the
greeter I suppose... ;-)
Thomas Lübking wrote:
Ahhh... me was too smart in the multiscreen code ;-)
the for loop in desktopResized() (which is relevant for the
m_immediateLock handling) won't be entered since no screen changed.
Instead you'd run
for (int i = 0; i desktop()-screenCount(); ++i) {
QDeclarativeProperty(m_views.at(i)-rootObject(),
locked).write(true);
}
aside fixing the m_immediateLock value.
Sorry for the false information.
Wolfgang Bauer wrote:
Ok, that works.
Thank you for the help! :-)
Btw, I noticed that UnlockApp::setLockedPropertyOnViews() basically does
the same, so I'm just calling that instead to avoid code duplication.
I have uploaded a new patch.
Christoph Feck wrote:
What is the status of this patch? Do we have someone (besides yourself),
who has enough knowledge of the screen locker to approve it?
Well, this patch fixes the issue on all systems I tried (which is only 2).
It is also part of openSUSE's KDE 4.12/4.13 (actually
kdebase4-workspace-4.11.8) packages since over a week.
I think this is a grave bug, and really would like to see this fixed in 4.11.9.
Added plasma now to the group of reviewers, maybe somebody there is willing to
approve it...
- Wolfgang
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/117091/#review54247
---
On April 22, 2014, 6:41 p.m., Wolfgang Bauer wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/117091/
---
(Updated April 22, 2014, 6:41 p.m.)
Review request for kde-workspace, Plasma and Aaron J. Seigo.
Bugs: 327947 and 329076
http://bugs.kde.org/show_bug.cgi?id=327947
http://bugs.kde.org/show_bug.cgi?id=329076
Repository: kde-workspace
Description
---
If the screen locker is set to not require a password to unlock, it will not
show the password input field even when the powermanagement settings suspend
the system and are set to require a password after resume (when it was
already running at that point).
This locks people out of their system.
This patch adds a signal handler for SIGUSR1 that switches the running
greeter to immediateLock mode. The locker sends that signal to make sure the
greeter shows the password input field when necessary.
Diffs
-
ksmserver/screenlocker/greeter/greeterapp.h 8b79188
ksmserver/screenlocker/greeter/greeterapp.cpp c5e2f85
ksmserver/screenlocker/greeter/main.cpp d898734
ksmserver/screenlocker/ksldapp.cpp 3dfcc9e
Diff: https://git.reviewboard.kde.org/r/117091/diff/
Testing
---
Disable Require password after in the screen locker settings (the default),
set it to start after 1 min. (for easier testing).
Enable Suspend session after and set it to 2 minutes. (set the action to
Suspend, Hibernate, or Lock Screen, doesn't matter)
Make sure Lock screen on resume is enabled in the powermanagements
Advanced Options (it is by default).
After 1 minute the screen locker kicks in, and doesn't require a password.
After 2 minutes the session gets suspended, hibernated or locked, and
requires a password to resume.
Without this patch no password dialog is shown, the user cannot resume the
session by entering the password.
With this patch this works: there is a password input field, the session is
unlocked when the user enters the password.
Thanks,
Wolfgang Bauer
Re: Review Request 117091: Force the screen locker's greeter to show the password input field in case of immediateLock
On April 22, 2014, 7:34 p.m., Thomas Lübking wrote: ksmserver/screenlocker/greeter/main.cpp, line 89 https://git.reviewboard.kde.org/r/117091/diff/2/?file=262501#file262501line89 ooc.: what's wrong about just: signal(SIGUSR1, signalHandler); ? signal() is deprecated, according to its manpage. man signal even says: The behavior of signal() varies across UNIX versions, and has also var- ied historically across different versions of Linux. Avoid its use: use sigaction(2) instead. See Portability below. On April 22, 2014, 7:34 p.m., Thomas Lübking wrote: ksmserver/screenlocker/greeter/main.cpp, line 38 https://git.reviewboard.kde.org/r/117091/diff/2/?file=262501#file262501line38 it's nearly academical, but m_instance should be defaulted to NULL and tested here Yeah, it can't really happen that m_instance isn't set, but anyway. ;) Before somebody is asking, I'm going to use the signal handler for another bugfix as well. (bug#224200) That's why I'm doing the check this way. - Wolfgang --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117091/#review56190 --- On April 22, 2014, 9:28 p.m., Wolfgang Bauer wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117091/ --- (Updated April 22, 2014, 9:28 p.m.) Review request for kde-workspace, Plasma and Aaron J. Seigo. Bugs: 327947 and 329076 http://bugs.kde.org/show_bug.cgi?id=327947 http://bugs.kde.org/show_bug.cgi?id=329076 Repository: kde-workspace Description --- If the screen locker is set to not require a password to unlock, it will not show the password input field even when the powermanagement settings suspend the system and are set to require a password after resume (when it was already running at that point). This locks people out of their system. This patch adds a signal handler for SIGUSR1 that switches the running greeter to immediateLock mode. The locker sends that signal to make sure the greeter shows the password input field when necessary. Diffs - ksmserver/screenlocker/greeter/greeterapp.h 8b79188 ksmserver/screenlocker/greeter/greeterapp.cpp c5e2f85 ksmserver/screenlocker/greeter/main.cpp d898734 ksmserver/screenlocker/ksldapp.cpp 3dfcc9e Diff: https://git.reviewboard.kde.org/r/117091/diff/ Testing --- Disable Require password after in the screen locker settings (the default), set it to start after 1 min. (for easier testing). Enable Suspend session after and set it to 2 minutes. (set the action to Suspend, Hibernate, or Lock Screen, doesn't matter) Make sure Lock screen on resume is enabled in the powermanagements Advanced Options (it is by default). After 1 minute the screen locker kicks in, and doesn't require a password. After 2 minutes the session gets suspended, hibernated or locked, and requires a password to resume. Without this patch no password dialog is shown, the user cannot resume the session by entering the password. With this patch this works: there is a password input field, the session is unlocked when the user enters the password. Thanks, Wolfgang Bauer
Re: Review Request 117091: Force the screen locker's greeter to show the password input field in case of immediateLock
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117091/ --- (Updated April 22, 2014, 9:56 p.m.) Review request for kde-workspace, Plasma and Aaron J. Seigo. Changes --- Added a space and a newline. Bugs: 327947 and 329076 http://bugs.kde.org/show_bug.cgi?id=327947 http://bugs.kde.org/show_bug.cgi?id=329076 Repository: kde-workspace Description --- If the screen locker is set to not require a password to unlock, it will not show the password input field even when the powermanagement settings suspend the system and are set to require a password after resume (when it was already running at that point). This locks people out of their system. This patch adds a signal handler for SIGUSR1 that switches the running greeter to immediateLock mode. The locker sends that signal to make sure the greeter shows the password input field when necessary. Diffs (updated) - ksmserver/screenlocker/greeter/greeterapp.h 8b79188 ksmserver/screenlocker/greeter/greeterapp.cpp c5e2f85 ksmserver/screenlocker/greeter/main.cpp d898734 ksmserver/screenlocker/ksldapp.cpp 3dfcc9e Diff: https://git.reviewboard.kde.org/r/117091/diff/ Testing --- Disable Require password after in the screen locker settings (the default), set it to start after 1 min. (for easier testing). Enable Suspend session after and set it to 2 minutes. (set the action to Suspend, Hibernate, or Lock Screen, doesn't matter) Make sure Lock screen on resume is enabled in the powermanagements Advanced Options (it is by default). After 1 minute the screen locker kicks in, and doesn't require a password. After 2 minutes the session gets suspended, hibernated or locked, and requires a password to resume. Without this patch no password dialog is shown, the user cannot resume the session by entering the password. With this patch this works: there is a password input field, the session is unlocked when the user enters the password. Thanks, Wolfgang Bauer
Re: Review Request 117091: Force the screen locker's greeter to show the password input field in case of immediateLock
On April 22, 2014, 9:48 p.m., Thomas Lübking wrote: ksmserver/screenlocker/greeter/main.cpp, line 38 https://git.reviewboard.kde.org/r/117091/diff/3/?file=267733#file267733line38 /me would not insist on newline, but there needs to be a blank after the if if (!m_instance) return; OK, I added a newline as well, just to be sure... ;) - Wolfgang --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117091/#review56209 --- On April 22, 2014, 9:56 p.m., Wolfgang Bauer wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117091/ --- (Updated April 22, 2014, 9:56 p.m.) Review request for kde-workspace, Plasma and Aaron J. Seigo. Bugs: 327947 and 329076 http://bugs.kde.org/show_bug.cgi?id=327947 http://bugs.kde.org/show_bug.cgi?id=329076 Repository: kde-workspace Description --- If the screen locker is set to not require a password to unlock, it will not show the password input field even when the powermanagement settings suspend the system and are set to require a password after resume (when it was already running at that point). This locks people out of their system. This patch adds a signal handler for SIGUSR1 that switches the running greeter to immediateLock mode. The locker sends that signal to make sure the greeter shows the password input field when necessary. Diffs - ksmserver/screenlocker/greeter/greeterapp.h 8b79188 ksmserver/screenlocker/greeter/greeterapp.cpp c5e2f85 ksmserver/screenlocker/greeter/main.cpp d898734 ksmserver/screenlocker/ksldapp.cpp 3dfcc9e Diff: https://git.reviewboard.kde.org/r/117091/diff/ Testing --- Disable Require password after in the screen locker settings (the default), set it to start after 1 min. (for easier testing). Enable Suspend session after and set it to 2 minutes. (set the action to Suspend, Hibernate, or Lock Screen, doesn't matter) Make sure Lock screen on resume is enabled in the powermanagements Advanced Options (it is by default). After 1 minute the screen locker kicks in, and doesn't require a password. After 2 minutes the session gets suspended, hibernated or locked, and requires a password to resume. Without this patch no password dialog is shown, the user cannot resume the session by entering the password. With this patch this works: there is a password input field, the session is unlocked when the user enters the password. Thanks, Wolfgang Bauer
Re: Review Request 117091: Force the screen locker's greeter to show the password input field in case of immediateLock
On April 22, 2014, 7:34 p.m., Thomas Lübking wrote: ksmserver/screenlocker/greeter/main.cpp, line 89 https://git.reviewboard.kde.org/r/117091/diff/2/?file=262501#file262501line89 ooc.: what's wrong about just: signal(SIGUSR1, signalHandler); ? Wolfgang Bauer wrote: signal() is deprecated, according to its manpage. man signal even says: The behavior of signal() varies across UNIX versions, and has also var- ied historically across different versions of Linux. Avoid its use: use sigaction(2) instead. See Portability below. Thomas Lübking wrote: the posix manpage does of course not state that ... luckily it wraps around sigaction on BSD defaults on modern glibc systems - pfeew ;-) sigaction(SIGUSR1, sa, NULL); // for later nullptr i doubt that you'll need another update for that, though. Well, should I change it? In kscreensaver/libkscreensaver/main.cpp they used sigaction() as well. - Wolfgang --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117091/#review56190 --- On April 22, 2014, 9:56 p.m., Wolfgang Bauer wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117091/ --- (Updated April 22, 2014, 9:56 p.m.) Review request for kde-workspace, Plasma and Aaron J. Seigo. Bugs: 327947 and 329076 http://bugs.kde.org/show_bug.cgi?id=327947 http://bugs.kde.org/show_bug.cgi?id=329076 Repository: kde-workspace Description --- If the screen locker is set to not require a password to unlock, it will not show the password input field even when the powermanagement settings suspend the system and are set to require a password after resume (when it was already running at that point). This locks people out of their system. This patch adds a signal handler for SIGUSR1 that switches the running greeter to immediateLock mode. The locker sends that signal to make sure the greeter shows the password input field when necessary. Diffs - ksmserver/screenlocker/greeter/greeterapp.h 8b79188 ksmserver/screenlocker/greeter/greeterapp.cpp c5e2f85 ksmserver/screenlocker/greeter/main.cpp d898734 ksmserver/screenlocker/ksldapp.cpp 3dfcc9e Diff: https://git.reviewboard.kde.org/r/117091/diff/ Testing --- Disable Require password after in the screen locker settings (the default), set it to start after 1 min. (for easier testing). Enable Suspend session after and set it to 2 minutes. (set the action to Suspend, Hibernate, or Lock Screen, doesn't matter) Make sure Lock screen on resume is enabled in the powermanagements Advanced Options (it is by default). After 1 minute the screen locker kicks in, and doesn't require a password. After 2 minutes the session gets suspended, hibernated or locked, and requires a password to resume. Without this patch no password dialog is shown, the user cannot resume the session by entering the password. With this patch this works: there is a password input field, the session is unlocked when the user enters the password. Thanks, Wolfgang Bauer
Review Request 117644: screenlocker: don't leave behind screensaver processes
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117644/ --- Review request for kde-workspace and Plasma. Bugs: 224200 http://bugs.kde.org/show_bug.cgi?id=224200 Repository: kde-workspace Description --- Currently the screen locker just kills the greeter (kscreenlocker_greet) when the screen is unlocked by the user during the grace time. But apparently this can leave behind running screensaver processes launched by the greeter, see the bug report (which has the highest number of votes of all open bugs AFAICS). This patch changes this to only terminate the greeter, and adds a signal handler to the greeter to exit gracefully in this case. The signal handler exits with return code 1, so that it is not possible to circumvent the password input by just sending a SIGTERM. (the screen locker restarts the greeter in case it doesn't quit with exit code 0) Diffs - ksmserver/screenlocker/ksldapp.cpp 3dfcc9e ksmserver/screenlocker/greeter/main.cpp d898734 Diff: https://git.reviewboard.kde.org/r/117644/diff/ Testing --- Configure a legacy screensaver in Systemsettings-Display and Monitor-Screen Locker, be sure to leave Require Password after disabled. Wait for the screen locker to kick in. Unlock the screen by moving the mouse or pressing a key. Check the process list. Without this patch at least kswarm.kss and kblankscreen.kss reliably kept running after unlocking the screen on my system. With this patch they quit themselves. I'm using this patch for over two weeks now, and I haven't seen any left-over screen saver processes any more (and I even set the timeout to 1 minute). I also tried to terminate kscreenlocker_greet manually by running killall kscreenlocker_greet from a text console in case of a password required, and the locker didn't quit, you still have to enter the password. Thanks, Wolfgang Bauer
Re: Review Request 117644: screenlocker: don't leave behind screensaver processes
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117644/ --- (Updated April 22, 2014, 10:41 p.m.) Review request for kde-workspace and Plasma. Changes --- Adapted the patch to a change in https://git.reviewboard.kde.org/r/117091/ Bugs: 224200 http://bugs.kde.org/show_bug.cgi?id=224200 Repository: kde-workspace Description --- Currently the screen locker just kills the greeter (kscreenlocker_greet) when the screen is unlocked by the user during the grace time. But apparently this can leave behind running screensaver processes launched by the greeter, see the bug report (which has the highest number of votes of all open bugs AFAICS). This patch changes this to only terminate the greeter, and adds a signal handler to the greeter to exit gracefully in this case. The signal handler exits with return code 1, so that it is not possible to circumvent the password input by just sending a SIGTERM. (the screen locker restarts the greeter in case it doesn't quit with exit code 0) Diffs (updated) - ksmserver/screenlocker/greeter/main.cpp d898734 ksmserver/screenlocker/ksldapp.cpp 3dfcc9e Diff: https://git.reviewboard.kde.org/r/117644/diff/ Testing --- Configure a legacy screensaver in Systemsettings-Display and Monitor-Screen Locker, be sure to leave Require Password after disabled. Wait for the screen locker to kick in. Unlock the screen by moving the mouse or pressing a key. Check the process list. Without this patch at least kswarm.kss and kblankscreen.kss reliably kept running after unlocking the screen on my system. With this patch they quit themselves. I'm using this patch for over two weeks now, and I haven't seen any left-over screen saver processes any more (and I even set the timeout to 1 minute). I also tried to terminate kscreenlocker_greet manually by running killall kscreenlocker_greet from a text console in case of a password required, and the locker didn't quit, you still have to enter the password. Thanks, Wolfgang Bauer
Re: Review Request 117091: Force the screen locker's greeter to show the password input field in case of immediateLock
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117091/ --- (Updated April 22, 2014, 10:34 p.m.) Review request for kde-workspace, Plasma and Aaron J. Seigo. Changes --- Changed 0 to NULL in the sigaction() call. Bugs: 327947 and 329076 http://bugs.kde.org/show_bug.cgi?id=327947 http://bugs.kde.org/show_bug.cgi?id=329076 Repository: kde-workspace Description --- If the screen locker is set to not require a password to unlock, it will not show the password input field even when the powermanagement settings suspend the system and are set to require a password after resume (when it was already running at that point). This locks people out of their system. This patch adds a signal handler for SIGUSR1 that switches the running greeter to immediateLock mode. The locker sends that signal to make sure the greeter shows the password input field when necessary. Diffs (updated) - ksmserver/screenlocker/greeter/greeterapp.cpp c5e2f85 ksmserver/screenlocker/greeter/main.cpp d898734 ksmserver/screenlocker/greeter/greeterapp.h 8b79188 ksmserver/screenlocker/ksldapp.cpp 3dfcc9e Diff: https://git.reviewboard.kde.org/r/117091/diff/ Testing --- Disable Require password after in the screen locker settings (the default), set it to start after 1 min. (for easier testing). Enable Suspend session after and set it to 2 minutes. (set the action to Suspend, Hibernate, or Lock Screen, doesn't matter) Make sure Lock screen on resume is enabled in the powermanagements Advanced Options (it is by default). After 1 minute the screen locker kicks in, and doesn't require a password. After 2 minutes the session gets suspended, hibernated or locked, and requires a password to resume. Without this patch no password dialog is shown, the user cannot resume the session by entering the password. With this patch this works: there is a password input field, the session is unlocked when the user enters the password. Thanks, Wolfgang Bauer
Re: Review Request 117091: Force the screen locker's greeter to show the password input field in case of immediateLock
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117091/ --- (Updated April 3, 2014, 5:15 p.m.) Review request for kde-workspace and Aaron J. Seigo. Changes --- Added a signal handler for SIGUSR1 that switches the greeter to immediateLock mode. The locker now sends that signal instead of killing the greeter. Bugs: 327947 and 329076 http://bugs.kde.org/show_bug.cgi?id=327947 http://bugs.kde.org/show_bug.cgi?id=329076 Repository: kde-workspace Description (updated) --- If the screen locker is set to not require a password to unlock, it will not show the password input field even when the powermanagement settings suspend the system and are set to require a password after resume (when it was already running at that point). This locks people out of their system. This patch adds a signal handler for SIGUSR1 that switches the running greeter to immediateLock mode. The locker sends that signal to make sure the greeter shows the password input field when necessary. Diffs (updated) - ksmserver/screenlocker/greeter/greeterapp.h 8b79188 ksmserver/screenlocker/greeter/greeterapp.cpp c5e2f85 ksmserver/screenlocker/greeter/main.cpp d898734 ksmserver/screenlocker/ksldapp.cpp 3dfcc9e Diff: https://git.reviewboard.kde.org/r/117091/diff/ Testing (updated) --- Disable Require password after in the screen locker settings (the default), set it to start after 1 min. (for easier testing). Enable Suspend session after and set it to 2 minutes. (set the action to Suspend, Hibernate, or Lock Screen, doesn't matter) Make sure Lock screen on resume is enabled in the powermanagements Advanced Options (it is by default). After 1 minute the screen locker kicks in, and doesn't require a password. After 2 minutes the session gets suspended, hibernated or locked, and requires a password to resume. Without this patch no password dialog is shown, the user cannot resume the session by entering the password. With this patch this works: there is a password input field, the session is unlocked when the user enters the password. Thanks, Wolfgang Bauer
Re: Review Request 117091: Force the screen locker's greeter to show the password input field in case of immediateLock
On March 26, 2014, 11:07 p.m., Thomas Lübking wrote:
you could sighup or sigusr the greeter process and have it
setImmediateLock(true);
desktopResized();
in return
Wolfgang Bauer wrote:
I agree, this would be a bit nicer...
But I tried it and cannot get it to work.
My signal handler is called, and both setImmediateLock(true) and
desktopResized() are called, (I verified this with debug output statements)
but the password dialog still doesn't show.
Wolfgang Bauer wrote:
Oops, sorry. I just noticed that the signal handler is apparently only
called when I run kscreenlocker_greet manually (and do kill -USR1), but not
when the locker runs it.
Will have to investigate this.
Wolfgang Bauer wrote:
Forget my previous comment.
The signal handler was actually called all the time.
But setImmediateLock(true); followed by desktopResized(); DOES NOT
have any (visual) effect.
I have yet to find out what else to call to make that dialog appear. Any
idea maybe?
Calling exit(1) does work, but that's not much different to killing the
greeter I suppose... ;-)
Thomas Lübking wrote:
Ahhh... me was too smart in the multiscreen code ;-)
the for loop in desktopResized() (which is relevant for the
m_immediateLock handling) won't be entered since no screen changed.
Instead you'd run
for (int i = 0; i desktop()-screenCount(); ++i) {
QDeclarativeProperty(m_views.at(i)-rootObject(),
locked).write(true);
}
aside fixing the m_immediateLock value.
Sorry for the false information.
Ok, that works.
Thank you for the help! :-)
Btw, I noticed that UnlockApp::setLockedPropertyOnViews() basically does the
same, so I'm just calling that instead to avoid code duplication.
I have uploaded a new patch.
- Wolfgang
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/117091/#review54247
---
On April 3, 2014, 5:15 p.m., Wolfgang Bauer wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/117091/
---
(Updated April 3, 2014, 5:15 p.m.)
Review request for kde-workspace and Aaron J. Seigo.
Bugs: 327947 and 329076
http://bugs.kde.org/show_bug.cgi?id=327947
http://bugs.kde.org/show_bug.cgi?id=329076
Repository: kde-workspace
Description
---
If the screen locker is set to not require a password to unlock, it will not
show the password input field even when the powermanagement settings suspend
the system and are set to require a password after resume (when it was
already running at that point).
This locks people out of their system.
This patch adds a signal handler for SIGUSR1 that switches the running
greeter to immediateLock mode. The locker sends that signal to make sure the
greeter shows the password input field when necessary.
Diffs
-
ksmserver/screenlocker/greeter/greeterapp.h 8b79188
ksmserver/screenlocker/greeter/greeterapp.cpp c5e2f85
ksmserver/screenlocker/greeter/main.cpp d898734
ksmserver/screenlocker/ksldapp.cpp 3dfcc9e
Diff: https://git.reviewboard.kde.org/r/117091/diff/
Testing
---
Disable Require password after in the screen locker settings (the default),
set it to start after 1 min. (for easier testing).
Enable Suspend session after and set it to 2 minutes. (set the action to
Suspend, Hibernate, or Lock Screen, doesn't matter)
Make sure Lock screen on resume is enabled in the powermanagements
Advanced Options (it is by default).
After 1 minute the screen locker kicks in, and doesn't require a password.
After 2 minutes the session gets suspended, hibernated or locked, and
requires a password to resume.
Without this patch no password dialog is shown, the user cannot resume the
session by entering the password.
With this patch this works: there is a password input field, the session is
unlocked when the user enters the password.
Thanks,
Wolfgang Bauer
Re: Review Request 117091: Force the screen locker's greeter to show the password input field in case of immediateLock
On March 26, 2014, 11:07 p.m., Thomas Lübking wrote: you could sighup or sigusr the greeter process and have it setImmediateLock(true); desktopResized(); in return Wolfgang Bauer wrote: I agree, this would be a bit nicer... But I tried it and cannot get it to work. My signal handler is called, and both setImmediateLock(true) and desktopResized() are called, (I verified this with debug output statements) but the password dialog still doesn't show. Wolfgang Bauer wrote: Oops, sorry. I just noticed that the signal handler is apparently only called when I run kscreenlocker_greet manually (and do kill -USR1), but not when the locker runs it. Will have to investigate this. Forget my previous comment. The signal handler was actually called all the time. But setImmediateLock(true); followed by desktopResized(); DOES NOT have any (visual) effect. I have yet to find out what else to call to make that dialog appear. Any idea maybe? Calling exit(1) does work, but that's not much different to killing the greeter I suppose... ;-) - Wolfgang --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117091/#review54247 --- On March 26, 2014, 5:58 p.m., Wolfgang Bauer wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117091/ --- (Updated March 26, 2014, 5:58 p.m.) Review request for kde-workspace and Aaron J. Seigo. Bugs: 327947 and 329076 http://bugs.kde.org/show_bug.cgi?id=327947 http://bugs.kde.org/show_bug.cgi?id=329076 Repository: kde-workspace Description --- If the screen locker is set to not require a password to unlock, it will not show the password input field even when the powermanagement settings suspend the system and are set to require a password after resume (when it was already running at that point). This locks people out of their system. As there seems to be no way to switch the already running greeter to immediateLock mode, it is killed in that case by this patch. It gets restarted immediately with the --immediatelock option in KSldApp::lockProcessFinished() and shows the password input field then. Diffs - ksmserver/screenlocker/ksldapp.cpp 3dfcc9e Diff: https://git.reviewboard.kde.org/r/117091/diff/ Testing --- Disable Require password after in the screen locker settings (the default), set it to start after 1 min. (for easier testing). Enable Suspend session after and set it to 2 minutes. (set the action to Suspend, Hibernate, or Lock Screen, doesn't matter) Make sure Lock screen on resume is enabled in the powermanagements Advanced Options (it is by default). After 1 minute the screen locker kicks in, and doesn't require a password. After 2 minutes the session gets suspended, hibernated or locked, and requires a password to resume. Without this patch no password dialog is shown, the user cannot resume the session by entering the password. With this patch this works: there is a password input field, the session is unlocked when the user enters the password. Other openSUSE users have tested this as well, see f.e. https://bugzilla.novell.com/show_bug.cgi?id=864305#c4 Thanks, Wolfgang Bauer
Re: Review Request 117091: Force the screen locker's greeter to show the password input field in case of immediateLock
On March 26, 2014, 11:07 p.m., Thomas Lübking wrote: you could sighup or sigusr the greeter process and have it setImmediateLock(true); desktopResized(); in return I agree, this would be a bit nicer... But I tried it and cannot get it to work. My signal handler is called, and both setImmediateLock(true) and desktopResized() are called, (I verified this with debug output statements) but the password dialog still doesn't show. - Wolfgang --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117091/#review54247 --- On March 26, 2014, 5:58 p.m., Wolfgang Bauer wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117091/ --- (Updated March 26, 2014, 5:58 p.m.) Review request for kde-workspace and Aaron J. Seigo. Bugs: 327947 and 329076 http://bugs.kde.org/show_bug.cgi?id=327947 http://bugs.kde.org/show_bug.cgi?id=329076 Repository: kde-workspace Description --- If the screen locker is set to not require a password to unlock, it will not show the password input field even when the powermanagement settings suspend the system and are set to require a password after resume (when it was already running at that point). This locks people out of their system. As there seems to be no way to switch the already running greeter to immediateLock mode, it is killed in that case by this patch. It gets restarted immediately with the --immediatelock option in KSldApp::lockProcessFinished() and shows the password input field then. Diffs - ksmserver/screenlocker/ksldapp.cpp 3dfcc9e Diff: https://git.reviewboard.kde.org/r/117091/diff/ Testing --- Disable Require password after in the screen locker settings (the default), set it to start after 1 min. (for easier testing). Enable Suspend session after and set it to 2 minutes. (set the action to Suspend, Hibernate, or Lock Screen, doesn't matter) Make sure Lock screen on resume is enabled in the powermanagements Advanced Options (it is by default). After 1 minute the screen locker kicks in, and doesn't require a password. After 2 minutes the session gets suspended, hibernated or locked, and requires a password to resume. Without this patch no password dialog is shown, the user cannot resume the session by entering the password. With this patch this works: there is a password input field, the session is unlocked when the user enters the password. Other openSUSE users have tested this as well, see f.e. https://bugzilla.novell.com/show_bug.cgi?id=864305#c4 Thanks, Wolfgang Bauer
Re: Review Request 117091: Force the screen locker's greeter to show the password input field in case of immediateLock
On March 26, 2014, 11:07 p.m., Thomas Lübking wrote: you could sighup or sigusr the greeter process and have it setImmediateLock(true); desktopResized(); in return Wolfgang Bauer wrote: I agree, this would be a bit nicer... But I tried it and cannot get it to work. My signal handler is called, and both setImmediateLock(true) and desktopResized() are called, (I verified this with debug output statements) but the password dialog still doesn't show. Oops, sorry. I just noticed that the signal handler is apparently only called when I run kscreenlocker_greet manually (and do kill -USR1), but not when the locker runs it. Will have to investigate this. - Wolfgang --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117091/#review54247 --- On March 26, 2014, 5:58 p.m., Wolfgang Bauer wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117091/ --- (Updated March 26, 2014, 5:58 p.m.) Review request for kde-workspace and Aaron J. Seigo. Bugs: 327947 and 329076 http://bugs.kde.org/show_bug.cgi?id=327947 http://bugs.kde.org/show_bug.cgi?id=329076 Repository: kde-workspace Description --- If the screen locker is set to not require a password to unlock, it will not show the password input field even when the powermanagement settings suspend the system and are set to require a password after resume (when it was already running at that point). This locks people out of their system. As there seems to be no way to switch the already running greeter to immediateLock mode, it is killed in that case by this patch. It gets restarted immediately with the --immediatelock option in KSldApp::lockProcessFinished() and shows the password input field then. Diffs - ksmserver/screenlocker/ksldapp.cpp 3dfcc9e Diff: https://git.reviewboard.kde.org/r/117091/diff/ Testing --- Disable Require password after in the screen locker settings (the default), set it to start after 1 min. (for easier testing). Enable Suspend session after and set it to 2 minutes. (set the action to Suspend, Hibernate, or Lock Screen, doesn't matter) Make sure Lock screen on resume is enabled in the powermanagements Advanced Options (it is by default). After 1 minute the screen locker kicks in, and doesn't require a password. After 2 minutes the session gets suspended, hibernated or locked, and requires a password to resume. Without this patch no password dialog is shown, the user cannot resume the session by entering the password. With this patch this works: there is a password input field, the session is unlocked when the user enters the password. Other openSUSE users have tested this as well, see f.e. https://bugzilla.novell.com/show_bug.cgi?id=864305#c4 Thanks, Wolfgang Bauer
Review Request 117091: Force the screen locker's greeter to show the password input field in case of immediateLock
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117091/ --- Review request for kde-workspace and Aaron J. Seigo. Bugs: 327947 and 329076 http://bugs.kde.org/show_bug.cgi?id=327947 http://bugs.kde.org/show_bug.cgi?id=329076 Repository: kde-workspace Description --- If the screen locker is set to not require a password to unlock, it will not show the password input field even when the powermanagement settings suspend the system and are set to require a password after resume (when it was already running at that point). This locks people out of their system. As there seems to be no way to switch the already running greeter to immediateLock mode, it is killed in that case by this patch. It gets restarted immediately with the --immediatelock option in KSldApp::lockProcessFinished() and shows the password input field then. Diffs - ksmserver/screenlocker/ksldapp.cpp 3dfcc9e Diff: https://git.reviewboard.kde.org/r/117091/diff/ Testing --- Disable Require password after in the screen locker settings (the default), set it to start after 1 min. (for easier testing). Enable Suspend session after and set it to 2 minutes. (set the action to Suspend, Hibernate, or Lock Screen, doesn't matter) Make sure Lock screen on resume is enabled in the powermanagements Advanced Options (it is by default). After 1 minute the screen locker kicks in, and doesn't require a password. After 2 minutes the session gets suspended, hibernated or locked, and requires a password to resume. Without this patch no password dialog is shown, the user cannot resume the session by entering the password. With this patch this works: there is a password input field, the session is unlocked when the user enters the password. Other openSUSE users have tested this as well, see f.e. https://bugzilla.novell.com/show_bug.cgi?id=864305#c4 Thanks, Wolfgang Bauer
Re: Review Request 114737: KInfocenter/OpenGL: reimplement the ReadPipe() function with QProcess
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/114737/#review48287 --- I made another change because I noticed a QProcess: Destroyed while process is still running. message in the terminal. I now call QProcess::waitForFinished() instead of QProcess::waitForReadyRead(). This works as well, and gets rid of that warning. - Wolfgang Bauer On Jan. 4, 2014, 5:54 p.m., Wolfgang Bauer wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/114737/ --- (Updated Jan. 4, 2014, 5:54 p.m.) Review request for kde-workspace and David Stephen Hubner. Repository: kde-workspace Description --- This patch reimplements the ReadPipe() function by using QProcess instead of popen(). This should make it more portable. As a positive side-effect, this also removes those sh: lspci: command not found. messages when run in Konsole and lspci is not in the user's path. This was suggested on the kde-core-devel mailinglist in November: http://lists.kde.org/?l=kde-core-develm=138407113011843w=2 http://lists.kde.org/?l=kde-core-develm=138409755820003w=2 Diffs - kinfocenter/Modules/opengl/opengl.cpp 8901957 Diff: https://git.reviewboard.kde.org/r/114737/diff/ Testing --- Ran KInfocenter with lspci in /usr/bin/ (i.e. in the user's path) and /sbin/ (not in the user's path). The OpenGL module showed the 3D accelerator info correctly in both cases. With lspci removed completely it showed unknown as expected. Thanks, Wolfgang Bauer
Re: Review Request 114737: KInfocenter/OpenGL: reimplement the ReadPipe() function with QProcess
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/114737/ --- (Updated Jan. 26, 2014, 10:43 a.m.) Review request for kde-workspace and David Stephen Hubner. Changes --- - changed the type of the FileName parameter to const QString - call QProcess::waitForFinished() instead of QProcess::waitForReadyRead() to get rid of a runtime warning (QProcess: Destroyed while process is still running.) Repository: kde-workspace Description --- This patch reimplements the ReadPipe() function by using QProcess instead of popen(). This should make it more portable. As a positive side-effect, this also removes those sh: lspci: command not found. messages when run in Konsole and lspci is not in the user's path. This was suggested on the kde-core-devel mailinglist in November: http://lists.kde.org/?l=kde-core-develm=138407113011843w=2 http://lists.kde.org/?l=kde-core-develm=138409755820003w=2 Diffs (updated) - kinfocenter/Modules/opengl/opengl.cpp 8901957 Diff: https://git.reviewboard.kde.org/r/114737/diff/ Testing --- Ran KInfocenter with lspci in /usr/bin/ (i.e. in the user's path) and /sbin/ (not in the user's path). The OpenGL module showed the 3D accelerator info correctly in both cases. With lspci removed completely it showed unknown as expected. Thanks, Wolfgang Bauer
Re: Review Request 114841: Screenlocker: don't set the mouse cursor when grabbing the mouse
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/114841/ --- (Updated Jan. 8, 2014, 8:59 a.m.) Status -- This change has been marked as submitted. Review request for kde-workspace and Martin Gräßlin. Bugs: 311571 and 316459 http://bugs.kde.org/show_bug.cgi?id=311571 http://bugs.kde.org/show_bug.cgi?id=316459 Repository: kde-workspace Description --- Setting the cursor to ArrowCursor when calling XGrabPointer() prevents the Screen savers from blanking the mouse cursor. I don't know why this has been done in the first place, but I couldn't see any negative effect by setting it to None. Now the mouse cursor even changes to the IBeam again when over the password field, which I find more intuitive. Diffs - ksmserver/screenlocker/ksldapp.cpp f0526cf Diff: https://git.reviewboard.kde.org/r/114841/diff/ Testing --- Configure a Screen saver in systemsettings and wait for it to kick in (or lock the screen manually). Previously (since 4.10) the mouse cursor stayed visible, now it is blanked like it was the case before 4.10. Moving the mouse/pressing a key (to quit the Screen saver) makes the mouse cursor appear again as it should, regardless of whether the screen is locked or not. Thanks, Wolfgang Bauer
Re: Review Request 114841: Screenlocker: don't set the mouse cursor when grabbing the mouse
On Jan. 8, 2014, 8:10 a.m., Martin Gräßlin wrote: If you have the possibility (build setup) please merge to master and fix the merge conflict I expect to see :-) I merged 4.11 into master yesterday so there should no be anything else which could conflict. Wolfgang Bauer wrote: I have committed to 4.11, but I don't have a KF5/PW2 build setup at the moment. Martin Gräßlin wrote: I just merged it to master and pushed as http://commits.kde.org/kde-workspace/98768f680480df64a60fbe1802ca8ee05fb28887 OK, thank you! - Wolfgang --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/114841/#review47020 --- On Jan. 8, 2014, 9:59 a.m., Wolfgang Bauer wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/114841/ --- (Updated Jan. 8, 2014, 9:59 a.m.) Review request for kde-workspace and Martin Gräßlin. Bugs: 311571 and 316459 http://bugs.kde.org/show_bug.cgi?id=311571 http://bugs.kde.org/show_bug.cgi?id=316459 Repository: kde-workspace Description --- Setting the cursor to ArrowCursor when calling XGrabPointer() prevents the Screen savers from blanking the mouse cursor. I don't know why this has been done in the first place, but I couldn't see any negative effect by setting it to None. Now the mouse cursor even changes to the IBeam again when over the password field, which I find more intuitive. Diffs - ksmserver/screenlocker/ksldapp.cpp f0526cf Diff: https://git.reviewboard.kde.org/r/114841/diff/ Testing --- Configure a Screen saver in systemsettings and wait for it to kick in (or lock the screen manually). Previously (since 4.10) the mouse cursor stayed visible, now it is blanked like it was the case before 4.10. Moving the mouse/pressing a key (to quit the Screen saver) makes the mouse cursor appear again as it should, regardless of whether the screen is locked or not. Thanks, Wolfgang Bauer
Re: Review Request 114841: Screenlocker: don't set the mouse cursor when grabbing the mouse
On Jan. 7, 2014, 9:51 a.m., Martin Gräßlin wrote: how does that behave with the normal locker (that is no screensaver)? Thomas Lübking wrote: None is not blank - the patch is correct. If a cursor is specified, it is displayed regardless of what window the pointer is in. If None is specified, the normal cursor for that window is displayed when the pointer is in grab_window or one of its subwindows; otherwise, the cursor for grab_window is displayed. --- http://tronche.com/gui/x/xlib/input/XGrabPointer.html Since the screenlocker qml window (nor the batterysucking fancy show) is not the grab_window, both will initially get the default cursor of the grab_window (atm. the left_arrow) and not receive mouse events (so w/ or w/o the patch, the qml locker will not display an I-beam when hovering the lineedit - at least not here, one would have to poll the mouse for this and don't you dare* ;-) W/o an explicit grab_cursor, it however should be possible (never tested, but seems the case given the patch) to alter the cursor shape from other clients at any time (because it's not important for the grabbing client) * unless we make use of http://keithp.com/blogs/Cursor_tracking/ for PW/2 With the normal locker, I see no change in behaviour as well. The mouse cursor doesn't change, even when hovering over the password input field. If a screen saver is configured, the mouse cursor will change to the IBeam (i.e. text input) shape over the password field. But just to clarify: this is intended for the 4.11 branch only. Xscreensaver support is dropped for PW/2 anyway, right? - Wolfgang --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/114841/#review46960 --- On Jan. 5, 2014, 9:55 a.m., Wolfgang Bauer wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/114841/ --- (Updated Jan. 5, 2014, 9:55 a.m.) Review request for kde-workspace and Martin Gräßlin. Bugs: 311571 and 316459 http://bugs.kde.org/show_bug.cgi?id=311571 http://bugs.kde.org/show_bug.cgi?id=316459 Repository: kde-workspace Description --- Setting the cursor to ArrowCursor when calling XGrabPointer() prevents the Screen savers from blanking the mouse cursor. I don't know why this has been done in the first place, but I couldn't see any negative effect by setting it to None. Now the mouse cursor even changes to the IBeam again when over the password field, which I find more intuitive. Diffs - ksmserver/screenlocker/ksldapp.cpp f0526cf Diff: https://git.reviewboard.kde.org/r/114841/diff/ Testing --- Configure a Screen saver in systemsettings and wait for it to kick in (or lock the screen manually). Previously (since 4.10) the mouse cursor stayed visible, now it is blanked like it was the case before 4.10. Moving the mouse/pressing a key (to quit the Screen saver) makes the mouse cursor appear again as it should, regardless of whether the screen is locked or not. Thanks, Wolfgang Bauer
Review Request 114841: Screenlocker: don't set the mouse cursor when grabbing the mouse
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/114841/ --- Review request for kde-workspace and Martin Gräßlin. Bugs: 311571 and 316459 http://bugs.kde.org/show_bug.cgi?id=311571 http://bugs.kde.org/show_bug.cgi?id=316459 Repository: kde-workspace Description --- Setting the cursor to ArrowCursor when calling XGrabPointer() prevents the Screen savers from blanking the mouse cursor. I don't know why this has been done in the first place, but I couldn't see any negative effect by setting it to None. Now the mouse cursor even changes to the IBeam again when over the password field, which I find more intuitive. Diffs - ksmserver/screenlocker/ksldapp.cpp f0526cf Diff: https://git.reviewboard.kde.org/r/114841/diff/ Testing --- Configure a Screen saver in systemsettings and wait for it to kick in (or lock the screen manually). Previously (since 4.10) the mouse cursor stayed visible, now it is blanked like it was the case before 4.10. Moving the mouse/pressing a key (to quit the Screen saver) makes the mouse cursor appear again as it should, regardless of whether the screen is locked or not. Thanks, Wolfgang Bauer
Review Request 114737: KInfocenter/OpenGL: reimplement the ReadPipe() function with QProcess
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/114737/ --- Review request for kde-workspace and David Stephen Hubner. Repository: kde-workspace Description --- This patch reimplements the ReadPipe() function by using QProcess instead of popen(). This should make it more portable. As a positive side-effect, this also removes those sh: lspci: command not found. messages when run in Konsole and lspci is not in the user's path. This was suggested on the kde-core-devel mailinglist in November: http://lists.kde.org/?l=kde-core-develm=138407113011843w=2 http://lists.kde.org/?l=kde-core-develm=138409755820003w=2 Diffs - kinfocenter/Modules/opengl/opengl.cpp 8901957 Diff: https://git.reviewboard.kde.org/r/114737/diff/ Testing --- Ran KInfocenter with lspci in /usr/bin/ (i.e. in the user's path) and /sbin/ (not in the user's path). The OpenGL module showed the 3D accelerator info correctly in both cases. With lspci removed completely it showed unknown as expected. Thanks, Wolfgang Bauer
Re: Review Request 114737: KInfocenter/OpenGL: reimplement the ReadPipe() function with QProcess
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/114737/ --- (Updated Jan. 4, 2014, 5:54 p.m.) Review request for kde-workspace and David Stephen Hubner. Changes --- Addressed the mentioned issue. Repository: kde-workspace Description --- This patch reimplements the ReadPipe() function by using QProcess instead of popen(). This should make it more portable. As a positive side-effect, this also removes those sh: lspci: command not found. messages when run in Konsole and lspci is not in the user's path. This was suggested on the kde-core-devel mailinglist in November: http://lists.kde.org/?l=kde-core-develm=138407113011843w=2 http://lists.kde.org/?l=kde-core-develm=138409755820003w=2 Diffs (updated) - kinfocenter/Modules/opengl/opengl.cpp 8901957 Diff: https://git.reviewboard.kde.org/r/114737/diff/ Testing --- Ran KInfocenter with lspci in /usr/bin/ (i.e. in the user's path) and /sbin/ (not in the user's path). The OpenGL module showed the 3D accelerator info correctly in both cases. With lspci removed completely it showed unknown as expected. Thanks, Wolfgang Bauer
Re: Review Request 109609: disable session management for screensavers
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/109609/ --- (Updated Jan. 2, 2014, 6:57 p.m.) Status -- This change has been marked as submitted. Review request for kde-workspace and Oliver Henshaw. Bugs: 314859 http://bugs.kde.org/show_bug.cgi?id=314859 Repository: kde-workspace Description --- This patch disables session management for screensavers. Therefore no screensaver windows popup anymore on login if processes keep on running when the user logs out. As proposed in https://bugs.kde.org/show_bug.cgi?id=314859#c18 This doesn't solve the underlying problem of bug#224200 of course, but provides better user experience for now, so please accept. And IMHO it doesn't make sense to restore screensavers on login, anyway. Diffs - kscreensaver/libkscreensaver/main.cpp 6851ba1 Diff: https://git.reviewboard.kde.org/r/109609/diff/ Testing --- - Turn on the Blank Screen (kblankscrn.kss) screensaver in systemsettings. - Wait for the screensaver to kick in - Verify that the process is still running (due to bug#224200) - log out - log in again Now no kblankscrn.kss window is opened anymore (without the patch, a window should appear) Thanks, Wolfgang Bauer
Re: Review Request 109609: disable session management for screensavers
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/109609/#review46471 --- Ping? Any opinions/comments? Btw, this patch is part of openSUSE's official KDE packages since September. - Wolfgang Bauer On Sept. 11, 2013, 1:15 a.m., Wolfgang Bauer wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/109609/ --- (Updated Sept. 11, 2013, 1:15 a.m.) Review request for kde-workspace and Oliver Henshaw. Bugs: 314859 http://bugs.kde.org/show_bug.cgi?id=314859 Repository: kde-workspace Description --- This patch disables session management for screensavers. Therefore no screensaver windows popup anymore on login if processes keep on running when the user logs out. As proposed in https://bugs.kde.org/show_bug.cgi?id=314859#c18 This doesn't solve the underlying problem of bug#224200 of course, but provides better user experience for now, so please accept. And IMHO it doesn't make sense to restore screensavers on login, anyway. Diffs - kscreensaver/libkscreensaver/main.cpp 6851ba1 Diff: https://git.reviewboard.kde.org/r/109609/diff/ Testing --- - Turn on the Blank Screen (kblankscrn.kss) screensaver in systemsettings. - Wait for the screensaver to kick in - Verify that the process is still running (due to bug#224200) - log out - log in again Now no kblankscrn.kss window is opened anymore (without the patch, a window should appear) Thanks, Wolfgang Bauer
Re: kde-workspace git broken ?
Am Mittwoch, 13. November 2013, 18:42:33 schrieb Ben Cooksley: On Tue, Nov 12, 2013 at 2:21 AM, Wolfgang Bauer wba...@tmo.at wrote: Sorry, it was my fault. And there's also commit 925af7a94eacbe7fef101c6e0f6415b1898d3bfe now. Hi Wolfgang, The sha you have just referenced is a blob, not a commit - at least in kde-workspace. As far as I am aware this issue is now resolved. That was in the KDE/4.11 branch. But it was removed along with the other commits. Can those 4 commits somehow be reverted? To my knowledge, the defective entries have been purged from the history of the repository in their entirety. Yes, all of them are gone as far as I can see... Kindest Regards, Wolfgang
Re: Review Request 113779: KInfocenter/OpenGL: fix ReadPipe() in the case that the command cannot be run
Am Sonntag, 10. November 2013, 10:32:08 schrieb Michael Pyne: I would recommend applying the patch to 4.11 and master and then investigating whether QProcess would be suitable for master (this might also help with KF5 and Windows porting, not that I expect KInfoCenter to do great things on Windows). Ok, I will commit this then. Thank you. I will have a look in porting this to QProcess in a few days. Kind Regards, Wolfgang
Re: kde-workspace git broken ?
Am Montag, 11. November 2013, 14:13:22 schrieben Sie: On Monday 11 November 2013 13:29:32 Hugo Pereira Da Costa wrote: Hello, I think commits 9f70241d57f3ba1013b9f28650478c8bbb1233e0 137dd285bdf821fd2c8a5c17e30dc9c1a6eca87b 09ea308ab55505efe7aeaebcd4aef6292cd884e6 seriously broke kde-workspace yes, it's broken. I already created a sysadmin request. Unfortunately a similar commit got just pushed to KDE/4.11. So at the moment both master and KDE/4.11 branch are broken. Please remember: do not push a revert of a merge commit. Cheers Martin Sorry, it was my fault. And there's also commit 925af7a94eacbe7fef101c6e0f6415b1898d3bfe now. My most sincere apologies for that. But I do not really know what I did wrong in the first place. Can those 4 commits somehow be reverted? Thanks. Kindest Regards,
Re: Review Request 113779: KInfocenter/OpenGL: fix ReadPipe() in the case that the command cannot be run
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/113779/ --- (Updated Nov. 11, 2013, 11:31 a.m.) Status -- This change has been marked as submitted. Review request for kde-workspace and David Stephen Hubner. Bugs: 327382 http://bugs.kde.org/show_bug.cgi?id=327382 Repository: kde-workspace Description --- ReadPipe() doesn't return 0 as expected in the case that the command is not found. but the length of sh's output which is command not found in this case. This is because popen() does not fail if the command is not found, because it _can_ run sh. (according to the man page, popen calls /bin/sh -c command) To fix this, ReadPipe() should check the return code of the call to pclose() (see man pclose), and return 0 if this is not equal to 0. Diffs - kinfocenter/Modules/opengl/opengl.cpp 7df2b17 Diff: http://git.reviewboard.kde.org/r/113779/diff/ Testing --- Run KInfocenter on openSUSE, where lspci is in /sbin and that is not in the user's path. Without this patch, 3D Accelerator will be shown as unknown (because lspci cannot be run, with this patch it works as intended. I also tried with lspci in /usr/bin/ (i.e. in the path) and completely removed, worked as expected (correct information in the former case, unknown in the latter). Thanks, Wolfgang Bauer
RE: Review Request 113779: KInfocenter/OpenGL: fix ReadPipe() in the case that the command cannot be run
-Original Message- From: Rolf Eike Beer [mailto:k...@opensource.sf-tec.de] Sent: Sunday, November 10, 2013 9:11 AM To: kde-core-devel@kde.org; Wolfgang Bauer Cc: David Stephen Hubner Subject: Re: Review Request 113779: KInfocenter/OpenGL: fix ReadPipe() in the case that the command cannot be run Can't this be ported to simply use QProcess instead? Eike Well, I only wanted to fix this bug... ;-) But I can have a look at reimplementing this with QProcess as well. How should I proceed? Commit this for now to fix the bug (maybe to the 4.11 branch only)? Or discard this patch and just rewrite ReadPipe() using QProcess? Kind Regards, Wolfgang
Review Request 113779: KInfocenter/OpenGL: fix ReadPipe() in the case that the command cannot be run
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/113779/ --- Review request for kde-workspace and David Stephen Hubner. Bugs: 327382 http://bugs.kde.org/show_bug.cgi?id=327382 Repository: kde-workspace Description --- ReadPipe() doesn't return 0 as expected in the case that the command is not found. but the length of sh's output which is command not found in this case. This is because popen() does not fail if the command is not found, because it _can_ run sh. (according to the man page, popen calls /bin/sh -c command) To fix this, ReadPipe() should check the return code of the call to pclose() (see man pclose), and return 0 if this is not equal to 0. Diffs - kinfocenter/Modules/opengl/opengl.cpp 7df2b17 Diff: http://git.reviewboard.kde.org/r/113779/diff/ Testing --- Run KInfocenter on openSUSE, where lspci is in /sbin and that is not in the user's path. Without this patch, 3D Accelerator will be shown as unknown (because lspci cannot be run, with this patch it works as intended. I also tried with lspci in /usr/bin/ (i.e. in the path) and completely removed, worked as expected (correct information in the former case, unknown in the latter). Thanks, Wolfgang Bauer
Re: Review Request 113185: Cursor Theme KCM: Show correct resize cursor in preview for themes without a file called size_fdiag
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/113185/ --- (Updated Oct. 13, 2013, 4:18 p.m.) Status -- This change has been marked as submitted. Review request for kde-workspace, kwin, Fredrik Höglund, and Thomas Lübking. Bugs: 325837 http://bugs.kde.org/show_bug.cgi?id=325837 Repository: kde-workspace Description --- Apparently in XCursorTheme::findAlternative() (file kcontrol/input/xcursor/xcursortheme.cpp) the alternatives for size_bdiag and size_fdiag are swapped, so for themes not containing size_fdiag the wrong resize cursor is shown in the preview. This patch fixes that long standing bug. (there has been no change to that function since 2007!) This also fixes the glitch mentioned in bug#325763, that the wrong arrows are used for the window resize hint after the theme change is applied (for the current X session). Diffs - kcontrol/input/xcursor/xcursortheme.cpp 010c9ad Diff: http://git.reviewboard.kde.org/r/113185/diff/ Testing --- - Enter systemsettings-Workspace Appearance-Cursor Theme - Select a theme without size_fdiag, f.e.: crystalwhite, DMZ, Adwaita - Look at the preview: without the patch, the wrong resize cursor is shown, with the patch it's the same as for Oxygen e.g. See atached screenshots File Attachments KCM without the patch http://git.reviewboard.kde.org/media/uploaded/files/2013/10/10/9cb9ae8c-6614-49ea-aae2-fdbeb36dd71e__cursor.png KCM with the patch http://git.reviewboard.kde.org/media/uploaded/files/2013/10/10/f3cf8c6d-d2a0-4e96-8f77-75a53f66395f__cursor2.png Thanks, Wolfgang Bauer
Review Request 113185: Cursor Theme KCM: Show correct resize cursor in preview for themes without a file called size_fdiag
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/113185/ --- Review request for kde-workspace, kwin, Fredrik Höglund, and Thomas Lübking. Bugs: 325837 http://bugs.kde.org/show_bug.cgi?id=325837 Repository: kde-workspace Description --- Apparently in XCursorTheme::findAlternative() (file kcontrol/input/xcursor/xcursortheme.cpp) the alternatives for size_bdiag and size_fdiag are swapped, so for themes not containing size_fdiag the wrong resize cursor is shown in the preview. This patch fixes that long standing bug. (there has been no change to that function since 2007!) This also fixes the glitch mentioned in bug#325763, that the wrong arrows are used for the window resize hint after the theme change is applied (for the current X session). Diffs - kcontrol/input/xcursor/xcursortheme.cpp 010c9ad Diff: http://git.reviewboard.kde.org/r/113185/diff/ Testing --- - Enter systemsettings-Workspace Appearance-Cursor Theme - Select a theme without size_fdiag, f.e.: crystalwhite, DMZ, Adwaita, redglass - Look at the preview: without the patch, the wrong resize cursor is shown, with the patch it's the same as for Oxygen e.g. See atached screenshots File Attachments KCM without the patch http://git.reviewboard.kde.org/media/uploaded/files/2013/10/10/9cb9ae8c-6614-49ea-aae2-fdbeb36dd71e__cursor.png KCM with the patch http://git.reviewboard.kde.org/media/uploaded/files/2013/10/10/f3cf8c6d-d2a0-4e96-8f77-75a53f66395f__cursor2.png Thanks, Wolfgang Bauer
Re: Review Request 113185: Cursor Theme KCM: Show correct resize cursor in preview for themes without a file called size_fdiag
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/113185/ --- (Updated Oct. 10, 2013, 11:51 a.m.) Review request for kde-workspace, kwin, Fredrik Höglund, and Thomas Lübking. Bugs: 325837 http://bugs.kde.org/show_bug.cgi?id=325837 Repository: kde-workspace Description --- Apparently in XCursorTheme::findAlternative() (file kcontrol/input/xcursor/xcursortheme.cpp) the alternatives for size_bdiag and size_fdiag are swapped, so for themes not containing size_fdiag the wrong resize cursor is shown in the preview. This patch fixes that long standing bug. (there has been no change to that function since 2007!) This also fixes the glitch mentioned in bug#325763, that the wrong arrows are used for the window resize hint after the theme change is applied (for the current X session). Diffs - kcontrol/input/xcursor/xcursortheme.cpp 010c9ad Diff: http://git.reviewboard.kde.org/r/113185/diff/ Testing (updated) --- - Enter systemsettings-Workspace Appearance-Cursor Theme - Select a theme without size_fdiag, f.e.: crystalwhite, DMZ, Adwaita - Look at the preview: without the patch, the wrong resize cursor is shown, with the patch it's the same as for Oxygen e.g. See atached screenshots File Attachments KCM without the patch http://git.reviewboard.kde.org/media/uploaded/files/2013/10/10/9cb9ae8c-6614-49ea-aae2-fdbeb36dd71e__cursor.png KCM with the patch http://git.reviewboard.kde.org/media/uploaded/files/2013/10/10/f3cf8c6d-d2a0-4e96-8f77-75a53f66395f__cursor2.png Thanks, Wolfgang Bauer
Re: Review Request 113185: Cursor Theme KCM: Show correct resize cursor in preview for themes without a file called size_fdiag
On Oct. 10, 2013, 1:21 p.m., Thomas Lübking wrote: https://bugs.kde.org/show_bug.cgi?id=54359 If Fredrik's right, the tutorial used by most cursor creators would be wrong (great...) I guess to be absolutely sure, we'll have to XCreateFontCursor and then somehow get the bitmaps hash (XGetImage and on the bits? No idea...) For the moment i'll trust Fredrik more ;-) Well, according to https://bugs.kde.org/show_bug.cgi?id=248599#c6, size_fdiag in QCursor does have the hash c7088f0f3e6c8088236ef8e1e3e7, so my patch would be correct. I haven't verified that, though. - Wolfgang --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/113185/#review41484 --- On Oct. 10, 2013, 11:51 a.m., Wolfgang Bauer wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/113185/ --- (Updated Oct. 10, 2013, 11:51 a.m.) Review request for kde-workspace, kwin, Fredrik Höglund, and Thomas Lübking. Bugs: 325837 http://bugs.kde.org/show_bug.cgi?id=325837 Repository: kde-workspace Description --- Apparently in XCursorTheme::findAlternative() (file kcontrol/input/xcursor/xcursortheme.cpp) the alternatives for size_bdiag and size_fdiag are swapped, so for themes not containing size_fdiag the wrong resize cursor is shown in the preview. This patch fixes that long standing bug. (there has been no change to that function since 2007!) This also fixes the glitch mentioned in bug#325763, that the wrong arrows are used for the window resize hint after the theme change is applied (for the current X session). Diffs - kcontrol/input/xcursor/xcursortheme.cpp 010c9ad Diff: http://git.reviewboard.kde.org/r/113185/diff/ Testing --- - Enter systemsettings-Workspace Appearance-Cursor Theme - Select a theme without size_fdiag, f.e.: crystalwhite, DMZ, Adwaita - Look at the preview: without the patch, the wrong resize cursor is shown, with the patch it's the same as for Oxygen e.g. See atached screenshots File Attachments KCM without the patch http://git.reviewboard.kde.org/media/uploaded/files/2013/10/10/9cb9ae8c-6614-49ea-aae2-fdbeb36dd71e__cursor.png KCM with the patch http://git.reviewboard.kde.org/media/uploaded/files/2013/10/10/f3cf8c6d-d2a0-4e96-8f77-75a53f66395f__cursor2.png Thanks, Wolfgang Bauer
Re: Review Request 113185: Cursor Theme KCM: Show correct resize cursor in preview for themes without a file called size_fdiag
On Oct. 10, 2013, 1:21 p.m., Thomas Lübking wrote: https://bugs.kde.org/show_bug.cgi?id=54359 If Fredrik's right, the tutorial used by most cursor creators would be wrong (great...) I guess to be absolutely sure, we'll have to XCreateFontCursor and then somehow get the bitmaps hash (XGetImage and on the bits? No idea...) For the moment i'll trust Fredrik more ;-) Wolfgang Bauer wrote: Well, according to https://bugs.kde.org/show_bug.cgi?id=248599#c6, size_fdiag in QCursor does have the hash c7088f0f3e6c8088236ef8e1e3e7, so my patch would be correct. I haven't verified that, though. Thomas Lübking wrote: The cursor in cur_fdiag_bits displays the top_left_corner bits (nw-se) what Fredrik claims to be wrong (and for forward i'd indeed expect sw-ne) The output for the provided applet is: size_fdiag hash: c7088f0f3e6c8088236ef8e1e3e7 size_bdiag hash: fcf1c3c7cd4491d801f1e1c78f10 So the top_left_corner hash would be c7088f0f3e6c8088236ef8e1e3e7, what should also be size_bdiag. You mean the arrows from the top-left to the bottom-right corner? But then it is wrong in the Oxygen themes and KDE_Classic as shipped with KDE, size_bdiag is from bottom-left to top-right in both themes (and size_fdiag from top-left to bottom-right) - Wolfgang --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/113185/#review41484 --- On Oct. 10, 2013, 11:51 a.m., Wolfgang Bauer wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/113185/ --- (Updated Oct. 10, 2013, 11:51 a.m.) Review request for kde-workspace, kwin, Fredrik Höglund, and Thomas Lübking. Bugs: 325837 http://bugs.kde.org/show_bug.cgi?id=325837 Repository: kde-workspace Description --- Apparently in XCursorTheme::findAlternative() (file kcontrol/input/xcursor/xcursortheme.cpp) the alternatives for size_bdiag and size_fdiag are swapped, so for themes not containing size_fdiag the wrong resize cursor is shown in the preview. This patch fixes that long standing bug. (there has been no change to that function since 2007!) This also fixes the glitch mentioned in bug#325763, that the wrong arrows are used for the window resize hint after the theme change is applied (for the current X session). Diffs - kcontrol/input/xcursor/xcursortheme.cpp 010c9ad Diff: http://git.reviewboard.kde.org/r/113185/diff/ Testing --- - Enter systemsettings-Workspace Appearance-Cursor Theme - Select a theme without size_fdiag, f.e.: crystalwhite, DMZ, Adwaita - Look at the preview: without the patch, the wrong resize cursor is shown, with the patch it's the same as for Oxygen e.g. See atached screenshots File Attachments KCM without the patch http://git.reviewboard.kde.org/media/uploaded/files/2013/10/10/9cb9ae8c-6614-49ea-aae2-fdbeb36dd71e__cursor.png KCM with the patch http://git.reviewboard.kde.org/media/uploaded/files/2013/10/10/f3cf8c6d-d2a0-4e96-8f77-75a53f66395f__cursor2.png Thanks, Wolfgang Bauer
Re: Review Request 113185: Cursor Theme KCM: Show correct resize cursor in preview for themes without a file called size_fdiag
On Oct. 10, 2013, 1:21 p.m., Thomas Lübking wrote: https://bugs.kde.org/show_bug.cgi?id=54359 If Fredrik's right, the tutorial used by most cursor creators would be wrong (great...) I guess to be absolutely sure, we'll have to XCreateFontCursor and then somehow get the bitmaps hash (XGetImage and on the bits? No idea...) For the moment i'll trust Fredrik more ;-) Wolfgang Bauer wrote: Well, according to https://bugs.kde.org/show_bug.cgi?id=248599#c6, size_fdiag in QCursor does have the hash c7088f0f3e6c8088236ef8e1e3e7, so my patch would be correct. I haven't verified that, though. Thomas Lübking wrote: The cursor in cur_fdiag_bits displays the top_left_corner bits (nw-se) what Fredrik claims to be wrong (and for forward i'd indeed expect sw-ne) The output for the provided applet is: size_fdiag hash: c7088f0f3e6c8088236ef8e1e3e7 size_bdiag hash: fcf1c3c7cd4491d801f1e1c78f10 So the top_left_corner hash would be c7088f0f3e6c8088236ef8e1e3e7, what should also be size_bdiag. Wolfgang Bauer wrote: You mean the arrows from the top-left to the bottom-right corner? But then it is wrong in the Oxygen themes and KDE_Classic as shipped with KDE, size_bdiag is from bottom-left to top-right in both themes (and size_fdiag from top-left to bottom-right) Thomas Lübking wrote: Yes, and that also matches the preview image for Qt::SizeBDiagCursor ... So, given size_fdiag is indeed top_left_corner, ie. nw - se and size_bdiag is sw - ne ie. top_right_corner, the present hashes are wrong and the fix correct. Given that we read nw - se that actually makes sense and matches Qt's coordinate system (where forward = sw - ne is more the Math/GL coordinate system) However, Qt announces Qt::SizeBDiagCursor as sw - ne and that is what users should get. Wait 24h to see whether Fredrik has an additional comment on it and otherwise ShipIt! However, Qt announces Qt::SizeBDiagCursor as sw - ne and that is what users should get. Sorry, I don't get that comment, why the However? That's the same statement as in the other sentences. Or is there a typo somewhere? But, of course, I will wait those 24h hours before committing. Thanks for your review! - Wolfgang --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/113185/#review41484 --- On Oct. 10, 2013, 11:51 a.m., Wolfgang Bauer wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/113185/ --- (Updated Oct. 10, 2013, 11:51 a.m.) Review request for kde-workspace, kwin, Fredrik Höglund, and Thomas Lübking. Bugs: 325837 http://bugs.kde.org/show_bug.cgi?id=325837 Repository: kde-workspace Description --- Apparently in XCursorTheme::findAlternative() (file kcontrol/input/xcursor/xcursortheme.cpp) the alternatives for size_bdiag and size_fdiag are swapped, so for themes not containing size_fdiag the wrong resize cursor is shown in the preview. This patch fixes that long standing bug. (there has been no change to that function since 2007!) This also fixes the glitch mentioned in bug#325763, that the wrong arrows are used for the window resize hint after the theme change is applied (for the current X session). Diffs - kcontrol/input/xcursor/xcursortheme.cpp 010c9ad Diff: http://git.reviewboard.kde.org/r/113185/diff/ Testing --- - Enter systemsettings-Workspace Appearance-Cursor Theme - Select a theme without size_fdiag, f.e.: crystalwhite, DMZ, Adwaita - Look at the preview: without the patch, the wrong resize cursor is shown, with the patch it's the same as for Oxygen e.g. See atached screenshots File Attachments KCM without the patch http://git.reviewboard.kde.org/media/uploaded/files/2013/10/10/9cb9ae8c-6614-49ea-aae2-fdbeb36dd71e__cursor.png KCM with the patch http://git.reviewboard.kde.org/media/uploaded/files/2013/10/10/f3cf8c6d-d2a0-4e96-8f77-75a53f66395f__cursor2.png Thanks, Wolfgang Bauer
Re: Review Request 113185: Cursor Theme KCM: Show correct resize cursor in preview for themes without a file called size_fdiag
On Oct. 10, 2013, 1:21 p.m., Thomas Lübking wrote: https://bugs.kde.org/show_bug.cgi?id=54359 If Fredrik's right, the tutorial used by most cursor creators would be wrong (great...) I guess to be absolutely sure, we'll have to XCreateFontCursor and then somehow get the bitmaps hash (XGetImage and on the bits? No idea...) For the moment i'll trust Fredrik more ;-) Wolfgang Bauer wrote: Well, according to https://bugs.kde.org/show_bug.cgi?id=248599#c6, size_fdiag in QCursor does have the hash c7088f0f3e6c8088236ef8e1e3e7, so my patch would be correct. I haven't verified that, though. Thomas Lübking wrote: The cursor in cur_fdiag_bits displays the top_left_corner bits (nw-se) what Fredrik claims to be wrong (and for forward i'd indeed expect sw-ne) The output for the provided applet is: size_fdiag hash: c7088f0f3e6c8088236ef8e1e3e7 size_bdiag hash: fcf1c3c7cd4491d801f1e1c78f10 So the top_left_corner hash would be c7088f0f3e6c8088236ef8e1e3e7, what should also be size_bdiag. Wolfgang Bauer wrote: You mean the arrows from the top-left to the bottom-right corner? But then it is wrong in the Oxygen themes and KDE_Classic as shipped with KDE, size_bdiag is from bottom-left to top-right in both themes (and size_fdiag from top-left to bottom-right) Thomas Lübking wrote: Yes, and that also matches the preview image for Qt::SizeBDiagCursor ... So, given size_fdiag is indeed top_left_corner, ie. nw - se and size_bdiag is sw - ne ie. top_right_corner, the present hashes are wrong and the fix correct. Given that we read nw - se that actually makes sense and matches Qt's coordinate system (where forward = sw - ne is more the Math/GL coordinate system) However, Qt announces Qt::SizeBDiagCursor as sw - ne and that is what users should get. Wait 24h to see whether Fredrik has an additional comment on it and otherwise ShipIt! Wolfgang Bauer wrote: However, Qt announces Qt::SizeBDiagCursor as sw - ne and that is what users should get. Sorry, I don't get that comment, why the However? That's the same statement as in the other sentences. Or is there a typo somewhere? But, of course, I will wait those 24h hours before committing. Thanks for your review! Thomas Lübking wrote: However [that may be] - Whatsoever Sidenote: neutral++ has fd_double_arrow - top_right_corner but size_fdialog - top_left_corner This is a complete mess =D Ah ok. I read However as contradiction... ;-) In Crystal White fd_double_arrow is the same as top_right_corner, too. And fcf1c3c7cd4491d801f1e1c78f10 is a link to that. And the same with bd_double_arrow, top_left_corner and c7088f0f3e6c8088236ef8e1e3e7. - Wolfgang --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/113185/#review41484 --- On Oct. 10, 2013, 11:51 a.m., Wolfgang Bauer wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/113185/ --- (Updated Oct. 10, 2013, 11:51 a.m.) Review request for kde-workspace, kwin, Fredrik Höglund, and Thomas Lübking. Bugs: 325837 http://bugs.kde.org/show_bug.cgi?id=325837 Repository: kde-workspace Description --- Apparently in XCursorTheme::findAlternative() (file kcontrol/input/xcursor/xcursortheme.cpp) the alternatives for size_bdiag and size_fdiag are swapped, so for themes not containing size_fdiag the wrong resize cursor is shown in the preview. This patch fixes that long standing bug. (there has been no change to that function since 2007!) This also fixes the glitch mentioned in bug#325763, that the wrong arrows are used for the window resize hint after the theme change is applied (for the current X session). Diffs - kcontrol/input/xcursor/xcursortheme.cpp 010c9ad Diff: http://git.reviewboard.kde.org/r/113185/diff/ Testing --- - Enter systemsettings-Workspace Appearance-Cursor Theme - Select a theme without size_fdiag, f.e.: crystalwhite, DMZ, Adwaita - Look at the preview: without the patch, the wrong resize cursor is shown, with the patch it's the same as for Oxygen e.g. See atached screenshots File Attachments KCM without the patch http://git.reviewboard.kde.org/media/uploaded/files/2013/10/10/9cb9ae8c-6614-49ea-aae2-fdbeb36dd71e__cursor.png KCM with the patch http://git.reviewboard.kde.org/media/uploaded/files/2013/10/10/f3cf8c6d-d2a0-4e96-8f77-75a53f66395f__cursor2.png Thanks, Wolfgang Bauer
Re: Review Request 113127: Fix click on trash plasmoid when on desktop and widgets are unlocked
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/113127/ --- (Updated Oct. 7, 2013, 5:42 p.m.) Status -- This change has been marked as submitted. Review request for kde-workspace, Plasma and Marco Martin. Bugs: 325330 http://bugs.kde.org/show_bug.cgi?id=325330 Repository: kde-workspace Description --- Don't register the icon as a draggable with the applet. This causes events to be intercepted which prevents launching the file manager when the applet is movable. The same has been done for the icon plasmoid some time ago: http://commits.kde.org/kde-workspace/2a685d9a1d87d11680970cea88cdcc96da17d514 Diffs - plasma/desktop/applets/trash/trash.cpp d8007c0 Diff: http://git.reviewboard.kde.org/r/113127/diff/ Testing --- - Add a trash plasmoid to the desktop, make sure widgets are unlocked. - Click on the trash - file manager gets started every time, without the patch nothing happens (most of the time). Thanks, Wolfgang Bauer
Review Request 113127: Fix click on trash plasmoid when on desktop and widgets are unlocked
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/113127/ --- Review request for kde-workspace and Marco Martin. Bugs: 325330 http://bugs.kde.org/show_bug.cgi?id=325330 Repository: kde-workspace Description --- Don't register the icon as a draggable with the applet. This causes events to be intercepted which prevents launching the file manager when the applet is movable. The same has been done for the icon plasmoid some time ago: http://commits.kde.org/kde-workspace/2a685d9a1d87d11680970cea88cdcc96da17d514 Diffs - plasma/desktop/applets/trash/trash.cpp d8007c0 Diff: http://git.reviewboard.kde.org/r/113127/diff/ Testing --- - Add a trash plasmoid to the desktop, make sure widgets are unlocked. - Click on the trash - file manager gets started every time, without the patch nothing happens (most of the time). Thanks, Wolfgang Bauer
Re: Review Request 109609: disable session management for screensavers
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/109609/ --- (Updated Sept. 11, 2013, 1:15 a.m.) Review request for kde-workspace and Oliver Henshaw. Changes --- Changed the description a little bit, f.e. point to the proposal in the bug report. Description (updated) --- This patch disables session management for screensavers. Therefore no screensaver windows popup anymore on login if processes keep on running when the user logs out. As proposed in https://bugs.kde.org/show_bug.cgi?id=314859#c18 This doesn't solve the underlying problem of bug#224200 of course, but provides better user experience for now, so please accept. And IMHO it doesn't make sense to restore screensavers on login, anyway. This addresses bug 314859. http://bugs.kde.org/show_bug.cgi?id=314859 Diffs - kscreensaver/libkscreensaver/main.cpp 6851ba1 Diff: http://git.reviewboard.kde.org/r/109609/diff/ Testing --- - Turn on the Blank Screen (kblankscrn.kss) screensaver in systemsettings. - Wait for the screensaver to kick in - Verify that the process is still running (due to bug#224200) - log out - log in again Now no kblankscrn.kss window is opened anymore (without the patch, a window should appear) Thanks, Wolfgang Bauer
Re: Review Request 109611: Add option to show Recently Installed apps in kickoff plasmoid
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/109611/ --- (Updated April 2, 2013, 8:07 p.m.) Review request for kde-workspace. Changes --- added screenshots Description --- (This comes from a patch included in openSUSE) This patch makes kickoff remember all the .desktop files it sees (in kickoffrc). New ones are additionally shown in a seperate submenu named Recently Installed (for 3 days). This can be toggled on and off in the plasmoid's settings. This addresses bug 316916. http://bugs.kde.org/show_bug.cgi?id=316916 Diffs - plasma/desktop/applets/kickoff/applet/applet.cpp a6f7379 plasma/desktop/applets/kickoff/applet/kickoffConfig.ui 8664ac8 plasma/desktop/applets/kickoff/core/applicationmodel.h f0f8872 plasma/desktop/applets/kickoff/core/applicationmodel.cpp 57b6ba5 plasma/desktop/applets/kickoff/simpleapplet/simpleapplet.cpp 28fba18 plasma/desktop/applets/kickoff/ui/launcher.h 2a234c3 plasma/desktop/applets/kickoff/ui/launcher.cpp 4425bcc Diff: http://git.reviewboard.kde.org/r/109611/diff/ Testing --- Created a new .desktop file in ~/.local/share/applications, ran kbuildsycoca4 and the menu Recently Installed appeared with this entry. After logout/login this is still present. Deleted the .desktop file again, ran kbuildsycoca4 and the menu Recently Installed disappeared again. I have been using the (vanilla KDE) kickoff applet with this patch for about a week now. Also this patch is already part of openSUSE for several years... File Attachments (updated) Settings dialog (kickoff style) with patch http://git.reviewboard.kde.org/media/uploaded/files/2013/04/02/kickoff-settings.png Plasmoid (kickoff style) showing the Recently Installed submenu http://git.reviewboard.kde.org/media/uploaded/files/2013/04/02/kickoff.png Settings dialog (classic style) with patch http://git.reviewboard.kde.org/media/uploaded/files/2013/04/02/classic-settings.png Plasmoid (classic style) showing the Recently Installed submenu http://git.reviewboard.kde.org/media/uploaded/files/2013/04/02/classic.png Thanks, Wolfgang Bauer
Review Request 109611: Add option to show Recently Installed apps in kickoff plasmoid
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/109611/ --- Review request for kde-workspace. Description --- (This comes from a patch included in openSUSE) This patch makes kickoff remember all the .desktop files it sees (in kickoffrc). New ones are additionally shown in a seperate submenu named Recently Installed (for 3 days). This can be toggled on and off in the plasmoid's settings. This addresses bug 316916. http://bugs.kde.org/show_bug.cgi?id=316916 Diffs - plasma/desktop/applets/kickoff/applet/applet.cpp a6f7379 plasma/desktop/applets/kickoff/applet/kickoffConfig.ui 8664ac8 plasma/desktop/applets/kickoff/core/applicationmodel.h f0f8872 plasma/desktop/applets/kickoff/core/applicationmodel.cpp 57b6ba5 plasma/desktop/applets/kickoff/simpleapplet/simpleapplet.cpp 28fba18 plasma/desktop/applets/kickoff/ui/launcher.h 2a234c3 plasma/desktop/applets/kickoff/ui/launcher.cpp 4425bcc Diff: http://git.reviewboard.kde.org/r/109611/diff/ Testing --- Created a new .desktop file in ~/.local/share/applications, ran kbuildsycoca4 and the menu Recently Installed appeared with this entry. After logout/login this is still present. Deleted the .desktop file again, ran kbuildsycoca4 and the menu Recently Installed disappeared again. I have been using the (vanilla KDE) kickoff applet with this patch for about a week now. Also this patch is already part of openSUSE for several years... Thanks, Wolfgang Bauer
