If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!

Action Performed:

Open the app and login with user A
Open user B chat and send a message
Open user B email
Reply to user A message by email and send only emojis

Expected Result:

Size of emojis sent via email reply should be same as size of emoji sent via normal chat, emoji skin color should also match to the one used by the user

Actual Result:

Size of emojis is smaller then size of emojis sent via normal chat and skin color too does not match

Workaround:

unknown

Platforms:

Which of our officially supported platforms is this issue occurring on?

[ ] Android / native
[ ] Android / Chrome
[ ] iOS / native
[ ] iOS / Safari
[ ] MacOS / Chrome / Safari
[ ] MacOS / Desktop

Version Number: v1.2.63-0 Reproducible in staging?: y Reproducible in production?: y If this was caught during regression testing, add the test name, ID and link from TestRail: Email or phone of affected tester (no customers): Logs: https://stackoverflow.com/c/expensify/questions/4856 Notes/Photos/Videos:

https://user-images.githubusercontent.com/43996225/216351996-a259a36c-cd7d-4260-9e78-79d3714f275e.mp4

https://user-images.githubusercontent.com/43996225/216352049-c590c637-ddfb-46e4-9696-e002ff6bb357.mp4

Expensify/Expensify Issue URL: Issue reported by: @dhanashree-sawant Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1675272531555829

View all open jobs on GitHub

Upwork Automation - Do Not Edit

Upwork Job URL: https://www.upwork.com/jobs/~0174e50816828c7960
Upwork Job ID: 1622671435996983296
Last Price Increase: 2023-02-06

Issue Owner

Current Issue Owner: @mallenexpensify

Feb 02 '23 14:02 kavimuru

Triggered auto assignment to @mallenexpensify (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

Feb 02 '23 14:02 melvin-bot[bot]

Bug0 Triage Checklist (Main S/O)

[ ] This "bug" occurs on a supported platform (ensure Platforms in OP are ✅)
[ ] This bug is not a duplicate report (check E/App issues and #expensify-bugs)
- If it is, comment with a link to the original report, close the issue and add any novel details to the original issue instead
[ ] This bug is reproducible using the reproduction steps in the OP. S/O
- If the reproduction steps are clear and you're unable to reproduce the bug, check with the reporter and QA first, then close the issue.
- If the reproduction steps aren't clear and you determine the correct steps, please update the OP.
[ ] This issue is filled out as thoroughly and clearly as possible
- Pay special attention to the title, results, platforms where the bug occurs, and if the bug happens on staging/production.
[ ] I have reviewed and subscribed to the linked Slack conversation to ensure Slack/Github stay in sync

Feb 02 '23 14:02 melvin-bot[bot]

Testing, not getting emails for all chats sent from my test account.

Feb 03 '23 00:02 mallenexpensify

@mallenexpensify Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

Feb 06 '23 18:02 MelvinBot

ah ha! Was able to reproduce, nice find @dhanashree-sawant Pretty sure this can be external.

Feb 06 '23 18:02 mallenexpensify

Job added to Upwork: https://www.upwork.com/jobs/~0174e50816828c7960

Feb 06 '23 18:02 MelvinBot

Current assignee @mallenexpensify is eligible for the External assigner, not assigning anyone new.

Feb 06 '23 18:02 MelvinBot

Triggered auto assignment to Contributor-plus team member for initial proposal review - @mananjadhav (External)

Feb 06 '23 19:02 MelvinBot

Triggered auto assignment to @pecanoro (External), see https://stackoverflow.com/c/expensify/questions/7972 for more details.

Feb 06 '23 19:02 MelvinBot

Proposal

RCA

Whenever a comment is sent from the email, the update is of this format -

[{"onyxMethod":"merge","key":"report_7264088055740850","value":{"reportID":"7264088055740850","maxSequenceNumber":49,"lastActionCreated":"2023-02-06 21:10:18.038","lastReadTime":"2023-02-06 21:10:08.928","lastMessageText":"👴👴","lastActorEmail":"[email protected]"}},{"onyxMethod":"merge","key":"reportActions_7264088055740850","value":{"49":{"person":[{"type":"TEXT","style":"strong","text":"expert"}],"actorEmail":"[email protected]","actorAccountID":13064488,"message":[{"type":"COMMENT","html":"<div dir=\"ltr\">👴👴</div>","text":"👴👴","isEdited":false}],"originalMessage":{"html":"<div dir=\"ltr\">👴👴</div>","source":"email"},"avatar":"https://d1wpcgnaa73g0y.cloudfront.net/497a9e0151014341987046ea878258b3833ec0b7_128.jpeg","created":"2023-02-06 21:10:18.038","timestamp":1675717818,"reportActionTimestamp":1675717818038,"automatic":false,"sequenceNumber":49,"actionName":"ADDCOMMENT","shouldShow":true,"clientID":"","reportActionID":"7249547654175174326","isAttachment":false},"":null},"shouldNotify":true}]

From the backend, the emojis are enclosed with <div dir="ltr">{emojis}</div>. As per this code, we check if the text is to be rendered as HTML or Text component - https://github.com/Expensify/App/blob/37e93444ff414b7cb6de81933dbe194ce7f787fa/src/pages/home/report/ReportActionItemFragment.js#L101-L119

We only check for the existence of emoji only texts if the text is being rendered using the Text component and not as an HTML.

Solution

We can add conditions to check that if the text obtained from an email comment contains only emojis and if it does, apply the onlyEmojisText style to the element by enclosing it with a custom tag.

diff --git a/src/components/HTMLEngineProvider/BaseHTMLEngineProvider.js b/src/components/HTMLEngineProvider/BaseHTMLEngineProvider.js
index 456cd1ecf3..9402cd8314 100755
--- a/src/components/HTMLEngineProvider/BaseHTMLEngineProvider.js
+++ b/src/components/HTMLEngineProvider/BaseHTMLEngineProvider.js
@@ -35,6 +35,10 @@ const customHTMLElementModels = {
         tagName: 'comment',
         mixedUAStyles: {whiteSpace: 'pre'},
     }),
+    'emojis-only': defaultHTMLElementModels.div.extend({
+        tagName: 'emojis-only',
+        mixedUAStyles: {...styles.onlyEmojisText},
+    }),
     'email-comment': defaultHTMLElementModels.div.extend({
         tagName: 'email-comment',
         mixedUAStyles: {whiteSpace: 'normal'},

diff --git a/src/pages/home/report/ReportActionItemFragment.js b/src/pages/home/report/ReportActionItemFragment.js
index c4f593517d..a700b2a0e4 100644
--- a/src/pages/home/report/ReportActionItemFragment.js
+++ b/src/pages/home/report/ReportActionItemFragment.js
@@ -108,7 +108,8 @@ const ReportActionItemFragment = (props) => {
             // Only render HTML if we have html in the fragment
             if (!differByLineBreaksOnly) {
                 const editedTag = props.fragment.isEdited ? '<edited></edited>' : '';
-                const htmlContent = html + editedTag;
+                const emojiOnlyTag = EmojiUtils.containsOnlyEmojis(text) ? `<emojis-only>${html}</emojis-only>` : html;
+                const htmlContent = emojiOnlyTag + editedTag;
                 return (
                     <RenderHTML
                         html={props.source === 'email'

Regarding the emoji skin color issue, I believe it is not a bug and we should not handle that. When a user responds using email, they have specifically used the default emoji tone. If they were to instead use custom emoji skin colors, such as 🖐🏻🖐🏿🖐🏻, then when it renders in expensify, it renders correctly as 🖐🏻🖐🏿🖐🏻. If were to instead change all the emojis to the default skin color of the user, it would convert it to 🖐🏻🖐🏻🖐🏻, if the default tone was white. This would lead to a loss of information and the best way to deal with this is to do nothing. If the user chooses to send default skin tone, then we display default skin tone.

Results

https://user-images.githubusercontent.com/54077356/217096550-3b4b836e-655d-4abd-b25b-f175e58815f0.mp4

Feb 06 '23 21:02 Prince-Mendiratta

Proposal 2

Solution

We can simply check if the text contains only emojis. If it does, then display it using Text instead. The result is same as in my above proposal.

diff --git a/src/pages/home/report/ReportActionItemFragment.js b/src/pages/home/report/ReportActionItemFragment.js
index c4f593517d..3e4cb3915b 100644
--- a/src/pages/home/report/ReportActionItemFragment.js
+++ b/src/pages/home/report/ReportActionItemFragment.js
@@ -106,7 +106,7 @@ const ReportActionItemFragment = (props) => {
             const differByLineBreaksOnly = Str.replaceAll(html, '<br />', '\n') === text;
 
             // Only render HTML if we have html in the fragment
-            if (!differByLineBreaksOnly) {
+            if (!differByLineBreaksOnly && !EmojiUtils.containsOnlyEmojis(text)) {
                 const editedTag = props.fragment.isEdited ? '<edited></edited>' : '';
                 const htmlContent = html + editedTag;
                 return (

Proposal 3

Why are we adding the extra div in the backend? I'd say we can directly remove the div from the backend since then the component would use the Text component and it applies the ltr style anyways.

Feb 06 '23 22:02 Prince-Mendiratta

Proposal

diff --git a/src/pages/home/report/ReportActionItemFragment.js b/src/pages/home/report/ReportActionItemFragment.js
index c4f593517d..b4ced62e10 100644
--- a/src/pages/home/report/ReportActionItemFragment.js
+++ b/src/pages/home/report/ReportActionItemFragment.js
@@ -104,9 +104,10 @@ const ReportActionItemFragment = (props) => {
             // we render it as text, not as html.
             // This is done to render emojis with line breaks between them as text.
             const differByLineBreaksOnly = Str.replaceAll(html, '<br />', '\n') === text;
+            const differByDivTagsOnly = Str.replaceAll(html, /<div[^>]*>|<\/div>/g, '') === text;
 
             // Only render HTML if we have html in the fragment
-            if (!differByLineBreaksOnly) {
+            if (!differByLineBreaksOnly && !differByDivTagsOnly) {
                 const editedTag = props.fragment.isEdited ? '<edited></edited>' : '';
                 const htmlContent = html + editedTag;
                 return (

RCA

Sending messages from email will most likely be sent as html and that's the case here where emojis are sent as <div dir="ltr">😁</div>. Since the message is technically html it will be rendered using our HTML render engine. However as you can see there is nothing special about that message to treat it as html and we can instead just treat as a text.

We follow this approach in another similar case where the difference between the html message and the plaintext message is just line breaks (differByLineBreaksOnly) and I propose to do the same here by checking if the difference between the two messages is just those "useless" div tags (differByDivTagsOnly).

So, we strip the html message from those useless div tags and if it's the same as the text then just treat it as text.

We can extend the regex to cover other "useless" tags such as span, p, etc.
Why those tags are "useless"? because they produce no difference in the final markdown format.

Feb 06 '23 22:02 s77rt

I think this issue will eventually be fixed by this improvement.

Feb 07 '23 16:02 bernhardoj

@bernhardoj I think you are right. That issue encompasses a lot of fixes for emojis by making them consistent so I am going to add a HOLD on this one in benefit of that one. @mananjadhav What do you think? Or do you think we should fix this one separately?

Feb 07 '23 20:02 pecanoro

I can't make out if the email issue will be resolved, but we should hold this one anyway. The linked issue doesn't have any proposals so far, and I am fairly confident proposals will include some logic to match emoji, etc.

So yeah let's hold this one @pecanoro

Feb 08 '23 01:02 mananjadhav

Still on HOLD

Feb 17 '23 23:02 pecanoro

Still on HOLD waiting for https://github.com/Expensify/App/pull/15088

Feb 27 '23 17:02 pecanoro

The updated PR link we're waiting on is https://github.com/Expensify/App/pull/15611 (long story)

Mar 08 '23 17:03 mallenexpensify