Skip to content

fix(composer): prevent WebKit overpainting mention selections - #490

Merged
wesbillman merged 2 commits into
mainfrom
bad-selection-in-composer
Oct 1, 2026
Merged

fix(composer): prevent WebKit overpainting mention selections#490
wesbillman merged 2 commits into
mainfrom
bad-selection-in-composer

Conversation

@matt2e

@matt2e matt2e commented Oct 1, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Make native mention-token selection backgrounds transparent so WebKit no longer highlights unselected prose before a selected mention; retain the token’s own selected styling.
  • Add unit and browser coverage for keyboard/pointer selection across one-word and multi-word mentions, including pixel checks of the painted range.

Note

Wrapped lines ending in a token lose native selection fill between the token and the right edge when the selection continues onto the next line.

Testing

Not rerun during PR creation. The commits record verification in Chromium and WebKit on macOS and Linux.

🤖 Generated with Claude Code

matt2e and others added 2 commits October 1, 2026 15:51
@matt2e @claude
test(composer): pin Shift+Arrow selection across mention tokens
cd95643 
Root cause: the reported sequence (caret at the end of
"@imp say hello to @jitter ", Shift+Left selecting the trailing space,
a second Shift+Left selecting the whole message) did not reproduce in
Playwright Chromium or WebKit, in a system WKWebView driven by real
NSEvent Shift+Left key events (single press, held Shift with key
repeat, and a restored draft), or in jsdom with the browser's first
press modelled on the DOM selection. The second press extends over the
mention and keeps the anchor at the end. No product code is changed;
this commit adds the regression coverage that was missing so the
contract is pinned at both test layers.

Reviewed and found correct in src/features/messages/EditableInput.tsx:
`adjacent()` keeps `anchor` and moves only `head` to `token.from` or
`token.to`, so direction is preserved; `syncNativeSelection()` only
falls back to AllSelection when the DOM range already spans the whole
document, which no Shift+Arrow press produces.

Sibling cases broken and fixed: none.

Sibling cases checked and already correct:
- Shift+Right from directly before a mention selects the whole mention,
  then the trailing space; Shift+Left then shrinks it back.
- Shift+Left directly after a mention with no trailing space.
- Backward extension keeps a backward direction and anchor; forward
  extension keeps a forward direction and anchor; shrinking off a
  mention in either direction collapses to the mention edge.
- Mention at position 0 (Shift+Left stops at 0, Shift+Right from 0
  selects it) and mention at the very end of the document.
- Two adjacent @-mentions cannot exist: the mention boundary rule in
  mention-draft.ts demotes the second to plain text. Adjacent atoms are
  covered by tests/browser/emoji.spec.mjs.
- A selection started in prose crossing a mention in both directions.
- Shift+Home, Shift+End and Option/Ctrl+Shift+Arrow word jumps stay
  native and treat the mention as one word.
- Mouse drag from the end back into the prose selects the mention.

The jsdom cases drive ArrowLeft/ArrowRight keydown directly because
user-event cannot extend a selection. The browser spec covers the
parts jsdom cannot: native caret movement through prose, words and
lines, and pointer drag, handing off to the editor at the mention edge.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Signed-off-by: Matt Toohey <contact@matttoohey.com>
@matt2e @claude
fix(composer): stop WebKit painting prose before a selected mention
004c01c 
Root cause: a selection that starts exactly where prose meets a mention
token leaves that prose text box with an empty selected range, which
WebKit's RenderHighlight::highlightStateForTextBox reports as None. The
line's state then becomes End and RenderBlockFlow::inlineSelectionGaps
fills a gap from the block edge to the token, painted in the ::selection
colour of the token's parent span. The editor and DOM ranges were right
all along: after the second Shift+Left the selection is [mention, end,
backward] and the DOM range is exactly the chip plus the trailing space
(WebKit serialises it as "\n@Mary Jane\n ", the AXSelectedText shape in
the report). The AX range length exceeding the character count is the
two AX queries counting inline-flex chips differently, not a position
overshoot, so syncNativeSelection() and adjacent() in
src/features/messages/EditableInput.tsx needed no clamp.

The wrong file was src/features/messages/EditableInput.module.css: the
token span kept the editor's Highlight ::selection colour, so the false
gap was painted in it. The token already paints its own selected state
through data-editor-selected; its native ::selection background is now
transparent, which removes the gap in the system WKWebView on macOS 27
and in Playwright WebKit on macOS and Linux. Chromium never painted the
gap and is unchanged.

Trigger: any mention preceded by prose, extended over backward or
forward, with any label. A space in the display name is not the
trigger: a one-word label paints the same false gap. The two earlier
sweeps missed it because they read only the editor and DOM selection
values, which are correct, and never sampled the paint. Labels with a
space are still covered at both test layers so the contract cannot
depend on them.

Refs cd95643.

Sibling cases broken and fixed:
- Shift+Right from directly before a mention painted the same false gap
  (same range start); fixed by the same rule.
- The pointer-drag step of the browser spec from cd95643 failed on
  Linux in both engines: the press landed inside the preceding Shift+End
  range and started a drag of the selected text. The spec now collapses
  the caret before dragging.

Sibling cases checked and already correct, for a one-word label and a
label with a space ("Mary Jane"):
- Shift+Right into the mention from directly before it, then over the
  trailing space; Shift+Left shrinks back to the mention edge.
- Shift+Left directly after the mention with no trailing space.
- Backward extension keeps anchor and direction; forward extension keeps
  anchor and direction; shrinking off the mention in either direction.
- Mention at position 0 and at the document end.
- Two such mentions one space apart: extending backward over the first
  from a range that already covers the second, and forward over the
  second from a range that covers the first.
- Shift+Home, Shift+End and Option/Ctrl+Shift+Arrow word jumps stay
  native and treat the mention as one word.
- Two @-mentions directly adjacent remain impossible (mention-draft.ts
  boundary rule), unchanged from cd95643.

Known trade-off: a wrapped line that ends with a token loses the native
fill from the token to the line's right edge when a selection continues
onto the next line.

Tests: tests/browser/mention-selection.spec.mjs runs the matrix for both
label shapes and samples the paint from a CSS-pixel screenshot of the
line (chip padding and the centres of space characters, compared with an
idle capture rather than fixed colours). It fails on WebKit without the
CSS rule and passes on Chromium either way; verified on macOS and in the
Linux CI Playwright container for both engines. EditableInput.test.tsx
adds the label-with-a-space matrix in jsdom and asserts the selection
never exceeds the document.

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 10:03
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 actionable changes requested. The CSS reuses the existing selected-token styling; the added tests cover native selection handoffs and painted ranges without changing selection logic.

Star Lord’s automated source review via Wes — head 004c01cab66c5b2efd5284f4a715bad3a57b8122, base fefbfd05f60c5fe8623fcbc51dcd066e7be6043a. Source-only: no tests or app execution; CI was still running at inspection, and native rendering remains unverified here, including the documented wrapped-line trailing-fill tradeoff.

@wesbillman
wesbillman merged commit bc542f2 into main Oct 1, 2026
21 checks passed
@wesbillman
wesbillman deleted the bad-selection-in-composer branch October 1, 2026 14:15
@shellz-n-stuff shellz-n-stuff mentioned this pull request Oct 1, 2026
Merged
@wesbillman wesbillman mentioned this pull request Oct 1, 2026
Merged
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
zrmarley added a commit that referenced this pull request Oct 5, 2026
@zrmarley
Merge remote-tracking branch 'origin/main' into zmarley/bot-2015-uplo…
3d8bb10 
…ad-on-send

* origin/main: (155 commits)
  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)
  fix(shell): simplify top-bar controls and refine profile dropdown (#435)
  Fix macOS window dragging during identity setup (#424)
  ...

# Conflicts:
#	src/features/messages/MessageComposer.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