fix: Convert pasted emoji images to text in composer#89625
Conversation
Codecov Report❌ Looks like you've decreased code coverage for some files. Please write tests to increase, or at least maintain, the existing level of code coverage. See our documentation here for how to interpret this table.
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0c5ccbfda3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
|
Codex Review: Something went wrong. Try again later by commenting “@codex review”. ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
@Uzaifm127 Could you merge main? |
|
@truph01 Done. |
Explanation of Change
This PR is solving three related emoji paste issues in the composer.
Original issue: Emojis paste as broken images
Some apps copy emojis as
<img>tags instead of Unicode emoji text. Previously, those image tags were passed directly toParser.htmlToMarkdown(), which could turn them into markdown image syntax like:Those image URLs are not reliable in the app, so the composer could show broken images instead of emojis.
To fix this, we now parse the pasted HTML first, check all embedded images, and replace emoji images with text before converting the HTML to markdown.
Issue 1: First paste keeps Slack emoji shortcodes
Slack can provide emoji image
alttext as shortcode text, like:tada:. If we pass that through as-is, the composer can keep the shortcode on first paste.The root cause is that the pasted shortcode text was relying on the composer’s later shortcode-to-emoji conversion. That conversion runs in
ComposerWithSuggestionsthroughreplaceAndExtractEmojis():App/src/pages/inbox/report/ReportActionCompose/ComposerWithSuggestions/ComposerWithSuggestions.tsx
Line 433 in 1029bad
Inside
replaceEmojis(), if the emoji trie is not ready yet, it returns the original text unchanged:App/src/libs/EmojiUtils.tsx
Lines 397 to 400 in 1029bad
To fix this, when an emoji image has shortcode
alttext, we convert it directly to the Unicode emoji during HTML paste handling, before inserting it into the composer.Issue 2: iOS Safari pastes emojis as blob markdown images
This is different from the original broken image issue because iOS Safari does not keep the original Slack emoji CDN URL in the pasted HTML. Instead, it gives us a local blob image URL and puts the emoji information in the image
alttext as a codepoint filename, like:1f389@2x.pngPreviously, that blob image was still passed to
Parser.htmlToMarkdown(), so it could become markdown like:To fix this, we detect those codepoint-style image filenames and convert them to Unicode emoji before markdown conversion.
Fixed Issues
$ #85907
PROPOSAL: #85907 (comment)
Tests
Test 1 - Emojis pasted from Slack render as images
.Test 2 - First paste keeps slack emoji shortcodes
:tada:are converted to Unicode emojis like🎉.Test 3 - iOS safari pastes emojis as blob markdown images
.Offline tests
Same as Test
QA Steps
Same as Test
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand notonIconClick)src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))npm run compress-svg)Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.ScrollViewcomponent to make it scrollable when more elements are added to the page.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Android: Native
Android-native.mp4
Android: mWeb Chrome
android-mweb.mp4
iOS: Native
ios-native.mp4
iOS: mWeb Safari
ios-safari.mp4
MacOS: Chrome / Safari
macOS-1
macOS-1.mov
macOS-2
macOS-2.mov
macOS with logs
macOS-2-with-logs.mov