Skip to content

fix(extension): preserve plus keys in keyboard chords - #308

Merged
iuyo5678 merged 2 commits into
Tencent:mainfrom
MaxFreedomPollard:fix/press-plus-key-chords
Oct 27, 2026
Merged

iuyo5678 merged 2 commits into
Tencent:mainfrom
MaxFreedomPollard:fix/press-plus-key-chords

Conversation

@MaxFreedomPollard

Copy link
Copy Markdown
Contributor

bsk press 'Ctrl++' currently fails with unknown key "Ctrl++": splitting on + drops the base key. Ctrl+Shift++ and Cmd++ fail for the same reason, even though a separate + key with explicit modifiers already works.

Preserve the trailing literal + before parsing modifiers. The regression tests cover compound forms, aliases, whitespace, and the plain + key. A handler test also checks that Ctrl++ dispatches exactly the same events as + with an explicit Ctrl modifier.

Validation:

  • All six new regression cases fail on the base commit and pass with the fix.
  • Frontend lint, extension typecheck, production build, and extension/plugin/VOM/localization suites passed.
  • An isolated Chromium check exercised the real press handler with seven input forms, verifying trusted key events, modifier flags, and text insertion. On the base commit, the explicit-modifier form works and Ctrl++ fails.

@iuyo5678

Copy link
Copy Markdown
Collaborator

Thanks for the focused fix! Validation against main (8ca7911) confirms that the issue still exists and that this patch fixes the intended Ctrl++ / Cmd++ cases.
There’s one small edge case I’d like to tighten before merging: the trailing-plus check currently also accepts prefixes containing unknown tokens or an existing base key. For example:

  • Crtl++ (a typo) previously returned invalid_params, but now succeeds and inserts a literal + into the focused input.

  • Ctrl+Bogus++ previously failed, but now dispatches Ctrl + +.

  • Ctrl+A++ changes from the existing Ctrl+A interpretation to Ctrl + +.

Could we restrict the new plus-key handling to cases where the preceding tokens are all recognized modifiers? That would preserve the intended fix while avoiding these additional behavior changes.

Please also add regression coverage confirming that unknown-prefix cases return invalid_params without dispatching any key events, and that an existing base key is not overwritten by the new handling.

A small guard around the new branch should be sufficient here. With that adjustment and coverage, I’d be happy to move this toward merge.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants