diff --git a/apps/desktop/desktop_native/autotype/src/lib.rs b/apps/desktop/desktop_native/autotype/src/lib.rs index 4b9e65180e62..917f0f797b6b 100644 --- a/apps/desktop/desktop_native/autotype/src/lib.rs +++ b/apps/desktop/desktop_native/autotype/src/lib.rs @@ -1,5 +1,11 @@ use anyhow::Result; +#[cfg(target_os = "windows")] +mod modifier_keys; + +#[cfg(target_os = "windows")] +pub(crate) use modifier_keys::*; + #[cfg_attr(target_os = "linux", path = "linux.rs")] #[cfg_attr(target_os = "macos", path = "macos.rs")] #[cfg_attr(target_os = "windows", path = "windows/mod.rs")] diff --git a/apps/desktop/desktop_native/autotype/src/modifier_keys.rs b/apps/desktop/desktop_native/autotype/src/modifier_keys.rs new file mode 100644 index 000000000000..ea9cbf62d374 --- /dev/null +++ b/apps/desktop/desktop_native/autotype/src/modifier_keys.rs @@ -0,0 +1,44 @@ +use std::{collections::HashMap, sync::OnceLock}; + +// Electron modifier keys +// +pub(crate) const CONTROL_KEY_STR: &str = "Control"; +pub(crate) const ALT_KEY_STR: &str = "Alt"; +pub(crate) const SUPER_KEY_STR: &str = "Super"; + +// numeric values for modifier keys +pub(crate) const CONTROL_KEY: u16 = 0x11; +pub(crate) const ALT_KEY: u16 = 0x12; +pub(crate) const SUPER_KEY: u16 = 0x5B; + +/// A mapping of to +static MODIFIER_KEYS: OnceLock> = OnceLock::new(); + +/// Provides a mapping of the valid modifier keys' electron +/// string representation to the numeric representation. +pub(crate) fn get_modifier_keys() -> &'static HashMap<&'static str, u16> { + MODIFIER_KEYS.get_or_init(|| { + HashMap::from([ + (CONTROL_KEY_STR, CONTROL_KEY), + (ALT_KEY_STR, ALT_KEY), + (SUPER_KEY_STR, SUPER_KEY), + ]) + }) +} + +#[cfg(test)] +mod test { + use super::get_modifier_keys; + + #[test] + fn valid_modifier_keys() { + assert_eq!(get_modifier_keys().get("Control").unwrap(), &0x11); + assert_eq!(get_modifier_keys().get("Alt").unwrap(), &0x12); + assert_eq!(get_modifier_keys().get("Super").unwrap(), &0x5B); + } + + #[test] + fn does_not_contain_invalid_modifier_keys() { + assert!(!get_modifier_keys().contains_key("Shift")); + } +} diff --git a/apps/desktop/desktop_native/autotype/src/windows/mod.rs b/apps/desktop/desktop_native/autotype/src/windows/mod.rs index ba9bc4e1ac9f..058fdcbd49ea 100644 --- a/apps/desktop/desktop_native/autotype/src/windows/mod.rs +++ b/apps/desktop/desktop_native/autotype/src/windows/mod.rs @@ -44,7 +44,6 @@ pub fn get_foreground_window_title() -> Result { /// - Control /// - Alt /// - Super -/// - Shift /// - \[a-z\]\[A-Z\] struct KeyboardShortcutInput(INPUT); diff --git a/apps/desktop/desktop_native/autotype/src/windows/type_input.rs b/apps/desktop/desktop_native/autotype/src/windows/type_input.rs index 2ad9af65152c..419babb34fcd 100644 --- a/apps/desktop/desktop_native/autotype/src/windows/type_input.rs +++ b/apps/desktop/desktop_native/autotype/src/windows/type_input.rs @@ -7,12 +7,9 @@ use windows::Win32::UI::Input::KeyboardAndMouse::{ VIRTUAL_KEY, }; -use super::{ErrorOperations, KeyboardShortcutInput, Win32ErrorOperations}; +use crate::get_modifier_keys; -const SHIFT_KEY_STR: &str = "Shift"; -const CONTROL_KEY_STR: &str = "Control"; -const ALT_KEY_STR: &str = "Alt"; -const LEFT_WINDOWS_KEY_STR: &str = "Super"; +use super::{ErrorOperations, KeyboardShortcutInput, Win32ErrorOperations}; const IS_VIRTUAL_KEY: bool = true; const IS_REAL_KEY: bool = false; @@ -92,22 +89,19 @@ impl TryFrom<&str> for KeyboardShortcutInput { type Error = anyhow::Error; fn try_from(key: &str) -> std::result::Result { - const SHIFT_KEY: u16 = 0x10; - const CONTROL_KEY: u16 = 0x11; - const ALT_KEY: u16 = 0x12; - const LEFT_WINDOWS_KEY: u16 = 0x5B; - + // not modifier key + if key.len() == 1 { + let input = build_unicode_input(InputKeyPress::Up, get_alphabetic_hotkey(key)?); + return Ok(KeyboardShortcutInput(input)); + } // the modifier keys are using the Up keypress variant because the user has already // pressed those keys in order to trigger the feature. - let input = match key { - SHIFT_KEY_STR => build_virtual_key_input(InputKeyPress::Up, SHIFT_KEY), - CONTROL_KEY_STR => build_virtual_key_input(InputKeyPress::Up, CONTROL_KEY), - ALT_KEY_STR => build_virtual_key_input(InputKeyPress::Up, ALT_KEY), - LEFT_WINDOWS_KEY_STR => build_virtual_key_input(InputKeyPress::Up, LEFT_WINDOWS_KEY), - _ => build_unicode_input(InputKeyPress::Up, get_alphabetic_hotkey(key)?), - }; - - Ok(KeyboardShortcutInput(input)) + if let Some(numeric_modifier_key) = get_modifier_keys().get(key) { + let input = build_virtual_key_input(InputKeyPress::Up, *numeric_modifier_key); + Ok(KeyboardShortcutInput(input)) + } else { + Err(anyhow!("Unsupported modifier key: {key}")) + } } } @@ -279,7 +273,7 @@ mod tests { #[test] #[serial] fn keyboard_shortcut_conversion_succeeds() { - let keyboard_shortcut = [CONTROL_KEY_STR, SHIFT_KEY_STR, "B"]; + let keyboard_shortcut = ["Control", "Alt", "B"]; let _: Vec = keyboard_shortcut .iter() .map(|s| KeyboardShortcutInput::try_from(*s)) @@ -291,7 +285,19 @@ mod tests { #[serial] #[should_panic = "Letter is not ASCII Alphabetic ([a-z][A-Z]): '1'"] fn keyboard_shortcut_conversion_fails_invalid_key() { - let keyboard_shortcut = [CONTROL_KEY_STR, SHIFT_KEY_STR, "1"]; + let keyboard_shortcut = ["Control", "Alt", "1"]; + let _: Vec = keyboard_shortcut + .iter() + .map(|s| KeyboardShortcutInput::try_from(*s)) + .try_collect() + .unwrap(); + } + + #[test] + #[serial] + #[should_panic(expected = "Unsupported modifier key: Shift")] + fn keyboard_shortcut_conversion_fails_with_shift() { + let keyboard_shortcut = ["Control", "Shift", "B"]; let _: Vec = keyboard_shortcut .iter() .map(|s| KeyboardShortcutInput::try_from(*s)) diff --git a/apps/desktop/src/autofill/components/autotype-shortcut.component.html b/apps/desktop/src/autofill/components/autotype-shortcut.component.html index 6f73d4006acb..feb1f507c97f 100644 --- a/apps/desktop/src/autofill/components/autotype-shortcut.component.html +++ b/apps/desktop/src/autofill/components/autotype-shortcut.component.html @@ -5,7 +5,7 @@

- {{ "editAutotypeShortcutDescription" | i18n }} + {{ "editAutotypeKeyboardModifiersDescription" | i18n }}

{{ "typeShortcut" | i18n }} diff --git a/apps/desktop/src/autofill/components/autotype-shortcut.component.spec.ts b/apps/desktop/src/autofill/components/autotype-shortcut.component.spec.ts index 90aa493c596f..ea394274600a 100644 --- a/apps/desktop/src/autofill/components/autotype-shortcut.component.spec.ts +++ b/apps/desktop/src/autofill/components/autotype-shortcut.component.spec.ts @@ -30,11 +30,9 @@ describe("AutotypeShortcutComponent", () => { const validShortcuts = [ "Control+A", "Alt+B", - "Shift+C", "Win+D", "control+e", // case insensitive "ALT+F", - "SHIFT+G", "WIN+H", ]; @@ -46,14 +44,7 @@ describe("AutotypeShortcutComponent", () => { }); it("should accept two modifiers with letter", () => { - const validShortcuts = [ - "Control+Alt+A", - "Control+Shift+B", - "Control+Win+C", - "Alt+Shift+D", - "Alt+Win+E", - "Shift+Win+F", - ]; + const validShortcuts = ["Control+Alt+A", "Control+Win+C", "Alt+Win+D", "Alt+Win+E"]; validShortcuts.forEach((shortcut) => { const control = createControl(shortcut); @@ -63,7 +54,7 @@ describe("AutotypeShortcutComponent", () => { }); it("should accept modifiers in different orders", () => { - const validShortcuts = ["Alt+Control+A", "Shift+Control+B", "Win+Alt+C"]; + const validShortcuts = ["Alt+Control+A", "Win+Control+B", "Win+Alt+C"]; validShortcuts.forEach((shortcut) => { const control = createControl(shortcut); @@ -88,15 +79,14 @@ describe("AutotypeShortcutComponent", () => { const invalidShortcuts = [ "Control+1", "Alt+2", - "Shift+3", "Win+4", "Control+!", "Alt+@", - "Shift+#", + "Alt+#", "Win+$", "Control+Space", "Alt+Enter", - "Shift+Tab", + "Control+Tab", "Win+Escape", ]; @@ -111,12 +101,10 @@ describe("AutotypeShortcutComponent", () => { const invalidShortcuts = [ "Control", "Alt", - "Shift", "Win", "Control+Alt", - "Control+Shift", - "Alt+Shift", - "Control+Alt+Shift", + "Control+Win", + "Control+Alt+Win", ]; invalidShortcuts.forEach((shortcut) => { @@ -127,7 +115,7 @@ describe("AutotypeShortcutComponent", () => { }); it("should reject shortcuts with invalid modifier names", () => { - const invalidShortcuts = ["Ctrl+A", "Command+A", "Super+A", "Meta+A", "Cmd+A", "Invalid+A"]; + const invalidShortcuts = ["Ctrl+A", "Command+A", "Meta+A", "Cmd+A", "Invalid+A"]; invalidShortcuts.forEach((shortcut) => { const control = createControl(shortcut); @@ -137,7 +125,7 @@ describe("AutotypeShortcutComponent", () => { }); it("should reject shortcuts with multiple base keys", () => { - const invalidShortcuts = ["Control+A+B", "Alt+Ctrl+Shift"]; + const invalidShortcuts = ["Control+A+B", "Alt+Ctrl+Win"]; invalidShortcuts.forEach((shortcut) => { const control = createControl(shortcut); @@ -148,11 +136,10 @@ describe("AutotypeShortcutComponent", () => { it("should reject shortcuts with more than two modifiers", () => { const invalidShortcuts = [ - "Control+Alt+Shift+A", + "Control+Alt+Win+A", "Control+Alt+Win+B", - "Control+Shift+Win+C", - "Alt+Shift+Win+D", - "Control+Alt+Shift+Win+E", + "Control+Alt+Win+C", + "Alt+Control+Win+D", ]; invalidShortcuts.forEach((shortcut) => { @@ -221,7 +208,7 @@ describe("AutotypeShortcutComponent", () => { }); it("should handle very long strings", () => { - const longString = "Control+Alt+Shift+Win+A".repeat(100); + const longString = "Control+Alt+Win+A".repeat(100); const control = createControl(longString); const result = validator(control); expect(result).toEqual({ invalidShortcut: { message: "Invalid shortcut" } }); @@ -230,7 +217,7 @@ describe("AutotypeShortcutComponent", () => { describe("modifier combinations", () => { it("should accept all possible single modifier combinations", () => { - const modifiers = ["Control", "Alt", "Shift", "Win"]; + const modifiers = ["Control", "Alt", "Win"]; modifiers.forEach((modifier) => { const control = createControl(`${modifier}+A`); @@ -240,14 +227,7 @@ describe("AutotypeShortcutComponent", () => { }); it("should accept all possible two-modifier combinations", () => { - const combinations = [ - "Control+Alt+A", - "Control+Shift+A", - "Control+Win+A", - "Alt+Shift+A", - "Alt+Win+A", - "Shift+Win+A", - ]; + const combinations = ["Control+Alt+A", "Control+Win+A", "Alt+Win+A"]; combinations.forEach((shortcut) => { const control = createControl(shortcut); @@ -257,12 +237,7 @@ describe("AutotypeShortcutComponent", () => { }); it("should reject all three-modifier combinations", () => { - const combinations = [ - "Control+Alt+Shift+A", - "Control+Alt+Win+A", - "Control+Shift+Win+A", - "Alt+Shift+Win+A", - ]; + const combinations = ["Control+Alt+Win+A", "Alt+Control+Win+A", "Win+Alt+Control+A"]; combinations.forEach((shortcut) => { const control = createControl(shortcut); @@ -270,12 +245,6 @@ describe("AutotypeShortcutComponent", () => { expect(result).toEqual({ invalidShortcut: { message: "Invalid shortcut" } }); }); }); - - it("should reject all four modifiers combination", () => { - const control = createControl("Control+Alt+Shift+Win+A"); - const result = validator(control); - expect(result).toEqual({ invalidShortcut: { message: "Invalid shortcut" } }); - }); }); }); }); diff --git a/apps/desktop/src/autofill/components/autotype-shortcut.component.ts b/apps/desktop/src/autofill/components/autotype-shortcut.component.ts index 3c82d8297a14..4e1a0c2108c6 100644 --- a/apps/desktop/src/autofill/components/autotype-shortcut.component.ts +++ b/apps/desktop/src/autofill/components/autotype-shortcut.component.ts @@ -77,25 +77,31 @@ export class AutotypeShortcutComponent { } } + // private buildShortcutFromEvent(event: KeyboardEvent): string | null { const hasCtrl = event.ctrlKey; const hasAlt = event.altKey; const hasShift = event.shiftKey; - const hasMeta = event.metaKey; // Windows key on Windows, Command on macOS + const hasSuper = event.metaKey; // Windows key on Windows, Command on macOS - // Require at least one modifier (Control, Alt, Shift, or Super) - if (!hasCtrl && !hasAlt && !hasShift && !hasMeta) { + // Require at least one valid modifier (Control, Alt, Super) + if (!hasCtrl && !hasAlt && !hasSuper) { return null; } const key = event.key; - // Ignore pure modifier keys themselves - if (key === "Control" || key === "Alt" || key === "Shift" || key === "Meta") { + // disallow pure modifier keys themselves + if (key === "Control" || key === "Alt" || key === "Meta") { return null; } - // Accept a single alphabetical letter as the base key + // disallow shift modifier + if (hasShift) { + return null; + } + + // require a single alphabetical letter as the base key const isAlphabetical = typeof key === "string" && /^[a-z]$/i.test(key); if (!isAlphabetical) { return null; @@ -108,10 +114,7 @@ export class AutotypeShortcutComponent { if (hasAlt) { parts.push("Alt"); } - if (hasShift) { - parts.push("Shift"); - } - if (hasMeta) { + if (hasSuper) { parts.push("Super"); } parts.push(key.toUpperCase()); @@ -129,10 +132,9 @@ export class AutotypeShortcutComponent { } // Must include exactly 1-2 modifiers and end with a single letter - // Valid examples: Ctrl+A, Shift+Z, Ctrl+Shift+X, Alt+Shift+Q + // Valid examples: Ctrl+A, Alt+B, Ctrl+Alt+X, Alt+Control+Q, Win+B, Ctrl+Win+A // Allow modifiers in any order, but only 1-2 modifiers total - const pattern = - /^(?=.*\b(Control|Alt|Shift|Win)\b)(?:Control\+|Alt\+|Shift\+|Win\+){1,2}[A-Z]$/i; + const pattern = /^(?=.*\b(Control|Alt|Win)\b)(?:Control\+|Alt\+|Win\+){1,2}[A-Z]$/i; return pattern.test(value) ? null : { invalidShortcut: { message: this.i18nService.t("invalidShortcut") } }; diff --git a/apps/desktop/src/autofill/models/main-autotype-keyboard-shortcut.ts b/apps/desktop/src/autofill/models/main-autotype-keyboard-shortcut.ts index b26be92585ea..8b241ade032c 100644 --- a/apps/desktop/src/autofill/models/main-autotype-keyboard-shortcut.ts +++ b/apps/desktop/src/autofill/models/main-autotype-keyboard-shortcut.ts @@ -1,4 +1,14 @@ -import { defaultWindowsAutotypeKeyboardShortcut } from "../services/desktop-autotype.service"; +/** + Electron's representation of modifier keys + +*/ +export const CONTROL_KEY_STR = "Control"; +export const ALT_KEY_STR = "Alt"; +export const SUPER_KEY_STR = "Super"; + +export const VALID_SHORTCUT_MODIFIER_KEYS: string[] = [CONTROL_KEY_STR, ALT_KEY_STR, SUPER_KEY_STR]; + +export const DEFAULT_KEYBOARD_SHORTCUT: string[] = [CONTROL_KEY_STR, ALT_KEY_STR, "B"]; /* This class provides the following: @@ -13,7 +23,7 @@ export class AutotypeKeyboardShortcut { private autotypeKeyboardShortcut: string[]; constructor() { - this.autotypeKeyboardShortcut = defaultWindowsAutotypeKeyboardShortcut; + this.autotypeKeyboardShortcut = DEFAULT_KEYBOARD_SHORTCUT; } /* @@ -51,14 +61,16 @@ export class AutotypeKeyboardShortcut { This private function validates the strArray input to make sure the array contains valid, currently accepted shortcut keys for Windows. - Valid windows shortcut keys: Control, Alt, Super, Shift, letters A - Z - Valid macOS shortcut keys: Control, Alt, Command, Shift, letters A - Z (not yet supported) + Valid shortcut keys: Control, Alt, Super, letters A - Z + Platform specifics: + - On Windows, Super maps to the Windows key. + - On MacOS, Super maps to the Command key. + - On MacOS, Alt maps to the Option key. See Electron keyboard shorcut docs for more info: https://www.electronjs.org/docs/latest/tutorial/keyboard-shortcuts */ #keyboardShortcutIsValid(strArray: string[]) { - const VALID_SHORTCUT_CONTROL_KEYS: string[] = ["Control", "Alt", "Super", "Shift"]; const UNICODE_LOWER_BOUND = 65; // unicode 'A' const UNICODE_UPPER_BOUND = 90; // unicode 'Z' const MIN_LENGTH: number = 2; @@ -77,7 +89,7 @@ export class AutotypeKeyboardShortcut { // Ensure strArray is all modifier keys, and that the last key is a letter for (let i = 0; i < strArray.length; i++) { if (i < strArray.length - 1) { - if (!VALID_SHORTCUT_CONTROL_KEYS.includes(strArray[i])) { + if (!VALID_SHORTCUT_MODIFIER_KEYS.includes(strArray[i])) { return false; } } else { diff --git a/apps/desktop/src/autofill/services/desktop-autotype.service.ts b/apps/desktop/src/autofill/services/desktop-autotype.service.ts index 7ee889e7b811..8172c5d21f97 100644 --- a/apps/desktop/src/autofill/services/desktop-autotype.service.ts +++ b/apps/desktop/src/autofill/services/desktop-autotype.service.ts @@ -18,11 +18,10 @@ import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view"; import { UserId } from "@bitwarden/user-core"; import { AutotypeVaultData } from "../models/autotype-vault-data"; +import { DEFAULT_KEYBOARD_SHORTCUT } from "../models/main-autotype-keyboard-shortcut"; import { DesktopAutotypeDefaultSettingPolicy } from "./desktop-autotype-policy.service"; -export const defaultWindowsAutotypeKeyboardShortcut: string[] = ["Control", "Shift", "B"]; - export const AUTOTYPE_ENABLED = new KeyDefinition( AUTOTYPE_SETTINGS_DISK, "autotypeEnabled", @@ -52,7 +51,7 @@ export class DesktopAutotypeService { autotypeEnabledUserSetting$: Observable = of(false); resolvedAutotypeEnabled$: Observable = of(false); - autotypeKeyboardShortcut$: Observable = of(defaultWindowsAutotypeKeyboardShortcut); + autotypeKeyboardShortcut$: Observable = of(DEFAULT_KEYBOARD_SHORTCUT); constructor( private accountService: AccountService, @@ -75,7 +74,7 @@ export class DesktopAutotypeService { async init() { this.autotypeEnabledUserSetting$ = this.autotypeEnabledState.state$; this.autotypeKeyboardShortcut$ = this.autotypeKeyboardShortcut.state$.pipe( - map((shortcut) => shortcut ?? defaultWindowsAutotypeKeyboardShortcut), + map((shortcut) => shortcut ?? DEFAULT_KEYBOARD_SHORTCUT), ); // Currently Autotype is only supported for Windows diff --git a/apps/desktop/src/locales/en/messages.json b/apps/desktop/src/locales/en/messages.json index da8d9ea0e349..8df27286d301 100644 --- a/apps/desktop/src/locales/en/messages.json +++ b/apps/desktop/src/locales/en/messages.json @@ -4131,8 +4131,8 @@ "typeShortcut": { "message": "Type shortcut" }, - "editAutotypeShortcutDescription": { - "message": "Include one or two of the following modifiers: Ctrl, Alt, Win, or Shift, and a letter." + "editAutotypeKeyboardModifiersDescription": { + "message": "Include one or two of the following modifiers: Ctrl, Alt, Win, and a letter." }, "invalidShortcut": { "message": "Invalid shortcut"