From 98eb336ab09d12ef556987b54089aa8fb166c11e Mon Sep 17 00:00:00 2001 From: CJ Pais Date: Sat, 2 Aug 2025 18:43:59 -0700 Subject: [PATCH 1/5] disallow invalid bindings from frontend --- src-tauri/src/lib.rs | 2 + src-tauri/src/shortcut.rs | 51 +++++++++++++++++++++++ src/components/settings/HandyShortcut.tsx | 29 ++++++++++++- 3 files changed, 81 insertions(+), 1 deletion(-) diff --git a/src-tauri/src/lib.rs b/src-tauri/src/lib.rs index 59ecc47..68b5b52 100644 --- a/src-tauri/src/lib.rs +++ b/src-tauri/src/lib.rs @@ -183,6 +183,8 @@ pub fn run() { shortcut::change_selected_language_setting, shortcut::change_show_overlay_setting, shortcut::change_debug_mode_setting, + shortcut::suspend_binding, + shortcut::resume_binding, trigger_update_check, commands::cancel_operation, commands::get_app_dir_path, diff --git a/src-tauri/src/shortcut.rs b/src-tauri/src/shortcut.rs index d2d609b..cae54f3 100644 --- a/src-tauri/src/shortcut.rs +++ b/src-tauri/src/shortcut.rs @@ -56,6 +56,9 @@ pub fn change_binding( }); } + // Validate the new shortcut before we touch the current registration + validate_shortcut_string(&binding)?; + // Create an updated binding let mut updated_binding = binding_to_modify; updated_binding.current_binding = binding; @@ -143,13 +146,61 @@ pub fn change_debug_mode_setting(app: AppHandle, enabled: bool) -> Result<(), St Ok(()) } +/// Determine whether a shortcut string contains at least one non-modifier key. +/// We allow single non-modifier keys (e.g. "f5" or "space") but disallow +/// modifier-only combos (e.g. "ctrl" or "ctrl+shift"). +fn validate_shortcut_string(raw: &str) -> Result<(), String> { + let modifiers = [ + "ctrl", "control", "shift", "alt", "option", "meta", "command", "cmd", "super", "win", + "windows", + ]; + let has_non_modifier = raw + .split('+') + .any(|part| !modifiers.contains(&part.trim().to_lowercase().as_str())); + if has_non_modifier { + Ok(()) + } else { + Err("Shortcut must contain at least one non-modifier key".into()) + } +} + +/// Temporarily unregister a binding while the user is editing it in the UI. +/// This avoids firing the action while keys are being recorded. +#[tauri::command] +pub fn suspend_binding(app: AppHandle, id: String) -> Result<(), String> { + if let Some(b) = settings::get_bindings(&app).get(&id).cloned() { + _unregister_shortcut(&app, b)?; + } + Ok(()) +} + +/// Re-register the binding after the user has finished editing. +#[tauri::command] +pub fn resume_binding(app: AppHandle, id: String) -> Result<(), String> { + if let Some(b) = settings::get_bindings(&app).get(&id).cloned() { + _register_shortcut(&app, b)?; + } + Ok(()) +} + fn _register_shortcut(app: &AppHandle, binding: ShortcutBinding) -> Result<(), String> { + // Validate human-level rules first + validate_shortcut_string(&binding.current_binding)?; + // Parse shortcut and return error if it fails let shortcut = match binding.current_binding.parse::() { Ok(s) => s, Err(e) => return Err(format!("Failed to parse shortcut: {}", e)), }; + // Prevent duplicate registrations that would silently shadow one another + if app.global_shortcut().is_registered(shortcut) { + return Err(format!( + "Shortcut '{}' is already in use", + binding.current_binding + )); + } + // Clone binding.id for use in the closure let binding_id_for_closure = binding.id.clone(); diff --git a/src/components/settings/HandyShortcut.tsx b/src/components/settings/HandyShortcut.tsx index 6ea5459..2819a30 100644 --- a/src/components/settings/HandyShortcut.tsx +++ b/src/components/settings/HandyShortcut.tsx @@ -5,6 +5,7 @@ import { getKeyName } from "../../lib/utils/keyboard"; import ResetIcon from "../icons/ResetIcon"; import { SettingContainer } from "../ui/SettingContainer"; import { useSettings } from "../../hooks/useSettings"; +import { invoke } from "@tauri-apps/api/core"; interface HandyShortcutProps { descriptionMode?: "inline" | "tooltip"; @@ -82,6 +83,20 @@ export const HandyShortcut: React.FC = ({ // Keyboard event listeners const handleKeyDown = async (e: KeyboardEvent) => { + if (e.repeat) return; // ignore auto-repeat + if (e.key === "Escape") { + // Cancel recording + if (editingShortcutId) { + await invoke("resume_binding", { id: editingShortcutId }).catch( + console.error, + ); + } + setEditingShortcutId(null); + setKeyPressed([]); + setRecordedKeys([]); + setOriginalBinding(""); + return; + } e.preventDefault(); // Get the key and normalize it (unify left/right modifiers) @@ -118,6 +133,10 @@ export const HandyShortcut: React.FC = ({ if (editingShortcutId && bindings[editingShortcutId]) { try { await updateBinding(editingShortcutId, newShortcut); + // Re-register the shortcut now that recording is finished + await invoke("resume_binding", { id: editingShortcutId }).catch( + console.error, + ); } catch (error) { console.error("Failed to change binding:", error); } @@ -136,6 +155,11 @@ export const HandyShortcut: React.FC = ({ const activeElement = shortcutRefs.current.get(editingShortcutId); if (activeElement && !activeElement.contains(e.target as Node)) { // Cancel shortcut recording - the hook will handle rollback + if (editingShortcutId) { + invoke("resume_binding", { id: editingShortcutId }).catch( + console.error, + ); + } setEditingShortcutId(null); setKeyPressed([]); setRecordedKeys([]); @@ -162,9 +186,12 @@ export const HandyShortcut: React.FC = ({ ]); // Start recording a new shortcut - const startRecording = (id: string) => { + const startRecording = async (id: string) => { if (editingShortcutId === id) return; // Already editing this shortcut + // Suspend current binding to avoid firing while recording + await invoke("suspend_binding", { id }).catch(console.error); + // Store the original binding to restore if canceled setOriginalBinding(bindings[id]?.current_binding || ""); setEditingShortcutId(id); From 32a630cc5272a03bb49dcfe993f5315f6d594322 Mon Sep 17 00:00:00 2001 From: CJ Pais Date: Sat, 2 Aug 2025 19:03:56 -0700 Subject: [PATCH 2/5] potentially fix cross-platform keybinding handling Naming at the very least. --- src/components/settings/HandyShortcut.tsx | 89 ++++++++++------------ src/lib/utils/keyboard.ts | 91 ++++++++++++++++++----- 2 files changed, 110 insertions(+), 70 deletions(-) diff --git a/src/components/settings/HandyShortcut.tsx b/src/components/settings/HandyShortcut.tsx index 2819a30..b4ebd96 100644 --- a/src/components/settings/HandyShortcut.tsx +++ b/src/components/settings/HandyShortcut.tsx @@ -1,7 +1,12 @@ import React, { useEffect, useState, useRef } from "react"; import { BindingResponseSchema, ShortcutBindingsMap } from "../../lib/types"; import { type } from "@tauri-apps/plugin-os"; -import { getKeyName } from "../../lib/utils/keyboard"; +import { + getKeyName, + formatKeyCombination, + normalizeKey, + type OSType, +} from "../../lib/utils/keyboard"; import ResetIcon from "../icons/ResetIcon"; import { SettingContainer } from "../ui/SettingContainer"; import { useSettings } from "../../hooks/useSettings"; @@ -24,57 +29,42 @@ export const HandyShortcut: React.FC = ({ null, ); const [originalBinding, setOriginalBinding] = useState(""); - const [isMacOS, setIsMacOS] = useState(false); + const [osType, setOsType] = useState("unknown"); const shortcutRefs = useRef>(new Map()); const bindings = getSetting("bindings") || {}; - // Check if running on macOS + // Detect and store OS type useEffect(() => { - const checkOsType = async () => { + const detectOsType = async () => { try { - const osType = await type(); - setIsMacOS(osType === "macos"); + const detectedType = await type(); + let normalizedType: OSType; + + switch (detectedType) { + case "macos": + normalizedType = "macos"; + break; + case "windows": + normalizedType = "windows"; + break; + case "linux": + normalizedType = "linux"; + break; + default: + normalizedType = "unknown"; + } + + setOsType(normalizedType); } catch (error) { console.error("Error detecting OS type:", error); - setIsMacOS(false); + setOsType("unknown"); } }; - checkOsType(); + detectOsType(); }, []); - // Normalize modifier keys (unify left/right variants) - const normalizeKey = (key: string): string => { - // Handle left/right variants of modifier keys - if (key.startsWith("left ") || key.startsWith("right ")) { - const parts = key.split(" "); - if (parts.length === 2) { - // Return just the modifier name without left/right prefix - return parts[1]; - } - } - return key; - }; - - // Format keys for macOS display - const formatMacOSKeys = (key: string): string => { - if (!isMacOS) return key; // Only format for macOS - - const keyMap: Record = { - alt: "option", - }; - - return keyMap[key.toLowerCase()] || key; - }; - - // Format a key combination for display - const formatKeyCombination = (combination: string): string => { - if (!isMacOS) return combination; // Only format for macOS - - return combination.split("+").map(formatMacOSKeys).join(" + "); - }; - useEffect(() => { // Only add event listeners when we're in editing mode if (editingShortcutId === null) return; @@ -99,8 +89,8 @@ export const HandyShortcut: React.FC = ({ } e.preventDefault(); - // Get the key and normalize it (unify left/right modifiers) - const rawKey = getKeyName(e); + // Get the key with OS-specific naming and normalize it + const rawKey = getKeyName(e, osType); const key = normalizeKey(rawKey); console.log("You pressed", rawKey, "normalized to", key); @@ -117,8 +107,8 @@ export const HandyShortcut: React.FC = ({ const handleKeyUp = async (e: KeyboardEvent) => { e.preventDefault(); - // Get the key and normalize it - const rawKey = getKeyName(e); + // Get the key with OS-specific naming and normalize it + const rawKey = getKeyName(e, osType); const key = normalizeKey(rawKey); // Remove from currently pressed keys @@ -183,6 +173,7 @@ export const HandyShortcut: React.FC = ({ bindings, originalBinding, updateBinding, + osType, ]); // Start recording a new shortcut @@ -200,15 +191,11 @@ export const HandyShortcut: React.FC = ({ }; // Format the current shortcut keys being recorded - const formatCurrentKeys = () => { + const formatCurrentKeys = (): string => { if (recordedKeys.length === 0) return "Press keys..."; - if (!isMacOS) { - return recordedKeys.join("+"); - } - - // Map each key to its macOS-friendly name for display - return recordedKeys.map(formatMacOSKeys).join(" + "); + // Use the same formatting as the display to ensure consistency + return formatKeyCombination(recordedKeys.join("+"), osType); }; // Store references to shortcut elements @@ -275,7 +262,7 @@ export const HandyShortcut: React.FC = ({ className="px-2 py-1 text-sm font-semibold bg-mid-gray/10 border border-mid-gray/80 hover:bg-logo-primary/10 rounded cursor-pointer hover:border-logo-primary" onClick={() => startRecording(primaryId)} > - {formatKeyCombination(primaryBinding.current_binding)} + {formatKeyCombination(primaryBinding.current_binding, osType)} )}