D17706: Fix lock screen focus
This revision was automatically updated to reflect the committed changes. Closed by commit R133:2fdf002b11cf: Fix lock screen focus (authored by andreyby, committed by ngraham). REPOSITORY R133 KScreenLocker CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D17706?vs=50911=51194 REVISION DETAIL https://phabricator.kde.org/D17706 AFFECTED FILES greeter/greeterapp.cpp greeter/greeterapp.h To: andreyby, davidedmundson Cc: ngraham, graesslin, abetts, davidedmundson, plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, sebas, apol, mart
D17706: Fix lock screen focus
andreyby added a comment. mrd...@altlinux.org REVISION DETAIL https://phabricator.kde.org/D17706 To: andreyby, davidedmundson Cc: ngraham, graesslin, abetts, davidedmundson, plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, sebas, apol, mart
D17706: Fix lock screen focus
ngraham added a comment. @andreyby, can you let us know your email address so we can land this with correct authorship information? Thanks! REVISION DETAIL https://phabricator.kde.org/D17706 To: andreyby, davidedmundson Cc: ngraham, graesslin, abetts, davidedmundson, plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, sebas, apol, mart
D17706: Fix lock screen focus
andreyby added a comment. When we use the X11BypassWindowManagerHint flag with a call to the showFullScreen() function on the x11 platform, requestActivate does not make the desired window active. For correct work, on x11 we have to call the show() function. But when the focus is set on a window different from the main one, we cannot select a window without a focus with the mouse; for this, it is necessary to use mouse click events. REVISION DETAIL https://phabricator.kde.org/D17706 To: andreyby, davidedmundson Cc: ngraham, graesslin, abetts, davidedmundson, plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, sebas, apol, mart
D17706: Fix lock screen focus
ngraham added a comment. So what's required next here? REVISION DETAIL https://phabricator.kde.org/D17706 To: andreyby, davidedmundson Cc: ngraham, graesslin, abetts, davidedmundson, plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, sebas, apol, mart
D17706: Fix lock screen focus
davidedmundson added a comment. Seems better, but I'm not sure I understand exactly. The key difference here is we have an event filter on mouse clicks and call requestActivate when that happens. But you said earlier calling requestActivate doesn't do anything. REVISION DETAIL https://phabricator.kde.org/D17706 To: andreyby, davidedmundson Cc: ngraham, graesslin, abetts, davidedmundson, plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, sebas, apol, mart
D17706: Fix lock screen focus
andreyby updated this revision to Diff 50911. CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D17706?vs=50543=50911 REVISION DETAIL https://phabricator.kde.org/D17706 AFFECTED FILES greeter/greeterapp.cpp greeter/greeterapp.h To: andreyby, davidedmundson Cc: ngraham, graesslin, abetts, davidedmundson, plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, sebas, apol, mart
D17706: Fix lock screen focus
andreyby added a comment. On x11 flag removal does not affect the created window, we can use this to put the focus. INLINE COMMENTS > graesslin wrote in greeterapp.cpp:460 > David already said it: we cannot do this. Changing the > X11BypassWindowManagerHint flag after the window is created is not supported > by X11 protocol! We really, really cannot do this. I understand, but only without X11BypassWindowManagerHint flag set can the focus be set correctly REVISION DETAIL https://phabricator.kde.org/D17706 To: andreyby, davidedmundson Cc: ngraham, graesslin, abetts, davidedmundson, plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, sebas, apol, mart
D17706: Fix lock screen focus
andreyby updated this revision to Diff 50543. andreyby added a comment. It's works for x11 platform. CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D17706?vs=49444=50543 REVISION DETAIL https://phabricator.kde.org/D17706 AFFECTED FILES greeter/greeterapp.cpp To: andreyby, davidedmundson Cc: ngraham, graesslin, abetts, davidedmundson, plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, sebas, apol, mart
D17706: Fix lock screen focus
ngraham added a comment. @andreyby, can you adjust this patch to address @davidedmundson and @graesslin's concerns? REVISION DETAIL https://phabricator.kde.org/D17706 To: andreyby, davidedmundson Cc: ngraham, graesslin, abetts, davidedmundson, plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, sebas, apol, mart
D17706: Fix lock screen focus
andreyby updated this revision to Diff 49444. CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D17706?vs=47933=49444 REVISION DETAIL https://phabricator.kde.org/D17706 AFFECTED FILES greeter/greeterapp.cpp To: andreyby, davidedmundson Cc: graesslin, abetts, davidedmundson, plasma-devel, sukalyanbanga, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, sebas, apol, mart
D17706: Fix lock screen focus
graesslin added inline comments. INLINE COMMENTS > greeterapp.cpp:460 > // w->setFocus(Qt::OtherFocusReason); // FIXME > +w->setFlags(w->flags() | Qt::X11BypassWindowManagerHint); > } David already said it: we cannot do this. Changing the X11BypassWindowManagerHint flag after the window is created is not supported by X11 protocol! We really, really cannot do this. REPOSITORY R133 KScreenLocker REVISION DETAIL https://phabricator.kde.org/D17706 To: andreyby, davidedmundson Cc: graesslin, abetts, davidedmundson, plasma-devel, kvanton, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, sebas, apol, mart
D17706: Fix lock screen focus
davidedmundson requested changes to this revision. davidedmundson added a comment. This revision now requires changes to proceed. > so it needs to be removed, change the focus, put flag back. We can't do that. REPOSITORY R133 KScreenLocker REVISION DETAIL https://phabricator.kde.org/D17706 To: andreyby, davidedmundson Cc: abetts, davidedmundson, plasma-devel, kvanton, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, sebas, apol, mart
D17706: Fix lock screen focus
andreyby added a comment. What do you think? REPOSITORY R133 KScreenLocker REVISION DETAIL https://phabricator.kde.org/D17706 To: andreyby, davidedmundson Cc: abetts, davidedmundson, plasma-devel, kvanton, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, sebas, apol, mart
D17706: Fix lock screen focus
abetts added a comment. > **abetts ** > It is easy to reproduce when more than one monitor is connected, you must set the cursor on any monitor except the first one and block the screen with hot keys, after which you will notice that the focus of the password entry remains on the first screen. > This problem may be associated with the bug 395639. Thanks for the explanation. Makes sense! REPOSITORY R133 KScreenLocker REVISION DETAIL https://phabricator.kde.org/D17706 To: andreyby, davidedmundson Cc: abetts, davidedmundson, plasma-devel, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, sebas, apol, mart
D17706: Fix lock screen focus
andreyby added a comment. **davidedmundson ** //we don't want to cursor grab in testing mode, this change is lost//yes, did not notice //This is still relevant A cursor can be in none.//I think this is superfluous, because the default focus is on the first screen. //why are we changing the window flags?// Because with the flag X11BypassWindowManagerHint the focus will not be able to change, so it needs to be removed, change the focus, put flag back. I rewrited the patch with minimal changes. **abetts ** It is easy to reproduce when more than one monitor is connected, you must set the cursor on any monitor except the first one and block the screen with hot keys, after which you will notice that the focus of the password entry remains on the first screen. This problem may be associated with the bug 395639. REPOSITORY R133 KScreenLocker REVISION DETAIL https://phabricator.kde.org/D17706 To: andreyby, davidedmundson Cc: abetts, davidedmundson, plasma-devel, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, sebas, apol, mart
D17706: Fix lock screen focus
andreyby updated this revision to Diff 47933. REPOSITORY R133 KScreenLocker CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D17706?vs=47879=47933 REVISION DETAIL https://phabricator.kde.org/D17706 AFFECTED FILES greeter/greeterapp.cpp To: andreyby, davidedmundson Cc: abetts, davidedmundson, plasma-devel, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, sebas, apol, mart
D17706: Fix lock screen focus
abetts added a comment. Can you show a video or gif of this behavior? REPOSITORY R133 KScreenLocker REVISION DETAIL https://phabricator.kde.org/D17706 To: andreyby, davidedmundson Cc: abetts, davidedmundson, plasma-devel, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, sebas, apol, mart
D17706: Fix lock screen focus
davidedmundson added inline comments.
INLINE COMMENTS
> greeterapp.cpp:436
> +
> +view->setFlags(view->flags() | Qt::X11BypassWindowManagerHint);
> +
why are we changing the window flags?
> greeterapp.cpp:451-453
> -if (!w) { // fallback solution
> -w = m_views.first();
> -}
This is still relevant
A cursor can be in none.
> greeterapp.cpp:456
> -// focus setting is still required for proper internal QWidget state
> (and eg. visual reflection)
> -if (!m_testing) {
> -w->setKeyboardGrabEnabled(true); // TODO - check whether this still
> works in master!
we don't want to cursor grab in testing mode, this change is lost
REPOSITORY
R133 KScreenLocker
REVISION DETAIL
https://phabricator.kde.org/D17706
To: andreyby
Cc: davidedmundson, plasma-devel, GB_2, ragreen, Pitel, ZrenBot, lesliezhai,
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
D17706: Fix lock screen focus
andreyby created this revision. Herald added a project: Plasma. Herald added a subscriber: plasma-devel. andreyby requested review of this revision. REVISION SUMMARY When the screen is locked, the focus always remains on the main screen, this patch fixes the problem. Now the focus is on the screen where the cursor is located. REPOSITORY R133 KScreenLocker REVISION DETAIL https://phabricator.kde.org/D17706 AFFECTED FILES greeter/greeterapp.cpp To: andreyby Cc: plasma-devel, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
