Bug 407395 - [Wayland] Cannot assign to anything a keyboard shortcut that's already in use
Summary: [Wayland] Cannot assign to anything a keyboard shortcut that's already in use
Status: RESOLVED FIXED
Alias: None
Product: plasmashell
Classification: Plasma
Component: generic-wayland (show other bugs)
Version: 5.19.90
Platform: Arch Linux Linux
: NOR normal
Target Milestone: 1.0
Assignee: Plasma Bugs List
URL:
Keywords: wayland
: 427740 (view as bug list)
Depends on:
Blocks:
 
Reported: 2019-05-10 15:34 UTC by Patrick Silva
Modified: 2020-11-30 12:03 UTC (History)
4 users (show)

See Also:
Latest Commit:
Version Fixed In: 5.77


Attachments
screenshot (78.21 KB, image/png)
2020-04-01 18:53 UTC, Patrick Silva
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Patrick Silva 2019-05-10 15:34:07 UTC
SUMMARY
Currently meta+d triggers "Show desktop" widget on neon dev unstable,
but I want to set the same shortcut to "Minimize all windows" instead.
However I can't do it because when I press meta+d in the settings window
of "Minimize all windows" widget, plasma triggers "Show desktop" instead of
assign such shortcut to "Minimize all windows".
In the same way, when I try to assign meta+l, my screen is locked, etc.
The same problem affects widgets in the systray like networks (plasma-nm)
and sound (plasma-pa).

Operating System: KDE neon Unstable Edition
KDE Plasma Version: 5.15.80
KDE Frameworks Version: 5.58.0
Qt Version: 5.12.0
Comment 1 Nate Graham 2019-05-13 04:24:57 UTC
Does this happen only for shortcuts that use the Meta key? Or any shortcut using any combination of keys?
Comment 2 Nate Graham 2019-05-13 04:27:00 UTC
Also does it work if you first unset the shortcut from what's currently using it?
Comment 3 Patrick Silva 2019-05-13 08:37:21 UTC
(In reply to Nate Graham from comment #1)
> Does this happen only for shortcuts that use the Meta key? Or any shortcut
> using any combination of keys?

I also can't assign to widgets ctrl+esc, alt+space, alt+shift+f12 and ctrl+alt+k, for example.

(In reply to Nate Graham from comment #2)
> Also does it work if you first unset the shortcut from what's currently
> using it?
Yes, it works.
Comment 4 Patrick Silva 2020-04-01 18:53:27 UTC
Created attachment 127171 [details]
screenshot

No longer reproducible. As we can see in the attached screenshot, currently Plasma just shows a warning saying that the shortcut is already registered by another app.

Operating System: Arch Linux 
KDE Plasma Version: 5.18.4
KDE Frameworks Version: 5.68.0
Qt Version: 5.14.2
Comment 5 Patrick Silva 2020-04-10 13:42:11 UTC
ops, this issue persists only on Wayland session. :(

Operating System: Arch Linux 
KDE Plasma Version: 5.18.4
KDE Frameworks Version: 5.68.0
Qt Version: 5.14.2
Comment 6 Patrick Silva 2020-06-16 23:22:25 UTC
This bug does not only affect widgets.
On Wayland I can't, using the Global Shortcuts kcm, assign to Kate the shortcut meta+e assigned to Dolphin by default, for example.

Operating System: KDE neon Unstable Edition
KDE Plasma Version: 5.19.80
KDE Frameworks Version: 5.72.0
Qt Version: 5.14.2
Comment 7 Patrick Silva 2020-10-15 18:03:25 UTC
*** Bug 427740 has been marked as a duplicate of this bug. ***
Comment 8 David Redondo 2020-10-16 07:07:53 UTC
It's because it is currently not possible to grab the keyboard on wayland
Comment 9 phrxmd 2020-10-16 10:47:26 UTC
That would mean that effectively under Wayland it will for the foreseeable future be impossible to assign keyboard shortcuts in a general way, as long as we try to do it by grabbing the keyboard.

Maybe there could be another solution to getting the key. For example the global shortcut KCM could tell KWin that now we're expecting a keypress, so please report back the next keypress instead of executing the regular action associated with it.
Comment 10 Bug Janitor Service 2020-10-23 10:39:42 UTC
A possibly relevant merge request was started @ https://invent.kde.org/frameworks/kdeclarative/-/merge_requests/27
Comment 11 David Redondo 2020-11-12 15:52:57 UTC
Git commit dca78412606f1ea0e66bcafe14dbd2ebf9836674 by David Redondo.
Committed on 12/11/2020 at 15:52.
Pushed by davidre into branch 'master'.

Add KeySequenceRecorder as base for KKeySequenceWidget and KeySequenceItem

Currently we have two copies of the shortcut recording logic: One in
KKeySequenceWidget and the other one in the helper of KeySequenceItem. They are
mostly straight copies but there are slight differences. This situation is
naturally prone to divergence between both. If a bug is fixed in one, the fix
needs to be ported to the other one. If a need feature is added it has to be
added to both.
Facing this situation when wanting to add support for the shortcut inhibit
protocol on Wayland, I decided to abstact the recording logic out and make both
use it, instead of adding more duplicated code.
On X we can grab the keyboard while recording to prevent shortcuts from being
triggered but we can't do that on Wayland and the Compositor will trigger
shortcuts when the user actually wants to reassign them. Depending on the platform,
this new class will either grab the keyboard or inhibit shortcuts
on the window that is passsed to it. The recording logic is copied out of
KKeySequenceWidget and stays the same but is cleaned up a bit inside the methods
for in my opinion better readability.

M  +13   -2    CMakeLists.txt
M  +1    -0    autotests/CMakeLists.txt
A  +168  -0    autotests/keysequencerecordertest.cpp     [License: LGPL(3+eV) LGPL(v3.0) LGPL(v2.1)]
A  +27   -0    autotests/keysequencerecordertest.h     [License: LGPL(3+eV) LGPL(v3.0) LGPL(v2.1)]
M  +22   -0    src/CMakeLists.txt
A  +143  -0    src/recorder/keyboard-shortcuts-inhibit-unstable-v1.xml
A  +42   -0    src/recorder/keyboardgrabber.cpp     [License: LGPL(3+eV) LGPL(v3.0) LGPL(v2.1)]
A  +24   -0    src/recorder/keyboardgrabber_p.h     [License: LGPL(3+eV) LGPL(v3.0) LGPL(v2.1)]
A  +491  -0    src/recorder/keysequencerecorder.cpp     [License: LGPL(v2.0+)]
A  +132  -0    src/recorder/keysequencerecorder.h     [License: LGPL(v2.0+)]
A  +19   -0    src/recorder/shortcutinhibition_p.h     [License: LGPL(3+eV) LGPL(v3.0) LGPL(v2.1)]
A  +99   -0    src/recorder/waylandinhibition.cpp     [License: LGPL(3+eV) LGPL(v3.0) LGPL(v2.1)]
A  +31   -0    src/recorder/waylandinhibition_p.h     [License: LGPL(3+eV) LGPL(v3.0) LGPL(v2.1)]

https://invent.kde.org/frameworks/kguiaddons/commit/dca78412606f1ea0e66bcafe14dbd2ebf9836674
Comment 12 David Redondo 2020-11-12 16:28:01 UTC
Git commit 28a80aba1463b83ea8845ff22566a2110c8fb041 by David Redondo.
Committed on 12/11/2020 at 16:26.
Pushed by davidre into branch 'master'.

Rewrite KKeySequenceWidget to use KeySequenceRecorder

Use the new KeySequenceRecorder which was created for this purpose. This
has the benefit of simplifying the logic. While at it I sorted functions
and unified method naming which additionally helps in following what this
class does.

M  +1    -0    CMakeLists.txt
M  +1    -0    src/CMakeLists.txt
M  +324  -626  src/kkeysequencewidget.cpp
D  +0    -38   src/kkeysequencewidget_p.h

https://invent.kde.org/frameworks/kxmlgui/commit/28a80aba1463b83ea8845ff22566a2110c8fb041
Comment 13 David Redondo 2020-11-12 16:36:19 UTC
Git commit 63d4a2a0f29d0059f1076775cbf5b22b1ef7c29d by David Redondo.
Committed on 12/11/2020 at 16:32.
Pushed by davidre into branch 'master'.

Rewrite KeySequenceItem (and helper) to use KeySequenceRecorder

Use the new KeySequenceRecorder which was created for this purpose. We can
remove almost all logic from the helper aside from the conflict checking.
Related: bug 427730

M  +26   -24   src/qmlcontrols/kquickcontrols/KeySequenceItem.qml
M  +1    -0    src/qmlcontrols/kquickcontrols/private/CMakeLists.txt
M  +26   -388  src/qmlcontrols/kquickcontrols/private/keysequencehelper.cpp
M  +14   -89   src/qmlcontrols/kquickcontrols/private/keysequencehelper.h

https://invent.kde.org/frameworks/kdeclarative/commit/63d4a2a0f29d0059f1076775cbf5b22b1ef7c29d
Comment 14 Patrick Silva 2020-11-21 11:21:42 UTC
This issue is still reproducible when I try to assign a shortcut already in use
via Shortcuts KCM.

Operating System: KDE neon Unstable Edition
KDE Plasma Version: 5.20.80
KDE Frameworks Version: 5.77.0
Qt Version: 5.15.1
Comment 15 Patrick Silva 2020-11-22 14:23:29 UTC
(In reply to Patrick Silva from comment #14)
> This issue is still reproducible when I try to assign a shortcut already in
> use via Shortcuts KCM.

For example, my screen is locked when I try to assign meta+L (default shortcut to lock the screen) to Dolphin.
Comment 16 Bug Janitor Service 2020-11-23 13:43:11 UTC
A possibly relevant merge request was started @ https://invent.kde.org/frameworks/kguiaddons/-/merge_requests/8
Comment 17 David Redondo 2020-11-30 12:03:53 UTC
Git commit fc699b4c5c7ad622fa11f4f09098f070d2a49391 by David Redondo.
Committed on 30/11/2020 at 12:02.
Pushed by davidre into branch 'master'.

Make shortcut inhibition work from the get-go

Call the private slot directly instead of waiting for the queued call.

M  +3    -0    src/recorder/waylandinhibition.cpp

https://invent.kde.org/frameworks/kguiaddons/commit/fc699b4c5c7ad622fa11f4f09098f070d2a49391