fix: capitalize accelerator modifiers to fix custom keybindings#4134
fix: capitalize accelerator modifiers to fix custom keybindings#4134Bowl42 wants to merge 2 commits into
Conversation
…text#4123) The keybinding UI captured keyboard shortcuts with lowercase modifier names (e.g. "ctrl+alt+1") via getAcceleratorFromKeyboardEvent(), but Electron requires properly capitalized modifiers (e.g. "Ctrl+Alt+1"). - Add capitalizeAccelerator() utility to normalize modifier casing - Apply capitalization in key-input-dialog before saving - Fix _isDefaultBinding to use case-insensitive comparison - Add comprehensive unit tests for capitalizeAccelerator Fixes marktext#4123 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Apply capitalizeAccelerator when loading user keybindings from disk, healing legacy files that contain lowercase modifiers - Fix JSDoc: clarify that only modifiers are capitalized, not key names - Fix comment: map contains modifier names only - Add missing test cases: single modifier without key, Plus key, non-modifier special keys, idempotency Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Fixes custom keybinding registration failures caused by lowercase modifier names (e.g. ctrl+alt+1) by normalizing accelerator modifier casing to Electron’s expected format and ensuring default-binding comparisons are case-insensitive.
Changes:
- Added
capitalizeAccelerator()utility insrc/common/keybinding/index.jsto normalize modifier casing. - Applied accelerator capitalization when capturing shortcuts in the preferences UI and when loading legacy
keybindings.jsonentries. - Updated default binding detection to use
isEqualAcceleratorand added unit tests for the new utility.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| test/unit/specs/capitalize-accelerator.spec.js | Adds unit coverage for modifier capitalization behavior and edge cases. |
| src/common/keybinding/index.js | Introduces capitalizeAccelerator() for canonical modifier casing. |
| src/renderer/prefComponents/keybindings/KeybindingConfigurator.js | Makes default-binding detection case-insensitive via isEqualAccelerator. |
| src/renderer/prefComponents/keybindings/key-input-dialog.vue | Normalizes captured accelerators before validation/display/save. |
| src/main/keyboard/shortcutHandler.js | Normalizes modifier casing when loading user keybindings from disk. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| * @param {string} accelerator | ||
| * @returns {string} |
…ll position / XSS / contextIsolation - fix: 编辑菜单截图分割线仅 macOS 可见（Windows 不再出现双分割线） - fix(marktext#4093): 深色主题打开窗口预设背景色，消除白闪 - feat(marktext#4134): capitalizeAccelerator - 自定义快捷键修饰符大写修复 - fix(marktext#4146): 搜索打开文件后首次 muya 序列化不触发未保存状态 - fix(marktext#4075): 外部文件变更：已保存时静默加载，未保存时才提示 - fix(marktext#4152): 切换 Tab 保留滚动位置；打开文件光标定位到开头 - fix(marktext#4206): 图片属性 HTML 转义防 XSS/RCE - fix(marktext#4192): contextIsolation=true 防渲染进程调用 Node.js API
|
This project recently merged a large refactor (electron-vite migration, PR #4001) that upgraded dependencies and fixed several issues. Please review this PR and let us know if it is still needed, or if the changes have already been addressed upstream.
|
|
Hi @Bowl42, thanks for the well-tested PR. Before reviewing, we'd like to confirm a couple of things. The current
If so, we'll be happy to review. If you don't have the time to push it forward, we'll close after a few weeks. Thanks again for the detailed test coverage. |
Summary
Fixes #4123
getAcceleratorFromKeyboardEvent()which returns lowercase modifier names (e.g.ctrl+alt+1), but Electron requires properly capitalized modifiers (e.g.Ctrl+Alt+1) to register shortcuts correctly.capitalizeAccelerator()utility tosrc/common/keybinding/index.jsthat normalizes modifier casing against a canonical map (Ctrl, Alt, Shift, Cmd, Meta, Super, AltGr, Option, CommandOrControl, CmdOrCtrl, Command, Control).key-input-dialog.vueimmediately after capturing the accelerator, before saving._isDefaultBindinginKeybindingConfigurator.jsto useisEqualAccelerator(case-insensitive) instead of strict equality, so re-entering a default binding is correctly recognized.Test plan
capitalizeAccelerator()covering all modifier names, mixed case, empty/null input, and multi-modifier combinations (test/unit/specs/capitalize-accelerator.spec.js)Ctrl+Alt+1(notctrl+alt+1)🤖 Generated with Claude Code