[$500] Do not allow emojis to be formatted (ie italics)

[kavimuru]

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:

1. Go to android app
2. Go to any chat
3. Try to make emoji in italic form:-Type in : insertemoji For example:⁦:100:, ⁦:purple_heart: , ⁦:partying_face: ,
4. Notice that the top right part of the emoji cuts off. Few Example: the :100:, :purple_heart:, :partying_face: emoji has its top right section cut , (It happens on android only latest)

Expected Result:

Users should not be able to format emojis, lets keep them with normal style (no bold, italics, strikethrough etc)

Actual Result:

Users can make emoji italic which leads to the emojis to cut off a few parts of the emoji (for some emojis)

Workaround:

unknown

Platforms:

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

• [x] Android / native
• [x] MacOS Desktop
• [x] MacOS Chrome Web

Version Number: 1.2.62-1 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:

Expensify/Expensify Issue URL: Issue reported by: @priya-zha Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1675094271396099

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~017d08e0c09284d31f
• Upwork Job ID: 1620563000799895552
• Last Price Increase: 2024-04-19
• Automatic offers:
• eh2077 | Reviewer | 0
• wildan-m | Contributor | 0

Issue Owner

Current Issue Owner: @BartoszGrajdek / @mallenexpensify

[anmurali]

Confirmed that this happens on Android native app.

Confirmed it does not happen on Mac/Chrome. But related, when I try the same reproduction steps on iOS native, iOS Chrome and Mac Safari - I cannot italicize emojis at all. Is that expected?

[melvin-bot[bot]]

Job added to Upwork: https://www.upwork.com/jobs/~017d08e0c09284d31f

[melvin-bot[bot]]

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

[melvin-bot[bot]]

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

[melvin-bot[bot]]

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

[alexxxwork]

Proposal

Quick fix:

diff --git a/src/styles/styles.js b/src/styles/styles.js
index b826ed336f..84286591e9 100644
--- a/src/styles/styles.js
+++ b/src/styles/styles.js
@@ -63,6 +63,7 @@ const webViewStyles = {
         em: {
             fontFamily: fontFamily.EXP_NEUE,
             fontStyle: 'italic',
+            width: variables.fontSizeNormalHeight,
         },
 
         del: {

[flodnv]

I don't think we should be able to italicize emojis, asked here: https://expensify.slack.com/archives/C01GTK53T8Q/p1675266083150699

[JmillsExpensify]

Agree with @flodnv.

[tienifr]

Proposal

The Emoji is italicized because it is contained in the <em></em> tag (same as when we make a text italic). We can observe the behavior on both Chrome web and Android.

In order to remove the ability to italicize emojis, I'll wrap any emoji that are inside the <em></em> with a expensify-emoji custom HTML tag, and give it a fontStyle: 'normal' style. This is the same way that Github does it.

(The emoji regex below is taken from the CONST.REGEX.EMOJIS in Expensify app)

In https://github.com/Expensify/expensify-common

diff --git a/lib/ExpensiMark.js b/lib/ExpensiMark.js
index 7f94ffe..057e87c 100644
--- a/lib/ExpensiMark.js
+++ b/lib/ExpensiMark.js
@@ -134,6 +134,11 @@ export default class ExpensiMark {
                 regex: /(?!_blank")\b_((?!\s).*?\S)_\b(?![^<]*(<\/pre>|<\/code>|<\/a>|_blank))/g,
                 replacement: '<em>$1</em>',
             },
+            {
+                name: 'emoji',
+                regex: /(<em>.*?)([\p{Extended_Pictographic}\u200d\u{1f1e6}-\u{1f1ff}\u{1f3fb}-\u{1f3ff}\u{e0020}-\u{e007f}\u20E3\uFE0F]|[#*0-9]\uFE0F?\u20E3)(.*?<\/em>)/gu,
+                replacement: '$1<expensify-emoji>$2</expensify-emoji>$3',
+            },
             {
                 // Use \B in this case because \b doesn't match * or ~.
                 // \B will match everything that \b doesn't, so it works
                 

In https://github.com/Expensify/App

diff --git a/src/components/HTMLEngineProvider/BaseHTMLEngineProvider.js b/src/components/HTMLEngineProvider/BaseHTMLEngineProvider.js
index 456cd1ecf3..0bf75ea9d2 100755
--- a/src/components/HTMLEngineProvider/BaseHTMLEngineProvider.js
+++ b/src/components/HTMLEngineProvider/BaseHTMLEngineProvider.js
@@ -39,6 +39,10 @@ const customHTMLElementModels = {
         tagName: 'email-comment',
         mixedUAStyles: {whiteSpace: 'normal'},
     }),
+    'expensify-emoji': defaultHTMLElementModels.span.extend({
+        tagName: 'expensify-emoji',
+        mixedUAStyles: {fontStyle: 'normal'},
+    }),
 };
 
 const defaultViewProps = {style: [styles.alignItemsStart, styles.userSelectText]};

There're small possible variations based on our preference:

• We don't have to use a custom expensify-emoji tag but just use a span tag with a style. I prefer using a custom tag to make it clear

There're some further minor fixes required:

• when only there's only the emoji inside the em, no other text, the emoji will not be italicized but it will also not have the onlyEmojisText style, we can fix that with another regex.
• add logic to htmlToMarkdownRules to remove the Expensify-emoji tag

I'll add those if we want to go with this direction.

Pls note we have to fix the same in the BE as well, in order to test the above fix we need to enable offline mode.

[tienifr]

working well after the fix:

https://user-images.githubusercontent.com/113963320/216310065-25e96b0c-aa25-4f56-bd73-79cd9d89c878.mov

[s77rt]

@tienifr Your approach looks solid however the regex is not correct as it will only match one emoji, try sending hi 😁 there 😁, it will wrap the first emoji but not the second one. Also it will only work with em tags and I think it would be better to wrap the emojis all the time (this will also cover the bold case).

Here are my suggestions:

1. Replace the regex part with:

            {
                name: 'emoji',
                regex: /[\p{Extended_Pictographic}\u200d\u{1f1e6}-\u{1f1ff}\u{1f3fb}-\u{1f3ff}\u{e0020}-\u{e007f}\u20E3\uFE0F]|[#*0-9]\uFE0F?\u20E3/gu,
                replacement: match => `<expensify-emoji>${match}</expensify-emoji>`,
            },

2. Move the regex emoji to be before italic and after autolink (Not necessary, just a personal opinion as I like to preserve the order of italic, bold, strikethrough regex rules)
3. In mixedUAStyles: {fontStyle: 'normal'} add fontWeight: 'normal'

[tienifr]

Thanks @s77rt for your suggestion!

Updated proposal

Adding to the original proposal, Here's the my updated diff for the regex so that it matches all rather than just one.

diff --git a/lib/ExpensiMark.js b/lib/ExpensiMark.js
index c535048..62b183f 100644
--- a/lib/ExpensiMark.js
+++ b/lib/ExpensiMark.js
@@ -242,6 +242,11 @@ export default class ExpensiMark {
                 regex: /<(em|i)(?:"[^"]*"|'[^']*'|[^'">])*>(.*?)<\/\1>(?![^<]*(<\/pre>|<\/code>))/gi,
                 replacement: '_$2_',
             },
+            {
+                name: 'emoji',
+                regex: /([\p{Extended_Pictographic}\u200d\u{1f1e6}-\u{1f1ff}\u{1f3fb}-\u{1f3ff}\u{e0020}-\u{e007f}\u20E3\uFE0F]|[#*0-9]\uFE0F?\u20E3)/gu,
+                replacement: '<expensify-emoji>$1</expensify-emoji>',
+            },
             {
                 name: 'bold',
                 regex: /<(b|strong)(?:"[^"]*"|'[^']*'|[^'">])*>([\s\S]*?)<\/\1>(?![^<]*(<\/pre>|<\/code>))/gi,

[tienifr]

@s77rt Regarding the fontWeight: 'normal', I don't feel strongly about since it doesn't seem that the bold fontWeight affects how the emoji looks.

[s77rt]

@tienifr

• You can use the same regex as provided, there is no need to create a capturing group that captures the whole match (unnecessary doubling memory usage)
• If you bold the emoji it will be bold but it's really hard to notice the difference, the outer circle shadow will appear a bit thicker. Adding fontWeight: 'normal' ensures we are rendering the emoji correctly

[tienifr]

@s77rt I tried to zoom the emojis in but can't recognize any difference between an emoji that has fontWeight: 'bold' and a normal one. But we surely can add that if we want to be extra safe in that bold case (Github also does it).

Thanks for the input!

diff --git a/src/components/HTMLEngineProvider/BaseHTMLEngineProvider.js b/src/components/HTMLEngineProvider/BaseHTMLEngineProvider.js
index 456cd1ecf3..0bf75ea9d2 100755
--- a/src/components/HTMLEngineProvider/BaseHTMLEngineProvider.js
+++ b/src/components/HTMLEngineProvider/BaseHTMLEngineProvider.js
@@ -39,6 +39,10 @@ const customHTMLElementModels = {
         tagName: 'email-comment',
         mixedUAStyles: {whiteSpace: 'normal'},
     }),
+    'expensify-emoji': defaultHTMLElementModels.span.extend({
+        tagName: 'expensify-emoji',
+        mixedUAStyles: {fontStyle: 'normal', fontWeight: 'normal'},
+    }),
 };
 
 const defaultViewProps = {style: [styles.alignItemsStart, styles.userSelectText]};

[s77rt]

@tienifr While seaching I found this image.

The middle emoji is in bold but it's hard to notice also it may depend on the font. Adding fontWeight: 'normal' is more a safety step.

[MelvinBot]

@flodnv, @anmurali, @aimane-chnaif Eep! 4 days overdue now. Issues have feelings too...

[MelvinBot]

@flodnv, @anmurali, @aimane-chnaif Huh... This is 4 days overdue. Who can take care of this?

[aimane-chnaif]

reviewing proposals shortly

[bernhardoj]

Proposal

I was trying to find a solution for this and stumbled upon this issue. I would like to propose an alternative solution that would be useful later.

I agree with the above proposal to enclose all emoji with a custom tag to apply a custom style later on.

But the first thing to do is to change the emoji regex.

I previously worked on this issue by providing a new regex and it works well for that case. However, when we use that regex to match composite emojis, for example 👨‍👨‍👦, it will produce this array of string: ['👨', '‍', '👨', '‍', '👦']

The empty string is zero-width joiner. Again, this works well for the case I mentioned from the previous paragraph (we only care what the string contains). But for this case, we need to replace the emoji and enclose it with a tag. If we use the current emoji regex, it will enclose each part of the emoji with the tag, instead of the composited emoji.

Expected: <emoji>👨‍👨‍👦</emoji>

Actual: <emoji>👨</emoji><emoji></emoji><emoji>👨</emoji> and so on...

So, we need a better regex to capture the composite emoji, below is the new regex.

diff --git a/src/CONST.js b/src/CONST.js
index ae3ffca7f..8399de047 100755
--- a/src/CONST.js
+++ b/src/CONST.js
@@ -790,7 +790,7 @@ const CONST = {
         WEBSITE: /^((https?|ftp):\/\/)(([a-z\d]([a-z\d-]*[a-z\d])*)\.)+[a-z]{2,}(:\d+)?(\/[-a-z\d%_.~+]*)*(\?[;&a-z\d%_.~+=-]*)?(#[-a-z\d_]*)?$/i,
 
         // eslint-disable-next-line max-len, no-misleading-character-class
-        EMOJIS: /[\p{Extended_Pictographic}\u200d\u{1f1e6}-\u{1f1ff}\u{1f3fb}-\u{1f3ff}\u{e0020}-\u{e007f}\u20E3\uFE0F]|[#*0-9]\uFE0F?\u20E3/gu,
+        EMOJIS: /[\p{Extended_Pictographic}](\u200D[\p{Extended_Pictographic}]|[\u{1F3FB}-\u{1F3FF}]|[\u{E0020}-\u{E007F}]|\uFE0F|\u20E3)*|[\u{1F1E6}-\u{1F1FF}]{2}|[#*0-9]\uFE0F?\u20E3/gu,
         TAX_ID: /^\d{9}$/,
         NON_NUMERIC: /\D/g,
         EMOJI_NAME: /:[\w+-]+:/g,

I have tested the new regex against all emojis from assets/emojis file and make sure all EmojiTest.js test cases pass.

Here is how I test it:

diff --git a/tests/unit/EmojiTest.js b/tests/unit/EmojiTest.js
index 25626872c..b68bfbca0 100644
--- a/tests/unit/EmojiTest.js
+++ b/tests/unit/EmojiTest.js
@@ -1,5 +1,6 @@
 import _ from 'underscore';
 import Emoji from '../../assets/emojis';
+import CONST from '../../src/CONST';
 import * as EmojiUtils from '../../src/libs/EmojiUtils';
 
 describe('EmojiTest', () => {
@@ -11,12 +12,16 @@ describe('EmojiTest', () => {
             }
 
             // When we match every Emoji Code
-            const isEmojiMatched = EmojiUtils.containsOnlyEmojis(emoji.code);
+            const isEmojiMatched = EmojiUtils.containsOnlyEmojis(emoji.code) 
+                                && emoji.code.match(CONST.REGEX.EMOJIS).length === 1;
 
             let skinToneMatched = true;
             if (emoji.types) {
                 // and every skin tone variant of the Emoji code
-                skinToneMatched = _.every(emoji.types, emojiWithSkinTone => EmojiUtils.containsOnlyEmojis(emojiWithSkinTone));
+                skinToneMatched = _.every(emoji.types, emojiWithSkinTone => {
+                    return EmojiUtils.containsOnlyEmojis(emojiWithSkinTone) 
+                        && emoji.code.match(CONST.REGEX.EMOJIS).length === 1;
+                });
             }
             return skinToneMatched && isEmojiMatched;
         });

Here is the code part to enclose the emoji with tag.

explanation

I prefer to do the string replacement here compared to doing it on ExpensiMark. As I said at the first sentence above, this will make easier when we do the replacement inside our App code for this improvement. For the context, the mentioned improvement expects the emoji font size will have a different font size compared to the normal text, whether the text only contains emoji or also contains some text.

I know this issue does not ask for that, but if we do the replacement at ExpensiMark, later on when we work on that improvement, we need somehow to tackle the case for the emoji only text and it is not feasible to do it at ExpensiMark. If we do the replacement in our App code, we can just easily change the tag to another tag that indicates the text only contains emoji.

diff --git a/src/pages/home/report/ReportActionItemFragment.js b/src/pages/home/report/ReportActionItemFragment.js
index c4f593517..4f34733cc 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 htmlContent = EmojiUtils.encloseEmojisWithTag(html, 'emoji') + editedTag;
+                
                 return (
                     <RenderHTML
                         html={props.source === 'email'
diff --git a/src/libs/EmojiUtils.js b/src/libs/EmojiUtils.js
index d6ed8875e..dce4ad93c 100644
--- a/src/libs/EmojiUtils.js
+++ b/src/libs/EmojiUtils.js
@@ -245,6 +245,10 @@ function suggestEmojis(text, limit = 5) {
     return [];
 }
 
+function encloseEmojisWithTag(text, tag) {
+    return Str.replaceAll(text, CONST.REGEX.EMOJIS, g => `<${tag}>${g}</${tag}>`);
+}
+
 export {
     getDynamicHeaderIndices,
     mergeEmojisWithFrequentlyUsedEmojis,
@@ -253,4 +257,5 @@ export {
     replaceEmojis,
     suggestEmojis,
     trimEmojiUnicode,
+    encloseEmojisWithTag,
 };

Now, apply the style to the custom tag

diff --git a/src/styles/styles.js b/src/styles/styles.js
index 2b2a3498d..8e7d163af 100644
--- a/src/styles/styles.js
+++ b/src/styles/styles.js
@@ -1065,6 +1065,11 @@ const styles = {
         textDecorationLine: 'none',
     },
 
+    emojisText: {
+        fontStyle: 'normal', 
+        fontWeight: 'normal',
+    },
+
     onlyEmojisText: {
         fontSize: variables.fontSizeOnlyEmojis,
         lineHeight: variables.fontSizeOnlyEmojisHeight,
diff --git a/src/components/HTMLEngineProvider/BaseHTMLEngineProvider.js b/src/components/HTMLEngineProvider/BaseHTMLEngineProvider.js
index 456cd1ecf..fcfe6b809 100755
--- a/src/components/HTMLEngineProvider/BaseHTMLEngineProvider.js
+++ b/src/components/HTMLEngineProvider/BaseHTMLEngineProvider.js
@@ -39,6 +39,10 @@ const customHTMLElementModels = {
         tagName: 'email-comment',
         mixedUAStyles: {whiteSpace: 'normal'},
     }),
+    'emoji': defaultHTMLElementModels.span.extend({
+        tagName: 'emoji',
+        mixedUAStyles: styles.emojisText,
+    }),
 };
 
 const defaultViewProps = {style: [styles.alignItemsStart, styles.userSelectText]};

The last problem is when we type emojis inside code ( ) tag, the text does not show, for example 😄. The text will be converted to <code><emoji>😄</emoji></code>. We have a custom rendered for code tag and it's only taking a text inside the code tag, but the emoji is enclosed with emoji tag. The solution is to get the text inside of it.

The getEmojiTextInsideCode will iterate over the children inside the code tag and get the data (text) of it. For example, if we have this text a😄, the children will be <TText data="a" /> and <TText data="😄" />.

diff --git a/src/components/HTMLEngineProvider/htmlEngineUtils.js b/src/components/HTMLEngineProvider/htmlEngineUtils.js
index 71a5e9267..fb588ff08 100644
--- a/src/components/HTMLEngineProvider/htmlEngineUtils.js
+++ b/src/components/HTMLEngineProvider/htmlEngineUtils.js
@@ -44,8 +44,13 @@ function isInsideComment(tnode) {
     return false;
 }
 
+function getEmojiTextInsideCode(tnode) {
+    return tnode.children.reduce((prev, curr) => prev + curr.data, '');
+}
+
 export {
     computeEmbeddedMaxWidth,
     isInsideComment,
     isCommentTag,
+    getEmojiTextInsideCode,
 };
diff --git a/src/components/InlineCodeBlock/index.js b/src/components/InlineCodeBlock/index.js
index af33b4468..222b7d877 100644
--- a/src/components/InlineCodeBlock/index.js
+++ b/src/components/InlineCodeBlock/index.js
@@ -1,6 +1,7 @@
 import React from 'react';
 import inlineCodeBlockPropTypes from './inlineCodeBlockPropTypes';
 import Text from '../Text';
+import * as HTMLEngineUtils from '../HTMLEngineProvider/htmlEngineUtils';
 
 const InlineCodeBlock = (props) => {
     const TDefaultRenderer = props.TDefaultRenderer;
@@ -12,7 +13,9 @@ const InlineCodeBlock = (props) => {
             <Text
                 style={{...props.boxModelStyle, ...props.textStyle}}
             >
-                {props.defaultRendererProps.tnode.data}
+                {props.defaultRendererProps.tnode.data
+                    ? props.defaultRendererProps.tnode.data
+                    : HTMLEngineUtils.getEmojiTextInsideCode(props.defaultRendererProps.tnode)}
             </Text>
         </TDefaultRenderer>
     );
diff --git a/src/components/InlineCodeBlock/index.native.js b/src/components/InlineCodeBlock/index.native.js
index 8431ce61e..04adad238 100644
--- a/src/components/InlineCodeBlock/index.native.js
+++ b/src/components/InlineCodeBlock/index.native.js
@@ -2,6 +2,7 @@ import React from 'react';
 import styles from '../../styles/styles';
 import WrappedText from './WrappedText';
 import inlineCodeBlockPropTypes from './inlineCodeBlockPropTypes';
+import * as HTMLEngineUtils from '../HTMLEngineProvider/htmlEngineUtils';
 
 const InlineCodeBlock = (props) => {
     const TDefaultRenderer = props.TDefaultRenderer;
@@ -17,7 +18,9 @@ const InlineCodeBlock = (props) => {
                     styles.codeWordStyle,
                 ]}
             >
-                {props.defaultRendererProps.tnode.data}
+                {props.defaultRendererProps.tnode.data
+                    ? props.defaultRendererProps.tnode.data
+                    : HTMLEngineUtils.getEmojiTextInsideCode(props.defaultRendererProps.tnode)}
             </WrappedText>
         </TDefaultRenderer>
     );

Finally, I thought this improvement should be held waiting for this issue.

[tienifr]

thanks @bernhardoj for the additional insights 💪

I agree there’ll be some fixes here and there to make sure it works 100% in all cases, some of which were mentioned earlier and some you mentioned above.

Once the direction is confirmed we can dive deeper and resolve the remaining (potential) issues.

[tienifr]

@bernhardoj in the case of the 👨‍👨‍👦 emoji, it's indeed split into sub-emojis but it still renders correctly on the screen (since the sub-emojis are in the consecutive span tags and still render the correct composite emoji).

Just curious if you find any case of an emoji that has user-visible problem if it's not matched by your updated regex?

[bernhardoj]

Surprisingly it worked on web, but that's not the case for native platform.

[aimane-chnaif]

opened internal discussion here for ideal solution

[MelvinBot]

@flodnv @anmurali @aimane-chnaif this issue was created 2 weeks ago. Are we close to approving a proposal? If not, what's blocking us from getting this issue assigned? Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

[MelvinBot]

@flodnv @anmurali @aimane-chnaif this issue was created 2 weeks ago. Are we close to approving a proposal? If not, what's blocking us from getting this issue assigned? Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

[MelvinBot]

Upwork job price has been updated to $2000

[MelvinBot]

@flodnv, @anmurali, @aimane-chnaif Whoops! This issue is 2 days overdue. Let's get this updated quick!

[flodnv]

We're still discussing with @aimane-chnaif and @rushatgabhane

[anmurali]

No update over the weekend.