Fix lock screen focus
ClosedPublic

Authored by andreyby on Dec 20 2018, 9:30 AM.

Details

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.

BUG: 395639
FIXED-IN: 5.16.0

Diff Detail

Repository
R133 KScreenLocker
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
andreyby created this revision.Dec 20 2018, 9:30 AM
Restricted Application added a project: Plasma. · View Herald TranscriptDec 20 2018, 9:30 AM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
andreyby requested review of this revision.Dec 20 2018, 9:30 AM
davidedmundson added inline comments.
greeter/greeterapp.cpp
436

why are we changing the window flags?

451–453

This is still relevant

A cursor can be in none.

456

we don't want to cursor grab in testing mode, this change is lost

davidedmundson requested changes to this revision.Dec 20 2018, 10:29 AM
This revision now requires changes to proceed.Dec 20 2018, 10:29 AM
abetts added a subscriber: abetts.Dec 20 2018, 2:42 PM

Can you show a video or gif of this behavior?

andreyby updated this revision to Diff 47933.Dec 21 2018, 7:34 AM

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.

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!

What do you think?

davidedmundson requested changes to this revision.Jan 9 2019, 12:03 PM

so it needs to be removed, change the focus, put flag back.

We can't do that.

This revision now requires changes to proceed.Jan 9 2019, 12:03 PM
graesslin added inline comments.
greeter/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.

andreyby updated this revision to Diff 49444.Jan 14 2019, 2:32 PM
ngraham edited the summary of this revision. (Show Details)Thu, Jan 24, 6:40 PM
ngraham added a subscriber: ngraham.

@andreyby, can you adjust this patch to address @davidedmundson and @graesslin's concerns?

andreyby updated this revision to Diff 50543.Wed, Jan 30, 9:51 AM

It's works for x11 platform.

This comment was removed by andreyby.
greeter/greeterapp.cpp
460

I understand, but only without X11BypassWindowManagerHint flag set can the focus be set correctly

andreyby updated this revision to Diff 50911.Tue, Feb 5, 8:26 AM

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.

So what's required next here?

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.

davidedmundson accepted this revision.Wed, Feb 6, 12:14 PM
This revision is now accepted and ready to land.Wed, Feb 6, 12:14 PM

@andreyby, can you let us know your email address so we can land this with correct authorship information? Thanks!

mrdrew@altlinux.org

This revision was automatically updated to reflect the committed changes.