Fix invisible bidi characters copied with handles on web#11066
Open
a-y-ibrahim wants to merge 4 commits into
Open
Fix invisible bidi characters copied with handles on web#11066a-y-ibrahim wants to merge 4 commits into
a-y-ibrahim wants to merge 4 commits into
Conversation
forceLTR wrapped handles and display names in U+202A/U+202C directional formatting characters. On web these are part of the rendered text, so selecting and copying a handle from a profile, feed, or notification also copies the invisible characters, which break handle lookups in other Bluesky apps and tools on paste. On web, isolate direction with CSS (direction: ltr + unicode-bidi: isolate) instead, matching the existing pattern in the profile header handle, so the copied text stays clean. Native keeps the manual Unicode wrapping, which has no CSS equivalent there. Closes bluesky-social#8451
Locks in the bluesky-social#8451 fix: on web forceLTR returns the string unchanged so copied handles stay clean, and on native it wraps the string in directional formatting characters.
…chars # Conflicts: # src/view/com/notifications/NotificationFeedItem.tsx
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes an issue on web where copying handles/display names also copies invisible Unicode bidi control characters (U+202A/U+202C), which can break handle lookups when pasted elsewhere. The PR changes the web implementation to avoid injecting these characters and instead relies on web-only CSS direction isolation in key UI render sites.
Changes:
- Update
forceLTR()to return the string unchanged on web (retain Unicode wrapping on native). - Apply
direction: 'ltr'+unicode-bidi: 'isolate'viaweb(...)styling to several name/handle render locations. - Add Jest coverage for
forceLTR()web vs native behavior.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/view/com/util/PostMeta.tsx | Adds web-only CSS direction isolation to the author name text. |
| src/view/com/notifications/NotificationFeedItem.tsx | Adds web-only CSS direction isolation to the first author link text. |
| src/lib/strings/bidi.ts | Makes forceLTR() a no-op on web to prevent copying bidi control characters. |
| src/lib/strings/tests/bidi.test.ts | Adds tests validating forceLTR() behavior on web vs native. |
| src/components/ProfileCard.tsx | Adds web-only CSS direction isolation to the inline name text. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -0,0 +1,33 @@ | |||
| import {describe, expect, it} from '@jest/globals' | |||
Comment on lines
+16
to
+17
| const LEFT_TO_RIGHT_EMBEDDING = '' | ||
| const POP_DIRECTIONAL_FORMATTING = '' |
Import jest explicitly from @jest/globals instead of relying on it being global, and use explicit escapes instead of the literal invisible characters, matching the style already used in bidi.ts.
Author
|
Addressed both review comments in 4c64eb0:
Local jest run is green after the change. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Selecting and copying a handle (or display name) from a profile, feed, or notification on web also copies invisible Unicode bidi characters (U+202A and U+202C). They break handle lookups and interop when pasted into other Bluesky/ATProto apps and tools. Closes #8451.
Cause
forceLTR()insrc/lib/strings/bidi.tswraps the string in U+202A ... U+202C to keep it left-to-right. On web those control characters are part of the rendered DOM text, so they end up in the copied text.sanitizeHandle()appliesforceLTRby default, and a few display-name render sites call it directly.Change
On web, isolate the direction with CSS instead of injecting characters:
forceLTRreturns the string unchanged on web (native keeps the manual Unicode wrapping, which has no CSS equivalent there).direction: 'ltr'+unicode-bidi: 'isolate'.This mirrors the approach already used for the profile header handle in
src/screens/Profile/Header/Handle.tsx. Handles are ASCII so they stay visually LTR, and display names that may be RTL keep their isolation via the CSS, while the copied text stays clean.Updated sites:
ProfileCard,PostMeta,NotificationFeedItem. The centralforceLTRchange also covers everysanitizeHandlecall site on web.Notes
This follows the existing inline CSS pattern in
Handle.tsx. Happy to extract a shared<Bdi>component instead, as suggested in the thread, if you prefer.Testing
Ran locally against the repo toolchain:
typecheck(tsgo),lint(eslint),prettier --check, and the fulljestsuite all pass.