Repository navigation
fix(messages): exclude row chrome from cross-message copies - #645
Conversation
wesbillman
left a comment
wesbillman
left a comment
There was a problem hiding this comment.
One change needed: make the custom-emoji clipboard path honor the new row-chrome exclusions (inline P2).
Star Lord’s automated source review via Wes’s account. Reviewed head f2c88f8ebd5c0e2d0bf4d6be1356889dbc665012 against base 27d581e5ce0dc5bd6eb6cd09f6dd0c22545f4306. Source-only: no tests or app execution; hosted checks snapshot was successful except skipped Windows native validation. The attached recording was inspected through six sampled frames, not independently reproduced or exhaustively inspected.
| margin-inline-start: auto; | ||
| /* Controls are not conversation text: a selection that spans rows passes | ||
| through this bar, and engines copy button labels the user never saw. */ | ||
| user-select: none; |
There was a problem hiding this comment.
[P2] Apply these exclusions to custom-emoji clipboard serialization too
A cross-message selection containing a rendered custom emoji still copies the hidden dates, continuation bylines and hover timestamps. The bundled Emoji plugin registers copyEmoji on document (src/bundled/emoji/index.tsx:36–39); that handler clones the entire DOM range, replaces img[data-copy-emoji] with shortcodes, and writes selectedText(fragment) to the clipboard (copy-emoji.ts:17–37). Its text walker ignores user-select: none, so these CSS-only exclusions are bypassed. A custom quick-reaction image inside an intervening action bar can activate the same path even when the message bodies contain no emoji.
Make the existing serializer omit the marked non-copyable chrome while retaining body shortcodes, and add a cross-row regression that observes the copy-event payload with a custom emoji present. The new Selection.toString() assertion alone never invokes this handler.
Copying a selection that spanned messages included text the reader never saw: the hover action bar's quick-reaction glyphs (👍 ❤️ 😂), each continuation's hover-only clock, and the screen-reader-only full dates and bylines. Chromium and WebKit copy Selection.toString(), which includes button labels, opacity-0 text and visually hidden text. Mark the action bar, the continuation clock and the rows' sr-only spans user-select: none, which both engines honour in Selection.toString() and the copy payload. Render native quick-reaction glyphs as CSS content so no text node exists for selections or DOM-walking copy handlers to read. A jsdom test spans two real rows and asserts the glyphs are absent from the selection; a Playwright case proves in both engines that a selection across rows yields only the visible byline and bodies. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Signed-off-by: Matt Toohey <contact@matttoohey.com>
Remove the inline comment above the quick-reaction glyph as requested in review. Rendering and copy behavior remain unchanged. Signed-off-by: Matt Toohey <contact@matttoohey.com>
A selection spanning messages copied cleanly unless it contained a custom emoji image. The copy handler then re-added the row chrome 43c3392 excluded with user-select: none: hidden full dates, continuation bylines and hover clocks, and the shortcode of a custom quick reaction in a hovered action bar. Skip any subtree whose computed user-select is none in the shared selection serializer, which message surfaces and the emoji plugin both use. The emoji plugin now checks the live range for a selectable custom emoji, since detached clones have no computed style, so a custom emoji that sits only in excluded chrome leaves the engine's own copy in place. New jsdom tests cover cross-row exclusion, excluded-only emoji, boundary slicing and break/block/table separators. A Playwright case copies across two real rows in Chromium and WebKit and asserts the hidden full dates are absent; it fails without the serializer check. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Matt Toohey <contact@matttoohey.com>
f2c88f8 to
6915020
Compare
wesbillman
left a comment
wesbillman
left a comment
There was a problem hiding this comment.
No further changes requested: the prior clipboard-exclusion finding is addressed by the shared live-DOM serializer, with copy-event regression coverage added.
Star Lord’s automated source review via Wes’s account; head 691502082a0db177c4dd80c71193748b6fabd4fe, base 5aeeda7ecd723eef84212c4578611f59412d6af8. Source-only: tests, app/browser behavior and CI were not validated. Attachment inspection covered six cached sample frames of the old-head recording, not the full video/audio or current-head runtime behavior.
2 participants
Copying across messages now excludes quick-reaction glyphs, hover-only timestamps, and screen-reader-only metadata while preserving visible bylines and message bodies. Native quick reactions render through CSS content to keep glyphs out of DOM text selections.
Adds a colocated selection regression test and one browser case (none removed) for real-engine selection behavior that jsdom cannot verify.
Validation: diff whitespace check passed at f2c88f8; local and origin branch heads match. Test execution, fail-then-pass evidence, Chromium/WebKit checks, agent review, and human app verification remain unconfirmed; keeping this PR in draft.
Browser copy/paste comparison — Before
27d581e5(parent of43c33924) vs Afterf2c88f8e. The same synthetic messages are rendered by each revision’s production frontend in Chromium 153. Both sides visibly drag-select across messages, copy with ⌘C, and paste with ⌘V into an empty plain-text area. Before leaks 👍 ❤️ 😂, hidden dates and repeated bylines; After keeps the visible author/time and message bodies. Steps are synchronized and pasted text is unmodified. Expand the player for readability.cross-message-copy-before-after.mp4
Capture verification: both native clipboard/paste checks passed, and selection = clipboard = pasted text on each revision. This adds Chromium recording evidence; WebKit/native checks, review and human verification are not attested by this recording.