Add KDE Wayland native hotkey support and fix clipboard paste#486
Conversation
gabrielste1n
left a comment
gabrielste1n
left a comment
There was a problem hiding this comment.
Nice work, this follows the existing GNOME/Hyprland shortcut pattern well. Left a few comments, mostly around cleanup paths and one potential silent failure.
| for (const slotName of this.registeredSlots) { | ||
| const actionId = [COMPONENT_NAME, "OpenWhispr", slotName, `OpenWhispr ${slotName}`]; | ||
| try { | ||
| this.kglobalaccel?.unRegister(actionId); |
There was a problem hiding this comment.
unRegister() is async (D-Bus call) but it's called synchronously here without awaiting, and then bus.disconnect() runs right after the loop. The unregister calls will probably never actually complete since the bus gets torn down immediately.
If this is intentional for app shutdown and you're ok with best-effort cleanup, a comment would help. Otherwise you'd want to await these or at least add a small delay before disconnecting.
| // Register action then set shortcut | ||
| await this.kglobalaccel.doRegister(actionId); | ||
| const result = await this.kglobalaccel.setShortcut(actionId, [qtKey], 0x02); | ||
|
|
There was a problem hiding this comment.
If _listenForComponent() fails internally, it catches the error and just logs it. But registerKeybinding() still returns true here. So you'd end up with a keybinding that's registered in KDE but the app never receives press events because the component listener didn't get set up.
Might want to either return a boolean from _listenForComponent and check it here, or let the error propagate up.
|
|
||
| iface.on("globalShortcutPressed", (componentUnique, shortcutUnique, timestamp) => { | ||
| debugLogger.log("[KDEShortcut] Shortcut pressed", { componentUnique, shortcutUnique }); | ||
| // KGlobalAccel may send actionUnique or actionFriendly depending on version |
There was a problem hiding this comment.
This comment and the _findCallbackByFriendlyName fallback suggest we're not totally sure what KGlobalAccel sends as shortcutUnique. Have you tested this on a real KDE system? Would be good to nail down which value we're actually getting so we're not relying on the fallback in production.
| debugLogger.log( | ||
| "[HotkeyManager] KDE keybinding failed, falling back to globalShortcut" | ||
| ); | ||
| this.useKDE = false; |
There was a problem hiding this comment.
Both here and in the catch block below, when KDE registration fails you set useKDE = false and fall back to globalShortcut, but kdeManager is still alive. Should probably also do this.kdeManager.close() and this.kdeManager = null before falling back, otherwise you have a half-initialized manager hanging around.
| return { | ||
| isUsingGnome: this.windowManager.isUsingGnomeHotkeys(), | ||
| isUsingHyprland: this.windowManager.isUsingHyprlandHotkeys(), | ||
| isUsingKDE: this.windowManager.isUsingKDEHotkeys?.() || false, |
There was a problem hiding this comment.
The ?. is inconsistent with the lines right above it. isUsingGnomeHotkeys() and isUsingHyprlandHotkeys() don't use optional chaining. Since isUsingKDEHotkeys was added to WindowManager in this same PR, it'll always exist.
| const { getYdotoolStatus } = require("./ensureYdotool"); | ||
| return getYdotoolStatus(); | ||
| const status = getYdotoolStatus(); | ||
| const { execFileSync } = require("child_process"); |
There was a problem hiding this comment.
Nit: this require is inside the IPC handler so it runs on every get-ydotool-status call. Would be cleaner to hoist it to the top of the file with the other requires.
Revert all hotkeyManager.js changes — PR #486's KDE D-Bus as primary path is confirmed working. Only keep windowConfig.js fixes (overlay window type and always-on-top for KDE Wayland) and main.js comment.
…native-shortcuts Add KDE Wayland native hotkey support and fix clipboard paste
Revert all hotkeyManager.js changes — PR OpenWhispr#486's KDE D-Bus as primary path is confirmed working. Only keep windowConfig.js fixes (overlay window type and always-on-top for KDE Wayland) and main.js comment.
2 participants
globalShortcut.register() doesn't work on KDE native Wayland, the compositor blocks global input grabs. #470 forced XWayland as a workaround but broke clipboard paste (X11/Wayland clipboard desync).
Adds kdeShortcut.js that registers hotkeys through KGlobalAccel D-Bus and listens for globalShortcutPressed. Same approach as gnomeShortcut.js and hyprlandShortcut.js. KDE still runs on XWayland for clipboard, hotkeys go through KDE's shortcut system.
Clipboard writes on KDE use xclip/xsel instead of wl-copy. Both added as recommended deps in deb/rpm. Diagnostics check in Settings for KDE users missing xclip.
GNOME and Hyprland paths unchanged.
Supersedes #470