Skip to content

fix(composer): prevent arrow keys from inserting control characters - #488

Merged
wesbillman merged 2 commits into
mainfrom
right-arrow-bad-chars
Oct 1, 2026
Merged

fix(composer): prevent arrow keys from inserting control characters#488
wesbillman merged 2 commits into
mainfrom
right-arrow-bad-chars

Conversation

@matt2e

@matt2e matt2e commented Oct 1, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Reject native caret/function-key control-character insertions before they change the composer, with a fallback guard for uncancelable input.
  • Recover malformed caret keys from their physical key identity and move the caret instead, preserving modifier behavior and mention-chip navigation.
  • Add editor, composer, and browser regression coverage for unchanged message text and caret movement.

Testing

  • Not rerun during PR creation.

🤖 Generated with Claude Code

@matt2e @claude
fix(composer): refuse caret keys committed as control characters and …
8047263 
…move the caret instead

In the desktop build a Right Arrow press in the message composer could leave
a glyphless box in the text. The key event reaches WebKit with its text set to
the key's raw keyboard-layout translation, U+001D, instead of AppKit's
function-key character U+F703. WebKit blanks only the private-use block, so
the keydown carries `key: "\u001d"` with `code` and `keyCode` still naming
Right Arrow, no `moveRight:` command runs, and the control character is typed
through `insertText`. The persisted draft then keeps the character.

EditableInput now refuses such insertions at both native seams: `beforeinput`
cancels a cancelable `insertText` whose data is only control characters (C0,
DEL, C1 and U+F700 through U+F747, never tab, newline or carriage return), and
`handleTextInput` returns without a transaction for one that arrived anyway so
ProseMirror redraws the DOM from the unchanged document. Text mixing a real
character with a control character passes untouched.

A caret key reported that way is also recognised from its code and handled as
the key it is: token boundaries still go through `adjacent()`, an inline leaf
beside the caret is stepped over in document terms, and every other move uses
the browser's own `Selection.modify` with the platform's modifier meanings
(word and line ends sideways, paragraph and document ends vertically, Shift
extending), followed by the existing native-selection sync. Page keys and, on
Apple platforms, Home and End move nothing, as the hardware keys do not.

Verified in a system WKWebView loaded with the mentions fixture: a key event
with text U+001D, key code 124, now types nothing and moves the caret one
character right at either end of the text, and U+001F moves it down a line,
while hardware-shaped events behave as before. Vitest covers both seams for
twenty control characters, the caret-key matrix on Apple and non-Apple
platforms, mention-chip stepping and the host composer; the Playwright
journey runs the native insertion path and the mangled keydown on Chromium
and WebKit.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Signed-off-by: Matt Toohey <contact@matttoohey.com>
@matt2e
matt2e requested review from a team, comp615 and wesbillman as code owners October 1, 2026 08:10
wesbillman
wesbillman reviewed Oct 1, 2026
View reviewed changes

@wesbillman wesbillman left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two selection fixes are needed: preserve backward Shift-selection direction, and keep block nodes out of the inline-leaf shortcut (details inline).

Automated source review by Star Lord, via Wes’s account. Head: 80472631d0c52725b9d9210a88f20e1651d2bee6; base: fefbfd05f60c5fe8623fcbc51dcd066e7be6043a.

Source-only: no tests, app execution, or CI verification. The added browser case is appropriate for native insertion/caret behavior, but fail-before/pass-after evidence is not supplied in the PR description. Public description/diff/commit material checked; no privacy issue identified.

Comment thread src/features/messages/EditableInput.tsx Outdated
Comment thread src/features/messages/EditableInput.tsx Outdated
@matt2e @claude
fix(composer): keep recovered caret moves directional and restrict le…
44b45c0 
…af stepping to inline leaves

Two review threads on #488 found cases where `moveCaret`, which handles a
caret key the host reported as a control character, left the wrong selection.

After `Selection.modify` the DOM range is new while the editor still holds
the old one, so `syncNativeSelection` never took its early return for an
already-synchronized range. A Shift move that reached both document edges
(macOS Cmd+Shift+Up from the end of a draft) was therefore promoted to
AllSelection, whose anchor is always the document start, and reversing with
Shift+Down extended from the start instead of shrinking the selection.
`syncNativeSelection` now takes a `directional` option that keeps the
`TextSelection.between` range as it is; `moveCaret` passes it, and the
promotion remains for the DOM select-all path alone.

The leaf shortcut, which steps over a mention chip or hard break in document
terms, accepted any non-text node beside the head. After select-all the
AllSelection's head sits after the last block, so a recovered Shift+Left
subtracted that whole block's size and collapsed a one-paragraph draft to
(0, 0). The shortcut now requires an inline leaf and leaves blocks to the
browser's caret motion.

The Vitest stand-in for `Selection.modify` now resolves a focus at an element
boundary into text and models lines and document boundaries over the editor's
text, so the editor's selection sync after each move is exercised. Regressions
cover Cmd+Shift+Up from the end followed by Shift+Down, checking anchor, head
and text, and select-all followed by a recovered Shift+Left.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Signed-off-by: Matt Toohey <contact@matttoohey.com>
wesbillman
wesbillman reviewed Oct 1, 2026
View reviewed changes

@wesbillman wesbillman left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No further changes requested: both previous selection findings are addressed in source, with focused regressions for directional selection and select-all shrinking. The public PR description, changed files, and commit messages revealed no privacy issue; no images were attached.

Automated source re-review by Star Lord, via Wes’s account — head 44b45c0eb9f92ca3f59c63b97043c15b2435e191, base fefbfd05f60c5fe8623fcbc51dcd066e7be6043a. No tests, app execution, or CI verification; the selection stub does not establish native/browser behavior.

@wesbillman
wesbillman merged commit 5d5094e into main Oct 1, 2026
21 checks passed
@wesbillman
wesbillman deleted the right-arrow-bad-chars branch October 1, 2026 14:01
johnmatthewtennant added a commit that referenced this pull request Oct 1, 2026
@johnmatthewtennant
Merge remote-tracking branch 'origin/main' into sol/agents-dm-action
a468f2c 
* origin/main: (82 commits)
  Test provider connections before model selection (#500)
  Bundle Goose ACP with Buzz (#497)
  Discover saved identities across joined communities with names, pictures and retry (#291)
  Clarify design-system documentation and unify component examples (#498)
  feat(composer): convert typed Markdown live and refuse control characters committed as text (#455)
  fix(messages): stop three timeline scroll races that flake CI (#456)
  Improve Agent defaults pickers and provider keys (#392)
  fix(threads): keep thread history painted after scroll corrections (#493)
  feat(plugins): expose the agent protection service (#421)
  perf(sidebar): re-render only the changed row on a channel-list publish (#480)
  feat(agents): copy protection defaults into new agents (#420)
  feat(agents): support native launch protection providers (#415)
  fix(composer): prevent WebKit overpainting mention selections (#490)
  fix(composer): prevent arrow keys from inserting control characters (#488)
  perf(channels): fall back to one exact roster read when confirming agent adds (#485)
  fix(media): pause video only on comment composer focus (#483)
  fix(channels): dismiss management modals with outside clicks (#479)
  perf: reuse message date formats and stable reaction shortcuts (#477)
  feat(profile): run an unattended scenario file in web profiling (#476)
  feat(channels): administer channel members and roles (#453)
  ...

Signed-off-by: John Tennant <jtennant@block.xyz>

# Conflicts:
#	src/app/shell/usePanelLauncher.ts
#	src/bundled/agents/AgentsPage.tsx
#	src/bundled/agents/InventoryIdentityCard.tsx
#	src/bundled/agents/InventoryView.tsx
#	src/bundled/agents/UnifiedInventory.tsx
#	src/bundled/agents/index.tsx
johnmatthewtennant pushed a commit that referenced this pull request Oct 1, 2026
Sol
Merge remote-tracking branch 'origin/main' into sol/agents-dm-action
3825a9f 
* origin/main: (82 commits)
  Test provider connections before model selection (#500)
  Bundle Goose ACP with Buzz (#497)
  Discover saved identities across joined communities with names, pictures and retry (#291)
  Clarify design-system documentation and unify component examples (#498)
  feat(composer): convert typed Markdown live and refuse control characters committed as text (#455)
  fix(messages): stop three timeline scroll races that flake CI (#456)
  Improve Agent defaults pickers and provider keys (#392)
  fix(threads): keep thread history painted after scroll corrections (#493)
  feat(plugins): expose the agent protection service (#421)
  perf(sidebar): re-render only the changed row on a channel-list publish (#480)
  feat(agents): copy protection defaults into new agents (#420)
  feat(agents): support native launch protection providers (#415)
  fix(composer): prevent WebKit overpainting mention selections (#490)
  fix(composer): prevent arrow keys from inserting control characters (#488)
  perf(channels): fall back to one exact roster read when confirming agent adds (#485)
  fix(media): pause video only on comment composer focus (#483)
  fix(channels): dismiss management modals with outside clicks (#479)
  perf: reuse message date formats and stable reaction shortcuts (#477)
  feat(profile): run an unattended scenario file in web profiling (#476)
  feat(channels): administer channel members and roles (#453)
  ...

Signed-off-by: Sol <49aa1f65411fd096d2e2ec144f1e7aa36fdc76d1b907cfdf7be000c66f9d3b8e@buzz.block.builderlab.xyz>

# Conflicts:
#	src/app/shell/usePanelLauncher.ts
#	src/bundled/agents/AgentsPage.tsx
#	src/bundled/agents/InventoryIdentityCard.tsx
#	src/bundled/agents/InventoryView.tsx
#	src/bundled/agents/UnifiedInventory.tsx
#	src/bundled/agents/index.tsx
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@wesbillman wesbillman wesbillman approved these changes
@comp615 comp615 Awaiting requested review from comp615 comp615 is a code owner

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@matt2e @wesbillman