Bug 497245 - Global shortcut keys are leaking into applications
Summary: Global shortcut keys are leaking into applications
Status: RESOLVED FIXED
Alias: None
Product: kwin
Classification: Plasma
Component: general (show other bugs)
Version: master
Platform: Other Linux
: NOR normal
Target Milestone: ---
Assignee: KWin default assignee
URL:
Keywords: regression
: 498258 (view as bug list)
Depends on:
Blocks:
 
Reported: 2024-12-09 20:00 UTC by Nicolas Fella
Modified: 2025-01-16 22:28 UTC (History)
4 users (show)

See Also:
Latest Commit:
Version Fixed In: 6.3.0
Sentry Crash Report:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Nicolas Fella 2024-12-09 20:00:44 UTC
STEPS TO REPRODUCE
1. Configure Yakuake to open on Meta+Space 
2. Open a Youtube video in Firefox
3. Press Meta+Space to open Yakuake
4. Press Meta+Space to close Yakuake

OBSERVED RESULT
The video pauses, presumably because the browser receives a Space key event

EXPECTED RESULT
The video does not pause

SOFTWARE/OS VERSIONS
KDE Plasma Version: master
KDE Frameworks Version: master
Qt Version: 6.8

ADDITIONAL INFORMATION
Bisected to https://invent.kde.org/plasma/kwin/-/commit/78bc268876343a313fcb2e7346985371e030ee71
Comment 1 Nate Graham 2024-12-10 22:54:10 UTC
Cannot reproduce with current git master and Emoji Selector configured to launch on Meta+Space, FWIW.
Comment 2 Zamundaaa 2024-12-12 02:15:11 UTC
Can confirm with krunner set to Meta+Space
Comment 3 David Edmundson 2025-01-03 11:37:04 UTC
>Cannot reproduce with current git master and Emoji Selector configured to launch on Meta+Space, FWIW.

The shortcut needs to close an app, this causes the focus change and the app to get a key which would otherwise be filtered.
Comment 4 David Edmundson 2025-01-03 14:13:53 UTC
We are sending to the client an enter event with the held keys, and then a release event. It never gets a key down.
This isn't inherently wrong. 

This is a complex case where every other fix has potential to break something else. Having it so wayland clients are never notified of grabbed keys is trivial, but there's very deliberate code to keep the seat interface up to date.
Comment 5 David Edmundson 2025-01-03 14:31:12 UTC
What might work is making KeyboardInputRedirection update m_pressedKeys after filters for key presses, but update always for key releases. 

That should might work out ok. That should solve our issue, because then meta+space won't be in the pressed keys at the time the focus switch happens.
Comment 6 Vlad Zahorodnii 2025-01-03 14:46:43 UTC
The client may not see some pressed keys when it expects to see some though. The problem is not new. I used to see it when using the "q" shortcut in mpv as well, it's connected to implicit grabs not working as expected. See also https://invent.kde.org/plasma/kwin/-/merge_requests/6725#note_1064308
Comment 7 Vlad Zahorodnii 2025-01-03 14:53:04 UTC
As the first step, we need to add more tests.
Comment 8 Nate Graham 2025-01-04 21:05:07 UTC
*** Bug 498258 has been marked as a duplicate of this bug. ***
Comment 9 MZD 2025-01-04 23:58:37 UTC
I filed the duplicate bug #498258
I just wanted to note that I can not reproduce the issue as described on this bug.  I didn't follow exactly, but I use Meta+Space to switch windows, and switching-back to youtube in firefox does not pause the video.

I do see it only with Electron apps (VS Code, Typora), and that seems significant.

I'm ignorant about KWin development... but looking at the bisect commit, it seems like this is an approach type bug, rather than an oopsie type bug.
Comment 10 Bug Janitor Service 2025-01-15 11:43:25 UTC
A possibly relevant merge request was started @ https://invent.kde.org/plasma/kwin/-/merge_requests/7010
Comment 11 Vlad Zahorodnii 2025-01-16 15:26:53 UTC
Git commit 3eb29f1849c74bd59f0778f14aa58efc7ec24c99 by Vlad Zahorodnii.
Committed on 16/01/2025 at 15:06.
Pushed by vladz into branch 'master'.

Prevent including global shortcut keys in wl_keyboard.enter

78bc268876343a313fcb2e7346985371e030ee71 moved pressed key tracking to
a higher abstraction layer. It made pressed keys tracking less error
prone but it also uncovered that the old code used not to update pressed
keys in certain situations, e.g. when pressing a shortcut, but it's
possible that there are more cases.

With global shortcuts, the following can occur

- press Meta
- press Space
- focused keyboard surface changes, it has "Space" in the enter event
- release Space, it will be sent to the new focused surface
- release Meta

Depending on the implementation details, the client can register the
Space key getting pressed and released.

Another event sequence that can cause unexpected behavior is

- press q
- the client sees it and closes its surface
- focused keyboard surface changes, it has "q" in the enter event
- release q

The root cause is the lack of key stroke ownership. It's not expressed
in any possible way. We need to do it in long term, as a more short
term solution, this change adds filtered keys property. If a particular
event filter thinks that the clients should not see particular key, they
just add that key to that list. After the key is released, it will be
removed from the list.

M  +1    -0    autotests/integration/CMakeLists.txt
A  +303  -0    autotests/integration/keyboard_input_test.cpp     [License: GPL(v2.0+)]
M  +11   -0    autotests/integration/kwin_wayland_test.h
M  +62   -16   autotests/integration/test_helpers.cpp
M  +4    -1    src/input.cpp
M  +26   -1    src/keyboard_input.cpp
M  +12   -0    src/keyboard_input.h
M  +1    -1    src/popup_input_filter.cpp

https://invent.kde.org/plasma/kwin/-/commit/3eb29f1849c74bd59f0778f14aa58efc7ec24c99
Comment 12 Vlad Zahorodnii 2025-01-16 15:42:06 UTC
Git commit af643f84b57092647e8afdbc86ccdfa898a83850 by Vlad Zahorodnii.
Committed on 16/01/2025 at 15:29.
Pushed by vladz into branch 'Plasma/6.3'.

Prevent including global shortcut keys in wl_keyboard.enter

78bc268876343a313fcb2e7346985371e030ee71 moved pressed key tracking to
a higher abstraction layer. It made pressed keys tracking less error
prone but it also uncovered that the old code used not to update pressed
keys in certain situations, e.g. when pressing a shortcut, but it's
possible that there are more cases.

With global shortcuts, the following can occur

- press Meta
- press Space
- focused keyboard surface changes, it has "Space" in the enter event
- release Space, it will be sent to the new focused surface
- release Meta

Depending on the implementation details, the client can register the
Space key getting pressed and released.

Another event sequence that can cause unexpected behavior is

- press q
- the client sees it and closes its surface
- focused keyboard surface changes, it has "q" in the enter event
- release q

The root cause is the lack of key stroke ownership. It's not expressed
in any possible way. We need to do it in long term, as a more short
term solution, this change adds filtered keys property. If a particular
event filter thinks that the clients should not see particular key, they
just add that key to that list. After the key is released, it will be
removed from the list.


(cherry picked from commit 3eb29f1849c74bd59f0778f14aa58efc7ec24c99)

Co-authored-by: Vlad Zahorodnii <vlad.zahorodnii@kde.org>

M  +1    -0    autotests/integration/CMakeLists.txt
A  +303  -0    autotests/integration/keyboard_input_test.cpp     [License: GPL(v2.0+)]
M  +11   -0    autotests/integration/kwin_wayland_test.h
M  +62   -16   autotests/integration/test_helpers.cpp
M  +4    -1    src/input.cpp
M  +26   -1    src/keyboard_input.cpp
M  +12   -0    src/keyboard_input.h
M  +1    -1    src/popup_input_filter.cpp

https://invent.kde.org/plasma/kwin/-/commit/af643f84b57092647e8afdbc86ccdfa898a83850