[Hold for payment 2024-08-26] [$250] iOS - Chat - App crashes when adding a space at the end of hyperlink text while editing

[lanitochka17]

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

---

Version Number: 9.0.8-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: https://expensify.testrail.io/index.php?/tests/view/4732111
**Issue reported by:**Applause - Internal Team

Action Performed:

1. Log in to New Expensify
2. Navigate to any chat
3. Send message with any hyperlink, e.g. link.com
4. Edit the message
5. Try to add space at the end of link.com text part of hyperlink

Expected Result:

Space should be appended at the end of hyperlink text

Actual Result:

The app crashes

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

Bug6545344_1721230676639.NVXM4738.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01765448d1b685d840
• Upwork Job ID: 1813943785653464004
• Last Price Increase: 2024-07-25
• Automatic offers:
• DylanDylann | Contributor | 103339219
• dominictb | Contributor | 103353044

Issue Owner

Current Issue Owner: @

Issue Owner

Current Issue Owner: @twisterdotcom

[melvin-bot]

Triggered auto assignment to @twisterdotcom (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

[lanitochka17]

@twisterdotcom FYI I haven't added the External label as I wasn't 100% sure about this issue. Please take a look and add the label if you agree it's a bug and can be handled by external contributors

[lanitochka17]

We think that this bug might be related to #vip-vsp

[twisterdotcom]

Is odd. Going to put this in live markdown though.

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~01765448d1b685d840

[melvin-bot]

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

[allgandalf]

Ouch that was bad, @lanitochka17 does it not happen on android ?

[twisterdotcom]

No, on Android it isn't an issue, although it never saves the extra space.

[allgandalf]

can you put this to daily @twisterdotcom , crashes are really bad, was there a reason for weekly?

[twisterdotcom]

The reason I made it weekly is that it's a very niche flow. We can make it daily, but I don't think it will necessarily help it get done quicker.

[dominictb]

Proposal

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

iOS - Chat - App crashes when adding a space at the end of hyperlink text while editing

What is the root cause of that problem?

In here, we are trimming spaces from both end, so in here, the final converted text will be different from the input markdown, and this will trigger a console.error here.

However, in iOS, we are using bare Hermes runtime (check this) to evaluate the parser script. Unfortunately, according to facebook/hermes#675, the 'console' is not available in the bare Hermes runtime, hence the crash.

To reproduce, we can run the example iOS app in the react-native-live-markdown and follow the OP's step. We can see in the XCode console that the error is something like 'console' property is not defined - JSI evaluation....

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

Simplest solution is to remove console.error here. But if we want to raise the error in the console screen or capture this for investigation purpose, we can try to polyfill the console object, so in case console is undefined, we will do nothing

In here, add

const globalConsole = (globalThis.console ?? null) as Console | null

....
catch (error) {
    globalConsole?.error?.(error);
    // returning an empty array in case of error
    return [];
}

Additionally, we also need to allow trailing spaces on link name. This can be done by following code change in these 2 functions here.

replacement: (_extras, match, g1, g2) => {
    if (!g1.trim()) {
        return match;
    }
    return `<a href="${str_1.default.sanitizeURL(g2)}" target="_blank" rel="noreferrer noopener">${g1}</a>`;
},
rawInputReplacement: (_extras, match, g1, g2) => {
    if (!g1.trim()) {
        return match;
    }
    return `<a href="${str_1.default.sanitizeURL(g2)}" data-raw-href="${g2}" data-link-variant="labeled" target="_blank" 
   rel="noreferrer noopener">${g1}</a>`;
},

We can further extend by only allowing extra trailing spaces in link markdown parsing in react-native-live-markdown's parser by using the extras object from ExpensiMark, just in case we don't want trailing spaces when sending the markdown/html string to the BE.

What alternative solutions did you explore? (Optional)

We can utilize https://www.npmjs.com/package/babel-plugin-transform-globals to replace the console.error or console.log usage, so we can avoid weird implementation as in the main suggested solution.

[melvin-bot]

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

[allgandalf]

I will review the above proposal by EOD

[melvin-bot]

@twisterdotcom, @allgandalf Whoops! This issue is 2 days overdue. Let's get this updated quick!

[allgandalf]

Sorry, this fell through the cracks, i will review the proposal today

[melvin-bot]

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

[muttmuure]

I can't get the auto-assigner to kick in, but I've pinged @mountiny to see if he can merge the PR above

[dominictb]

This should be ready for payment, no?

[twisterdotcom]

Is this for #47199 (comment)? This was deployed to prod 2 days ago, so we should add [Hold for payment 2024-08-26] right?

[melvin-bot]

@twisterdotcom, @Gonals, @mountiny, @DylanDylann, @dominictb Whoops! This issue is 2 days overdue. Let's get this updated quick!

[melvin-bot]

@twisterdotcom, @Gonals, @mountiny, @DylanDylann, @dominictb 6 days overdue. This is scarier than being forced to listen to Vogon poetry!

[twisterdotcom]

I am OOO and will be back for payment on Thurs.

[melvin-bot]

@twisterdotcom, @Gonals, @mountiny, @DylanDylann, @dominictb Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[DylanDylann]

@twisterdotcom Kindly bump on payment

[melvin-bot]

@twisterdotcom, @Gonals, @mountiny, @DylanDylann, @dominictb Still overdue 6 days?! Let's take care of this!

[twisterdotcom]

Sorry this slipped through the cracks. Payment Summary:

• @DylanDylann paid $250 via Upwork here (C+)
• @dominictb paid $250 via Upwork here (Contributor)