Bug 341201 - lock screen should ignore first key press if on "sleep mode"
Summary: lock screen should ignore first key press if on "sleep mode"
Status: RESOLVED UPSTREAM
Alias: None
Product: ksmserver
Classification: Unmaintained
Component: lockscreen (show other bugs)
Version: unspecified
Platform: Other Linux
: NOR normal
Target Milestone: ---
Assignee: David Edmundson
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-11-23 21:24 UTC by Albert Astals Cid
Modified: 2016-02-15 14:57 UTC (History)
2 users (show)

See Also:
Latest Commit:
Version Fixed In: 5.6.0 (Wayland-only)
Sentry Crash Report:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Albert Astals Cid 2014-11-23 21:24:19 UTC
If the lock screen is on "sleep mode" i.e. the screen when to black, the first key press should only wake up the screen, not also move the list around (i.e if i press the right arrow for example)
Comment 1 Thomas Lübking 2014-11-23 21:55:56 UTC
Screenlocker grabs input. Not KWin related.
Comment 2 Albert Astals Cid 2014-11-23 22:01:23 UTC
Weren't all those blogs about how the screenlocker was now part of kwin since that was the only secure way to do it? Or did i read too much in between the lines?
Comment 3 Thomas Lübking 2014-11-23 22:05:37 UTC
It's in ksmserver - ide is that when you kill the screen grabber, you exit the session.

This are a little different for wayland (where I assume KWin will *have to* redirect the screensaver input as well) but even there the logics would happen inside the screenlocker process (which I doubt to be KWin then)
Comment 4 Martin Flöser 2014-11-24 08:17:26 UTC
(In reply to Albert Astals Cid from comment #2)
> Weren't all those blogs about how the screenlocker was now part of kwin
> since that was the only secure way to do it? Or did i read too much in
> between the lines?

see http://blog.martin-graesslin.com/blog/2011/11/work-not-done/

On the actual bug: it's tricky, I don't think the greeter process or ksld knows that the screen got blanked. Without that information one cannot do much. Personally I would say this is the wrong layer, such tasks should be done in X or Wayland to suppress the first input event.
Comment 5 Thomas Lübking 2014-11-24 15:38:16 UTC
Wouldn't it be sufficient (atm, on X11) to simply omit forwarding the unlocking event?

screenlocker/lockwindow.cpp:341
// XSendEvent(QX11Info::display(), targetWindow, False, NoEventMask, &ev2);
// TODO: send FocusIn instead ;-)
Comment 6 Martin Flöser 2014-11-24 16:21:59 UTC
> screenlocker/lockwindow.cpp:341
> // XSendEvent(QX11Info::display(), targetWindow, False, NoEventMask, &ev2);
> // TODO: send FocusIn instead ;-)

is that a kde4 path, because line 341 doesn't make sense on my checkout (it's 
a break).
Comment 7 Thomas Lübking 2014-11-24 16:26:50 UTC
Yupp, is now

0329                         if (responseType == XCB_KEY_PRESS || responseType == XCB_KEY_RELEASE) {
0330                             sendEvent<xcb_key_press_event_t>(event, window, targetX, targetY);
0331                         } else if (responseType == XCB_BUTTON_PRESS || responseType == XCB_BUTTON_RELEASE) {
0332                             sendEvent<xcb_button_press_event_t>(event, window, targetX, targetY);
0333                         } else if (responseType == XCB_MOTION_NOTIFY) {
0334                             sendEvent<xcb_motion_notify_event_t>(event, window, targetX, targetY);
0335                         }

Sorry.
Comment 8 Martin Flöser 2014-11-24 16:40:47 UTC
yeah that would work, but how do we know that the screen is blanked?
Comment 9 Thomas Lübking 2014-11-24 17:07:26 UTC
http://xcb.freedesktop.org/manual/group__XCB__DPMS__API.html

More readable API for brief oversight:
http://www.x.org/releases/X11R7.7/doc/libXext/dpmslib.html


Of course that does not hold for some in-screen magic, but I assume that's not the target here.

Another issue *could* be that we recive the event too late - we'd have to poll the dpms level to compare the event and the latest suspend time.
Comment 10 Martin Flöser 2015-02-11 09:41:40 UTC
I had a look at the extension yesterday. Unfortunately it does not provide any events to notify when DPMS switches states. This means the state needs to be polled which I wouldn't want to do after each key event as it introduces a roundtrip to the X server. As well it's probably rather pointless as the X server handles input events before sending to the clients. Whenever we get an input event DPMS will be off. The screen locker has no idea whether the input event triggered a disabling of DPMS.

This shows that there is exactly one place where this information is available and this place can filter out events: the X server. On further thought: this is not a screen locker specific problem. Also if you have DPMS enabled and the lock screen disabled, you probably want the first event to be discarded. That's something only the X Server could do -> RESOLVED UPSTREAM.
Comment 11 Martin Flöser 2016-02-15 14:56:39 UTC
Git commit 57b11f84296b69944b907a89d403dc50d36a75dc by Martin Gräßlin.
Committed on 15/02/2016 at 14:53.
Pushed by graesslin into branch 'master'.

[backends/drm] Use an InputEventFilter to reenable outputs

So far the DrmOutput connected to all input events when going into
power saving. As we now have the input filters it's better to just
install a filter when an output goes into powersave and remove the
input filter again when all outputs are enabled again.

To make this work InputRedirection gains a new method to add a new
filter as the first filter. This is a potentially dangerous method
as it allows to have a filter before LockScreenFilter gets the
events. But in case of DPMS it's something we actually want.

A nice new feature possible with the input filter is that we can
filter out the event which re-enables the outputs. Thus when getting
on a system with output off and screen locked, the first key hit
doesn't go to the lock screen.

Reviewed-By: Bhushan Shah
Fixed-in: 5.6.0 (Wayland-only)

M  +76   -14   backends/drm/drm_backend.cpp
M  +25   -1    backends/drm/drm_backend.h
M  +5    -0    input.cpp
M  +9    -1    input.h

http://commits.kde.org/kwin/57b11f84296b69944b907a89d403dc50d36a75dc