D20804: RFC: Try getting the X keyboard grab multiple times
This revision was automatically updated to reflect the committed changes. Closed by commit R133:4af9ea1e2c9d: Try getting the X keyboard grab multiple times (authored by davidedmundson). REPOSITORY R133 KScreenLocker CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D20804?vs=56950&id=59637 REVISION DETAIL https://phabricator.kde.org/D20804 AFFECTED FILES dbus/org.kde.screensaver.xml interface.cpp interface.h ksldapp.cpp ksldapp.h To: davidedmundson, #plasma, #kwin, zzag Cc: zzag, broulik, plasma-devel, LeGast00n, ericadams, jraleigh, GB_2, ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
D20804: RFC: Try getting the X keyboard grab multiple times
davidedmundson added a comment. > Arguably it won't help us with all cases because the clients would have to voluntarily participate in the protocol. Yes. I'm not pretending it will. --- > I wonder whether it's worth fixing this bug on X11 at all. There is a way to forcefully take a grab inside X internals. The only way this is exposed publicly is via the shortcut key XF86Ungrab setxkbmap -option grab:break_actions xdotool key XF86Ungrab It was meant as a dev tool so you could switch to a console to debug locks. Using this is a horrific idea as it introduces a keyboard shortcut to circumvent the screen locker!! There's no other public API, though I don't see why one couldn't be added. The bigger problem is that the client side has no indication that it loses it's lock which leaves it in a corrupt state when you log in. Adding that retroactively is the part I think is pragmatically impossible. REPOSITORY R133 KScreenLocker REVISION DETAIL https://phabricator.kde.org/D20804 To: davidedmundson, #plasma, #kwin Cc: zzag, broulik, plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
D20804: RFC: Try getting the X keyboard grab multiple times
zzag added a comment. Arguably it won't help us with all cases because the clients would have to voluntarily participate in the protocol. I wonder whether it's worth fixing this bug on X11 at all. REPOSITORY R133 KScreenLocker REVISION DETAIL https://phabricator.kde.org/D20804 To: davidedmundson, #plasma, #kwin Cc: zzag, broulik, plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
D20804: RFC: Try getting the X keyboard grab multiple times
zzag added a comment. > Wrote relevant matching kwin patch to close effects Have you uploaded it? :-) REPOSITORY R133 KScreenLocker REVISION DETAIL https://phabricator.kde.org/D20804 To: davidedmundson, #plasma, #kwin Cc: zzag, broulik, plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
D20804: RFC: Try getting the X keyboard grab multiple times
davidedmundson added a comment. > Shouldn't this and the above function name start with small letter? DBus methods and signals tend to start with an upper case. Though interface already consists of a mixture :/ Classes using Qt generated DBus adaptors have to match the interface they're implementing. REPOSITORY R133 KScreenLocker REVISION DETAIL https://phabricator.kde.org/D20804 To: davidedmundson, #plasma, #kwin Cc: shubham, broulik, plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
D20804: RFC: Try getting the X keyboard grab multiple times
shubham added inline comments. INLINE COMMENTS > interface.h:116 > > +void AboutToLock(); > + Shouldn't this and the above function name start with small letter? REPOSITORY R133 KScreenLocker REVISION DETAIL https://phabricator.kde.org/D20804 To: davidedmundson, #plasma, #kwin Cc: shubham, broulik, plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
D20804: RFC: Try getting the X keyboard grab multiple times
broulik added a comment. +1 REPOSITORY R133 KScreenLocker REVISION DETAIL https://phabricator.kde.org/D20804 To: davidedmundson, #plasma, #kwin Cc: broulik, plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
D20804: RFC: Try getting the X keyboard grab multiple times
davidedmundson created this revision. davidedmundson added reviewers: Plasma, KWin. Herald added a project: Plasma. Herald added a subscriber: plasma-devel. davidedmundson requested review of this revision. REVISION SUMMARY Right now when the screen locker starts it tries to grab the keyboard on X. This fails if any other application has grabbed the keyboard. In these situations the screen simply doesn't lock which is a pretty rubbish state. This can't be realistically fixed properly on X, on wayland it's a non-issue. However, we can minimise the occasions when this occurs. This patch emits a signal before locking and then tries the lack multiple times. Clients can listen for this event and release their keyboard grabs. TEST PLAN Wrote relevant matching kwin patch to close effects "sleep 5 ; loginctl lock-session" whilst a desktop effect was active still locked the session REPOSITORY R133 KScreenLocker BRANCH master REVISION DETAIL https://phabricator.kde.org/D20804 AFFECTED FILES dbus/org.kde.screensaver.xml interface.cpp interface.h ksldapp.cpp ksldapp.h To: davidedmundson, #plasma, #kwin Cc: plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
