Implement manual focus on click
ClosedPublic

Authored by fvogt on Feb 27 2017, 3:31 PM.

Details

Summary

Currently only the first created screenlock window gets focus.
On clicks, no focus events are sent, which makes it impossible to input
passwords. This patch now makes it possible to focus to a different
screenlock window (on a different monitor, for example) using a mouse
button press.
This should also fix newly created screenlock windows stealing the focus
of already displayed ones as only the first window gains automatic focus.

BUG: 348789
BUG: 374289

Test Plan

Locked the screen, now I can use the password input on the secondary screen
as well.

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.
fvogt created this revision.Feb 27 2017, 3:31 PM
Restricted Application added a project: Plasma. · View Herald TranscriptFeb 27 2017, 3:31 PM
broulik edited reviewers, added: Plasma; removed: plasma-devel.Feb 27 2017, 3:38 PM
graesslin accepted this revision.Feb 27 2017, 4:02 PM
This revision is now accepted and ready to land.Feb 27 2017, 4:02 PM
fvogt added a comment.Feb 27 2017, 5:49 PM

This is weird: While the fix works absolutely fine (and reliably) for me on my primary user account,
it does not have any effect for a different user around and @broulik

Even weirder: I cannot reproduce the original bug in any of my VMs using multihead QXL...

So while it does improve the situation I won't push this without further investigation for now.

Just as an fyi: in the build tree there is a test app which does the complete locking without the need for restart the session.

Overall I think that the problem might also be in the greeter. On Wayland we have the same issue, which could indicate a problem in the greeter as well.

fvogt added a comment.Feb 27 2017, 6:31 PM

Just as an fyi: in the build tree there is a test app which does the complete locking without the need for restart the session.

Would that really work for changes in x11locker.cpp as well? it's part of libkscreenlocker, linked to ksmserver.
So the only way to test it would be a nested/virtual X server with a separate ksmserver, wouldn't it?
As far as I can tell, kscreenlocker_test (if that's what you mean) only tests the locker and not the ksmserver part.

Overall I think that the problem might also be in the greeter.

It works fine with kscreenlocker_greet --testing

On Wayland we have the same issue, which could indicate a problem in the greeter as well.

Multiscreen on wayland is borked on my system for some reason, so I can't test that...

fvogt updated this revision to Diff 11916.Feb 27 2017, 7:32 PM

Handle the case better when the currently focused window vanishes

I finally figured the issue out.
Newly appearing windows try to steal focus, which makes m_focusedLockWindow actually wrong.
That explains why it works only for some and only for some setups...
I'll try to fix that by adding

setAttribute(Qt::WA_ShowWithoutActivating);

to the greeter windows.
Now the question is whether that would break the wayland locker as there is no call to fakeFocusIn on wayland.
Either that code needs to be X11 only or the focus handling fixed under wayland as well, which I cannot do as I
cannot get multiscreen on wayland to work...

fvogt updated this revision to Diff 11941.Feb 28 2017, 11:49 AM

Newly created windows do not steal focus anymore

Needs a quick test that it does not break wayland (e.g. no focus at all)

fvogt requested review of this revision.Feb 28 2017, 11:50 AM
fvogt edited edge metadata.
broulik edited edge metadata.Mar 1 2017, 9:20 AM

On X11 screen locker is initially focused and clicking on different screen focuses them. I didn't test screens being added/removed, though.
On Wayland screen locker is initially focused but clicking on different screens doesn't focus them. At least it doesn't make it worse than the status quo.

hein added a subscriber: hein.Mar 1 2017, 2:02 PM

On Wayland screen locker is initially focused but clicking on different screens doesn't focus them.

If this is outstanding, maybe add a FIXME TODO so we don't forget ...?

fvogt added a comment.Mar 1 2017, 2:13 PM
In D4821#91393, @hein wrote:

On Wayland screen locker is initially focused but clicking on different screens doesn't focus them.

If this is outstanding, maybe add a FIXME TODO so we don't forget ...?

Yes, but that can only be fixed in kwin (?)

The right approach to finally fix this issue (and also get rid of a lot of code) would be to get the shared typing working again, which likely needs a total rewrite for Qt Quick 2.

In D4821#91406, @fvogt wrote:
In D4821#91393, @hein wrote:

On Wayland screen locker is initially focused but clicking on different screens doesn't focus them.

If this is outstanding, maybe add a FIXME TODO so we don't forget ...?

Yes, but that can only be fixed in kwin (?)

We don't know what the problem is on Wayland. It might be a problem in kwin, kscreenlocker_greet or what I think: QtWayland. Given that I think we can ignore the Wayland case here and just fix the X11 case.

This revision was automatically updated to reflect the committed changes.