WIP: FEAT(client): Add support to XDG Desktop Portal GlobalShortcuts - #5976
WIP: FEAT(client): Add support to XDG Desktop Portal GlobalShortcuts#5976aleixpol wants to merge 1 commit into
Conversation
e9f84da to
c1fd93f
Compare
Krzmbrzl
left a comment
There was a problem hiding this comment.
Why is the implementation using the name "xdp" rather than "xdg"?
And I'm wondering: Given that this is such a new feature, do we want to keep the old wayland notice in-place and show, if on wayland and if the new impl is not available?
| qt_add_dbus_interface(mumble_xdp_SRCS org.freedesktop.portal.GlobalShortcuts.xml globalshortcuts_portal_interface) | ||
| find_file(PORTALSREQUEST_XML share/dbus-1/interfaces/org.freedesktop.portal.Request.xml PATH_SUFFIXES share PATHS /usr ${CMAKE_INSTALL_PREFIX}) | ||
| qt_add_dbus_interface(mumble_xdp_SRCS ${PORTALSREQUEST_XML} portalsrequest_interface) | ||
| target_sources(mumble_client_object_lib PRIVATE ${mumble_xdp_SRCS}) |
There was a problem hiding this comment.
I feel like this should be outsourced into a dedicated cmake function implemented in a file inside the cmake directory. Then this place here could simply call that function.
Plus, it seems that we currently don't handle the case when the searched for file can't be found.
There was a problem hiding this comment.
I moved both files to auxiliary_files.
I am not sure what you mean by adding the function. Do you still want it if we are not looking it up?
|
And out of curiosity: Will this be a flatpak-specific thing or will this (likely) solve the issue for Wayland users in general (also for non-flatpak apps)? |
e00c411 to
a59e8f3
Compare
Yes, this should solve it for every Wayland system. Also it can work on X11 provided it's properly implemented in the backend like we are doing in KDE/Plasma and I expect others will too. |
Krzmbrzl
left a comment
There was a problem hiding this comment.
Sorry for the long delay since my last review - I have been rather busy
| #ifdef Q_OS_LINUX | ||
| if (EnvUtils::waylandIsUsed()) { | ||
| // Due to the issues we're currently having on Wayland, we disable shortcuts by default | ||
| bShortcutEnable = false; | ||
| } | ||
| #endif |
There was a problem hiding this comment.
This should probably get the same treatment as the note in the settings UI
| // TODO | ||
| return {}; |
There was a problem hiding this comment.
Just marking this so we don't lose track of this TODO
There was a problem hiding this comment.
So to address this TODO we'll need to use an interface like this:
https://invent.kde.org/libraries/xdg-portal-test-kde/-/merge_requests/18
This means depending on Qt::GuiPrivate and libwaylandclient. If you are interested in this I can include it in this PR.
There was a problem hiding this comment.
Hm. This seems rather hacky. What are the implications of simply leaving the implementation as it is? Aka: what exactly is the parent window ID being used for?
Will this perhaps only become relevant when dealing with multiple Mumble instances?
There was a problem hiding this comment.
The parent window is simply to inform the portal who is calling this and which window the dialog should be on top.
The dialog will appear on top and focussed anyway, so it shouldn't be a big deal. FWIW, there will be public API for this in Qt 6 (6.5, if I'm not mistaken). If you don't want to depend on private API, just delaying until Qt 6 is a good option.
There was a problem hiding this comment.
Okay in that case it indeed seems a good idea to simply delay this 👍
6fbe414 to
62bb6d9
Compare
This makes it possible to have global shortcuts on systems running the XDG Desktop Portal service. This is especially relevant on Wayland where we are not able to run a system-wide keylogger to get the global shortcuts triggers. Fixes mumble-voip#5257
62bb6d9 to
0e0f075
Compare
|
Two awkward things I've noticed which may or may not be related to the implementation in this PR:
|
Correct, we don't have global shortcuts triggered my mouse in Plasma. It should be supported by Plasma or the Desktop Environment of choice of the user anyway.
Again, this is Plasma deciding that they are not the same shortcut and not triggering it. There's not much we can (or should) do from this side. I suggest we continue this discussion in the bug report you created: https://bugs.kde.org/show_bug.cgi?id=465867 I suggest explaining a bit more on the bug report why it's important for anymodifier to trigger the subject's shortcut. If we have clear use cases it will be easier to get it acted on. |
|
What's the status of this PR? Can't wait to enjoy global shortcuts in Mumble using KDE Wayland. |
|
Has any more progress been made on this? |
|
Can this still be moved forward or salvaged? The most popular Wayland compositors now support the GlobalShortcuts XDG desktop portal, many distros default to Wayland, and xorg is no longer shipping by default in some distros. One of the decisions from the code reviews was to wait for Qt 6. Mumble is now on Qt 6: #6516 Without intimate familiarity, it looks like this could move forward with relatively small changes. Is that accurate? Do we need volunteers to help move this forward? Thanks! |
|
I know I'm going to sound like, "that guy", however this has been the only thing keeping me on X11 for the past 2-3 years. I'd be great if it could finally be resolved as Wayland would fix several other bugs and shortcomings that I'm facing from unrelated applications. |
|
You're definitely not the only one in that boat. Though I'm using a workaround that at least lets me use Wayland for everything else. This is, of course, not an option for every DE, but I'm using Wayland KDE - what I do, is force mumble to use XWayland (just unsetting WAYLAND_DISPLAY in the startup shortcut), and then I enable the legacy X11 keyboard shortcuts in the KDE settings. I personally don't feel uncomfortable with the idea that one window could see the keystrokes of another (just watch this end up biting me in the future), but at the very least this lets me not be completely stuck with X11 at a top level. Of course, I would still much prefer this gets implemented properly for Wayland. |
Thanks, I wasn't aware of that option! I know there's workarounds for things like muting/deafening like Does this workaround work even when system focus is given to Wayland applications? |
|
It sure does! This is built into KDE, and since KDE is acting as the compositor, it has effectively raw access to the keyboard. |
|
Is there any update on this? KDE have announced they are moving towards a wayland only future, as is gnome and the whole linux desktop ecosystem. Currently the janky KDE x11 passthrough works for hotkeys whilst using an xwayland application, but wayland proton is also right around the corner. It's going to become very frustrating not have a PTT solution for mumble in the coming months if this doesn't get implemented soon. |
|
No updates. If there were any, they would be visible here. |
|
Well, I do have an update: turns out the Desktop Portal GlobalShortcuts as it stands right now is not great. The idea itself is nice because hotkeys/shortcuts for applications should indeed be managed from a single place that is runs on the system, for both consistency and security. However, the implementation is disappointing:
Since we need a unified interface for other operating systems such as Windows and macOS anyway, I'm working on a service with a GUI configurator. Still in very early development, but the overall idea is there. |
|
This would be a really nice to have. Recently started using mumble again and this is really the only pain point. |
Note that the portal is designed to allow backends to bind any event from any input device, be it device class or specific device, distinguish between different variants of the same modifier, etc. What currently is relatively limiting is only the hint the application passes about the preferred binding, but whether there are more advanced binding actually used is up to the portal backend. |
|
I personally feel that the global shortcuts portal is suitable for solving this, but that GNOME, Plasma, and other desktops need to improve their implementations. Implementing the portal in Mumble can show to the big desktops the use case for supporting things on their end that they currently don't. |
Could you elaborate more on that, please? |
The passed binding preference is only a hint, and to just start somewhere, we defined https://specifications.freedesktop.org/shortcuts/latest/ for the basic keyboard shortcut hints. But the backend is free to let the user bind any action it prefers, be it mouse/keyboard/pedals/butter fly flap/.... The portal API is designed to let backends dynamically grow capabilities independently of what can actually be specified using some spec. In other words, "I (user) want to bind this type of input action" feature requests can be routed to portal backends, and "I (developer) want to describe this hint" will typically go the route of first creating a specification how to describe such a hint, then amending the portal to allow passing such a hint. |
|
Neat! Could you also point out which portal backend(s) can/should be enhanced? |
The ones I know of that appear to implement it are xdg-desktop-portal-kde, xdg-desktop-portal-gnome and xdg-desktop-portal-hyprland. |
This makes it possible to have global shortcuts on systems running the XDG Desktop Portal service. This is especially relevant on Wayland where we are not able to run a system-wide keylogger to get the global shortcuts triggers.
Fixes #5257
Checks
WIP because I'm not very familiar because this is a codebase alien to me and I'm aware it's doing some nasty things. Still, I'd prefer to know how the maintainers want to do it rather than imagining myself what they want instead.
Note the GlobalShortcuts portal is merged to master but it still isn't released.