Skip to content

shortcuts: conflict checks compare accelerator strings while the matcher compares event candidates #205

Description

@meinharrd

Found by the alan review of #193 (round on head bab1d91e), filed as a follow-up rather than fixed there.

Problem. Every conflict layer in src/shared/shortcuts.js (findConflict, used by previewBinding, setOverride and the load/save sanitizeOverrides pass) compares accelerator strings, but the runtime matcher compares normalized event candidates. So two bindings that are different strings can match the same physical keypress and both fire.

Concrete, interactively reachable case (German layout). Recording Ctrl+Shift+0 in Settings > Shortcuts stores Ctrl+Shift+0 (the recorder is code-based). The conflict layers accept it as free — no entry has that string — but on a German layout Shift+0 produces =, so the matcher also resolves the same press to the zoom-in fixed alias CmdOrCtrl+Shift+=. Result: the user's remap and zoom-in double-fire.

Fix direction. Compare conflicts on the same normalized candidate set the matcher uses (per platform/layout), not on raw accelerator strings — i.e. findConflict should ask "would any existing binding match the events this accelerator matches?" This is a refactor of the conflict layer rather than a #193-scoped change, hence the separate issue.

Pointers. src/shared/shortcuts.js (findConflict ~:638, sanitizeOverrides), src/renderer/lib/shortcuts.js (matcher / candidates), review comment: #193

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions