[$500] The tricky emoji in the title and description seems to be inconsistent.

[kbecciv]

Held while working on an upstream feature request: https://native-html.canny.io/features/p/allow-for-multiple-font-families-when-rendering-on-the-web-via-react-native-web

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!

---

Version Number: 1.3.99.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
Expensify/Expensify Issue URL:
Issue reported by: Applause - Internal Team
Slack conversation:

Issue found when executing PR #31259

Action Performed:

1. Go to staging.new.expensify.com
2. Log in with any account
3. Click on Plus button - Assign Task
4. Input with emojis, especially tricky ones (e.g. 😶‍🌫️, 🏴󠁧󠁢󠁥󠁮󠁧󠁿),‍‍

Expected Result:

The tricky emoji in the title and description should be consistent.

Actual Result:

The tricky emoji in the title and description seems to be inconsistent.

Workaround:

Unknown

Platforms:

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

• Android: Native
• Android: mWeb Chrome
• iOS: Native
• iOS: mWeb Safari
• MacOS: Chrome / Safari
• MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

Recording.5422.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~012ade86ce4ff9db84
• Upwork Job ID: 1724832892698370048
• Last Price Increase: 2023-11-22
• Automatic offers:
• barros001 | Contributor | 27984024

[melvin-bot]

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

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~012ade86ce4ff9db84

[melvin-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

[melvin-bot]

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

[barros001]

Proposal

Please re-state the problem that we are trying to solve in this issue.

Certain emojis looks different when displayed on title and description when assigning a task.

What is the root cause of that problem?

The problem is because description, when rendered in the confirm task screen, is rendered using RenderHTML.

App/src/components/MenuItem.js

Lines 276 to 278 in 1eb1f72

	<View style={styles.renderHTMLTitle}>
	<RenderHTML html={getProcessedTitle} />
	</View>

RenderHTML, when setting the font family of a text, only sets to ONE family, as per their documentation

CSS ​font-family and ​font properties allow a comma-separated list of fonts, but React Native styles fontFamily allows only one font name. With the ​TRE, you can instruct which fonts are available in the system (or fonts you have added), and it will pick the first match! The prop to achieve that is ​systemFonts.

This causes the app to drop two emoji related fronts when rendering on the web, causing this inconsistency. Inspecting the title element on the web, we see the following font-family:

    font-family: ExpensifyNeue-Regular, "Segoe UI Emoji", "Noto Color Emoji";

But if you inspect the element on the description, we see this:

    font-family: ExpensifyNeue-Regular;

It did drop the other two fonts, which causes emojis to look different on certain platforms.

What changes do you think we should make in order to solve the problem?

As mentioned above, using only one font is the expected behavior as stated in their documentation. That's why all text nodes rendered by RenderHTML does not have the correct font-family set. To remedy that, I propose we wrap all emojis on the text with <emoji></emoji> tags and then create a custom renderer for this custom tag that will wrap it in a <Text> component, which ensure proper font families are set.

Wrapping emojis with a custom tag can be implemented here:

https://github.com/Expensify/App/blob/1eb1f7259bb8e45c1aad3a040539ce367178090b/src/components/RenderHTML.tsx

Such as:

            // wrap all emojis in <emoji></emoji> tags
            const emojis = _.uniq(html.match(CONST.REGEX.EMOJIS));
            if (emojis) {
                emojis.forEach((emoji) => {
                    html = html.replaceAll(emoji, `<emoji>${emoji}</emoji>`);
                });
            }

And the renderer would look like this:

function EmojiRenderer(props) {
    return <Text>{props.tnode.data}</Text>; // wrapping in <Text> will add the necessary fontFamily to it
}

export default {
    // Standard HTML tag renderers
    a: AnchorRenderer,
    code: CodeRenderer,
    img: ImageRenderer,
    emoji: EmojiRenderer, // new custom renderer

    // Custom tag renderers
    edited: EditedRenderer,
    pre: PreRenderer,
    'mention-user': MentionUserRenderer,
    'mention-here': MentionHereRenderer,
};

Technically this is only need for web so we could (but not necessary) create a web only version of this component and apply this wrapping only for web.

Having this wrapping inside of RenderHTML component has the benefiting of being reusable everywhere (I'm sure this same issue shows up in other places). Also, the wrapping is only made for presentation purposes and parts of the codebase outside of this component won't be aware of it, mitigating the risk of using the wrapped content somewhere else by accident, such as when setting the text to be copied to clipboard.

The new tag is required because we need a tag where the content are always only text. A customer renderer for the <comment> tag would not work because it wraps everything, including markdown tags. The renderer will have to at some point render props.tnode with <TNodeChildrenRenderer tnode={props.tnode} /> which will cause the same problem. With the <emoji> tag (or whatever we want to call it) we can render the raw content with (props.tnode.data) as we're 100% sure it's only text (the emoji).

There's one little problem with this solution as well. The single line code block renderer would need to be updated to look at tnode and if its not a text node (emoji inside of it), then it would have to extract the text from it. This is very simple to achieve and I don't see this being a problem.

Before:

After:

See below for other things tried, but none with success.

What alternative solutions did you explore? (Optional)

There's another way to accomplish this without any complexity at all. The code that verifies whether a font is supported or note, and pick the first one it finds is the the following code:

https://github.com/native-html/core/blob/4c37d2915feb8c071d060b08d54f5ef5fd9c9afc/packages/css-processor/src/validators/LongFontFamilyPropertyValidator.ts#L14-L19

It will call isFontSupported method, which is passed as a configuration, from react-native-render-html. If it returns true, then the font is supported and the first one is selected. false means it's not supported and it will try the next one. There's a 3rd value though, which is if isFontSupported returns a string. If it does, it will assume it's a fallback map and just use it, as is.

Now, here's the isFontSupported function:

https://github.com/meliorence/react-native-render-html/blob/d4e27b67105337bea775033f35eaee26ac683d25/packages/render-html/src/helpers/buildTREFromConfig.ts#L48-L54

It does two things: First it will check if there's a map for the font it's processing (fallback font). If there is, it will return that. Otherwise it will return a true if the font is part of systemFonts and false if not.

Now, if we combine the behavior of both functions above, we can see that we can pass an array of fallback fonts that would map a single font family to a multiple font family and react-native-render-html would take it as is, allowing us to bypass the "single font family" restriction. The code would look like this:

        <TRenderEngineProvider
            customHTMLElementModels={customHTMLElementModels}
            baseStyle={styles.webViewStyles.baseFontStyle}
            tagsStyles={styles.webViewStyles.tagStyles}
            enableCSSInlineProcessing={false}
            systemFonts={_.values(singleFontFamily)}
            fallbackFonts={{
                [singleFontFamily.EXP_NEUE]: multiFontFamily.EXP_NEUE,
                [singleFontFamily.MONOSPACE]: multiFontFamily.MONOSPACE,
            }}
            domVisitors={{
                // eslint-disable-next-line no-param-reassign
                onText: (text) => (text.data = convertToLTR(text.data)),
            }}
        >....</TRenderEngineProvider>

Note the fallbackFonts mapping single font family to multiple font family. The example above only maps two fonts but we can map all of our fonts here. Also, this example is modifying BaseHTMLEngineProvider, which we can't do as it would break native. This is just a quick example to illustrate the idea. The final solution would be to add a new fallbackFonts props to BaseHTMLEngineProvider and pass it only from HTMLEngineProvider/index.js, which only applies to web.

There's one caveat thought (there's always one right??). fallBackFonts is expected to be of type FallbackFontsDefinitions, defined as follows:

export interface FallbackFontsDefinitions {
  monospace: string;
  'sans-serif': string;
  serif: string;
}

That means, technically we're not supposed to pass anything other than the 3 fonts above. BaseHTMLEngineProvider is a pure JS file and would happily take anything (and it works great), but that doesn't mean it's right. If we ever convert this file to TypeScript, it won't be as happy and we would need to force it through with fallbackFonts={{something: 'something-else'} as unknown as FallbackFontsDefinitions}. This small detail might make this solution a non-starter but I thought it would be worth adding it here.

Below is my original alternate proposal:

Another option that does not require wrapping the emojis that could be a better option is to create a platform specific component (for web only) and use the ref callback to manually patch the text nodes, adding the missing font families. The code would look like this:

RenderHTML/index.web.ts

const appendEmojiFontFamily = (wrapper: HTMLElement) => {
    wrapper.querySelectorAll<HTMLElement>('[style*="font-family"]').forEach((e) => {
        if (e.style.fontFamily.includes('Emoji')) {
            return;
        }
        e.style.fontFamily = `${e.style.fontFamily}, ${multiFontFamily.EMOJI}`;
    });
};
function RenderHTML({html}: RenderHTMLProps) {
    const {windowWidth} = useWindowDimensions();
    const htmlRef = useRef('');

    return (
        <View
            ref={(ref) => {
                if (!ref || htmlRef.current === html) {
                    return;
                }

                htmlRef.current = html;

                appendEmojiFontFamily(ref as unknown as HTMLElement);
            }}
        >
            <RenderHTMLSource
                contentWidth={windowWidth * 0.8}
                source={{html}}
            />
        </View>
    );
}

We look for all nodes with font-family styles and if there's no emoji family in it, we manually add the missing families. It keeps track of HTML so it only checks for fonts when it changes to avoid performance issues.

Here's everything else that I tried, without success:

1. Adding missing fonts to systemFonts

App/src/components/HTMLEngineProvider/BaseHTMLEngineProvider.js

Line 73 in 88b1de1

systemFonts={_.values(singleFontFamily)}

systemFonts props tell them renderer engine which fonts are available. The code is currently getting only fonts from the singleFamilyFont, which exclude all emojis (these are for native apps only). I updated the code to grab all fonts from the multiFamilyFont in an attempt to make the html renderer use them. It failed for the reason described at the beginning of this proposal. The renderer will only use the first valid font it finds and discard everything else. That's because ReactNative on native platforms only take one front.

2. Adding a wrapper defining the font

I also tried creating a CommentRenderer (because the content is wrapped in a <comment/> tag. This Renderer would add a <View> wrapper around the entire content setting the font family to the correct value. I then removed the fontFamily definition from the baseStyle props, here:

App/src/styles/styles.ts

Lines 206 to 213 in 88b1de1

	baseFontStyle: {
	color: theme.text,
	fontSize: variables.fontSizeNormal,
	fontFamily: fontFamily.EXP_NEUE,
	flex: 1,
	lineHeight: variables.fontSizeNormalHeight,
	...writingDirection.ltr,
	},

This did what it was supposed to, now we have a wrapper defining a default fontFamily and the text elements don't have their own custom font settings. The problem here is that every <Text> component in ReactNative have their own default font, set to the system defaults. Chrome on Mac for example has this default settings:

font: 14px -apple-system, BlinkMacSystemFont, "Segoe UI", Roboto, Helvetica, Arial, sans-serif;

The means, regardless of adding a wrapper with a fontFamily and removing it from baseStyle, RN will still mess with the plan here and add its own thing, making this approach invalid. There's no clean way to remove this default font definition from RN without using some hacks, which we do not wat to do here.

3. Add a defaultTextprop to the renderer

I tried changed the defaultTextProp, as shown below, to force the engine to always append the expected font-family to ALL text nodes.

https://github.com/Expensify/App/blob/5e01376b6648410c95bcaf0ee487d6e8cde259b2/src/components/HTMLEngineProvider/BaseHTMLEngineProvider.js

function BaseHTMLEngineProvider(props) {
    // We need to memoize this prop to make it referentially stable.
    const defaultTextProps = useMemo(
        () => ({selectable: props.textSelectable, allowFontScaling: false, textBreakStrategy: 'simple', style: {fontFamily: styles.webViewStyles.baseFontStyle.fontFamily}}),
        [props.textSelectable],
    );
....

That did work! But not without problems.

text inside multi-line code blocks are no longer monospaced.

Multi-line code blocks are rendered using <pre> tag which has its own font definition, setting it to a monospaced version of our font. But because the contents can be more than just text, the custom renderer does not use tnode.data, but instead if renders tnode with TDefaultRenderer. That makes all text nodes inside of it get our font defined in the default props, removing the monospaced font from it. This is not acceptable as it would break things.

It did try everything I could think of but there was always a problem.

The ideal solution for this problem is to raise a PR with the react-native-render-html team to let it use multiple fonts. Their premise is that RN on native platforms do not support it, but we now have react-native-web, so it would be great if they would add support. This could take a while and if we don't want to wait, the only way out of this that I could think of was with the wrapping of emojis described above on my main proposal.

Reminder: Please use plain English, be brief and avoid jargon. Feel free to use images, charts or pseudo-code if necessary. Do not post large multi-line diffs or write walls of text. Do not create PRs unless you have been hired for this job.

[barros001]

Proposal

Updated

Added more information to the proposal as well as screenshots of before and after solution.

[5war00p]

Proposal

Please re-state the problem that we are trying to solve in this issue.

The tricky emoji in the title and description seems to be inconsistent

What is the root cause of that problem?

After some deep investigation, I was able to find the real root-cause.

As per the documentation what we did is we provided custom font styles here

App/src/components/HTMLEngineProvider/BaseHTMLEngineProvider.js

Line 73 in e87d35d

systemFonts={_.values(singleFontFamily)}

Here we're only providing the simple font i.e ExpensifyNeue-Regular which doesn't contain the font required to render the fonts.

Ref1:

App/src/styles/fontFamily/multiFontFamily.ts

Line 11 in bf35c29

EXP_NEUE: 'ExpensifyNeue-Regular, Segoe UI Emoji, Noto Color Emoji',

Ref2:

https://github.com/Expensify/App/blob/df377f22e5820e6e524b065360e186f77a8e9625/src/styles/fontFamily/singleFontFamily.ts#L7

The font-family inside Ref2 we're passing it here

App/src/components/HTMLEngineProvider/BaseHTMLEngineProvider.js

Line 70 in e87d35d

baseStyle={styles.webViewStyles.baseFontStyle}

So to put things into perspective, we need to update the font-style we're passing to the RenderHTML component which should contain the font-family of emojis to.

We should also need to update styles according to os which we're doing in multiFontFamily.ts

Even after adding the emoji font-family this issue won't be solved, because the underlying the HTMLEngine is stripping the font-family we're passing and only taking the first font-family we pass.

https://github.com/native-html/core/blob/4c37d2915feb8c071d060b08d54f5ef5fd9c9afc/packages/css-processor/src/validators/LongFontFamilyPropertyValidator.ts#L5-L22

The above code-snippet that is responsible for passing fontFamily css prop to the Text component.

If we observe the code carefully, the code split the font-family with comma , as a separator and check if the font is supported or not.

While checking the font is supported or not, by default every string is supported, we return the string.

Means if we have font-family ExpensifyNeue-Regular, Seoge UI Emoji the code block will just return ExpensifyNeue-Regular which doesn't give the OP we wanted.

https://github.com/native-html/core/blob/4c37d2915feb8c071d060b08d54f5ef5fd9c9afc/packages/css-processor/src/default.ts#L38-L40

What changes do you think we should make in order to solve the problem?

Patch with our App repo.

Fix

Since React Native only supports one font-family, but web-browser does and as this issue only occurs in Windows (tried in mac, but never was able to repro)

We have two function that returns the font value normalizeRawInlineCSSValue and normalizeNativeValue

We can update the code detect if the environment it's running is weather the runtime is browser or application.

If the environment is application we can leave the code as is, or the environment is browser we could simply return the value which will return all the font-family.

What alternative solutions did you explore? (Optional)

NA

Tested on environments like mweb, android app etc, it's working on everywhere.

We're using react-native-render-html in web as well which does built for react-native so this is simply not having web support for the package react-native-render-html Since font-family is the issue we're seeing we can simply have a patch in local.

2023-11-16.02-59-08.mp4