[HOLD for payment 2024-10-24] [$250] Room - Code block mark down after a space is not applied

[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.41-2
Reproducible in staging?: Y
Reproducible in production?: Y
If this was caught during regression testing, add the test name, ID and link from TestRail: N/A
Issue reported by: Applause - Internal Team

Action Performed:

1. Launch app
2. Open a room chat
3. Tap header -- room description
4. Enter and tap save
5. Tap description
6. Just add a space after description and tap save
7. Note code block markdown can be seen
8. Tap plus icon -- assign task
9. Enter title
10. Enter description and tap next
11. Tap description and add a space and save
12. Note now code block mark down not applied

Expected Result:

Code block mark down must be applied after a space

Actual Result:

Code block mark down after a space applied in room description but not in task description

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

Bug6617983_1727482223916.Screenrecorder-2024-09-28-05-29-03-760_compress_1.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~021843721884804343746
• Upwork Job ID: 1843721884804343746
• Last Price Increase: 2024-10-08
• Automatic offers:
• ZhenjaHorbach | Reviewer | 104341648

Issue Owner

Current Issue Owner: @twisterdotcom / @abekkala

[melvin-bot]

Triggered auto assignment to @abekkala (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]

@abekkala 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

[bernhardoj]

Proposal

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

Editing an inline code block with space-only content will remove all the spaces inside it.

What is the root cause of that problem?

In #48101, we allow spaces only inside an inline code block, for example:

`  `

We do that by replacing the whitespace with &nbsp.

In the new task description page, the saved description will be converted to the HTML first and then converted back to the markdown.

App/src/pages/tasks/NewTaskDescriptionPage.tsx

Line 80 in d187665

defaultValue={Parser.htmlToMarkdown(Parser.replace(task?.description ?? ''))}

`  ` -> <code>&nbsp;&nbsp;</code> -> `  ` (the space here is &nbsp;, non breaking space)

However, when submitting the form, the description value here contains the inline code block with empty content, all the   inside the content (``) are removed.

App/src/pages/tasks/NewTaskDescriptionPage.tsx

Lines 43 to 44 in d187665

	const onSubmit = (values: FormOnyxValues<typeof ONYXKEYS.FORMS.NEW_TASK_FORM>) => {
	TaskActions.setDescriptionValue(values.taskDescription);

That's because of the prepareValues here.

App/src/components/Form/FormProvider.tsx

Line 168 in d187665

const trimmedStringValues = shouldTrimValues ? ValidationUtils.prepareValues(inputValues) : inputValues;

In prepareValues, we are removing all invisible characters.

App/src/libs/ValidationUtils.ts

Line 460 in d187665

trimmedStringValues[inputID] = StringUtils.removeInvisibleCharacters(inputValue);

And   is considered as an invisible character.

App/src/libs/StringUtils.ts

Lines 35 to 39 in d187665

	// Remove spaces:
	// - \u200B: zero-width space
	// - \u00A0: non-breaking space
	// - \u2060: word joiner
	result = result.replace(/[\u200B\u00A0\u2060]/g, '');

Why this doesn't happen on the room description page? That's because, on the room description page, we manually managed the input state. So, when submitting, we use the state value instead of the form provider values.

App/src/pages/RoomDescriptionPage.tsx

Line 43 in d187665

const [description, setDescription] = useState(() => Parser.htmlToMarkdown(report?.description ?? ''));

App/src/pages/RoomDescriptionPage.tsx

Lines 57 to 63 in d187665

	const submitForm = useCallback(() => {
	const previousValue = report?.description ?? '';
	const newValue = description.trim();
	Report.updateDescription(report.reportID, previousValue, newValue);
	goBack();
	}, [report.reportID, report.description, description, goBack]);

If we use the form passed values, then the issue will happen too.

const newValue = values.reportDescription.trim();

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

Since we replace whitespace with   when converting markdown to HTML, we need to replace   with whitespace when converting from HTML to markdown.
https://github.com/Expensify/expensify-common/blob/f11184e88c2e211c3a9423d8c87291b44722576e/lib/ExpensiMark.ts#L638-L640

replacement: (_extras, _match, _g1, g2) => `\`${g2.replaceAll(' ', ' ')}\``,

What alternative solutions did you explore? (Optional)

I think we shouldn't include   as the invisible character because the character is totally visible. I think the reason we remove it is explained by these comments:

App/src/libs/StringUtils.ts

Lines 41 to 50 in d187665

	// Temporarily replace all newlines with non-breaking spaces
	// It is necessary because the next step removes all newlines because they are in the (Cc) category
	result = result.replace(/\n/g, '\u00A0');
	// Remove all characters from the 'Other' (C) category except for format characters (Cf)
	// because some of them are used for emojis
	result = result.replace(/[\p{Cc}\p{Cs}\p{Co}\p{Cn}]/gu, '');
	// Replace all non-breaking spaces with newlines
	result = result.replace(/\u00A0/g, '\n');

\p{Cc} matches the new line too, so to prevent replacing the new line with an empty string, we replace all new lines to nbsp first, and then convert all nbsp back to the new line. An alternative way to prevent removing the new line is by splitting the string by new line first, doing the replacement, and then join the strings back.

result = result.split("\n").map((part) => part.replace(/[\p{Cc}\p{Cs}\p{Co}\p{Cn}]/gu, '')).join("\n");

This way, we don't need to remove the nbsp.

App/src/libs/StringUtils.ts

Lines 35 to 39 in d187665

	// Remove spaces:
	// - \u200B: zero-width space
	// - \u00A0: non-breaking space
	// - \u2060: word joiner
	result = result.replace(/[\u200B\u00A0\u2060]/g, '');

result = result.replace(/[\u200B\u2060]/g, '');
result = result.split("\n").map((part) => part.replace(/[\p{Cc}\p{Cs}\p{Co}\p{Cn}]/gu, '')).join("\n");

[melvin-bot]

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

[melvin-bot]

@abekkala Eep! 4 days overdue now. Issues have feelings too...

[melvin-bot]

@abekkala Now this issue is 8 days overdue. Are you sure this should be a Daily? Feel free to change it!

[melvin-bot]

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

[melvin-bot]

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

[ZhenjaHorbach]

@bernhardoj
Thanks for your proposal !
Your solutions looks good
But I like the alternative solution a little better

🎀👀🎀 C+ reviewed

[melvin-bot]

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

[melvin-bot]

📣 @ZhenjaHorbach 🎉 An offer has been automatically sent to your Upwork account for the Reviewer role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job

[bernhardoj]

PR is ready

cc: @ZhenjaHorbach

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

[abekkala]

@twisterdotcom I'll be ooo until Mon Oct 21
Given that the PR has not yet deployed to prod starting the 7 day waiting period yet I don't expect this to come to payment before I return from ooo on Oct 21.

STATUS: proposal selected, PR created but not yet deployed to production

Payment Summary [date, TBD]:

• Fix: @bernhardoj [$250, if no regressions] payment made via NewDot
• PR Review: @ZhenjaHorbach [$250, if no regressions] OFFER

[melvin-bot]

Reviewing label has been removed, please complete the "BugZero Checklist".

[melvin-bot]

The solution for this issue has been 🚀 deployed to production 🚀 in version 9.0.49-2 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• Fix nbsp is removed from string when submitting form #50539

If no regressions arise, payment will be issued on 2024-10-24. 🎊

For reference, here are some details about the assignees on this issue:

• @bernhardoj requires payment through NewDot Manual Requests
• @ZhenjaHorbach requires payment automatic offer (Reviewer)

[melvin-bot]

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

• [@ZhenjaHorbach] The PR that introduced the bug has been identified. Link to the PR:
• [@ZhenjaHorbach] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment:
• [@ZhenjaHorbach] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion:
• [@ZhenjaHorbach] Determine if we should create a regression test for this bug.
• [@ZhenjaHorbach] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.
• [@twisterdotcom / @abekkala] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[abekkala]

I'm back from ooo - unassigning @twisterdotcom

[abekkala]

PAYMENT SUMMARY FOR OCT 24

• Fix: @bernhardoj [$250, if no regressions] payment via NewDot
• PR Review: @ZhenjaHorbach [$250, if no regressions] Offer
  please complete checklist

[bernhardoj]

Requested in ND.

[abekkala]

@ZhenjaHorbach, can you complete the checklist above so that I can issue payment today? Thanks!

[ZhenjaHorbach]

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

• [@ZhenjaHorbach] The PR that introduced the bug has been identified. Link to the PR:

Expensify/expensify-common#797

• [@ZhenjaHorbach] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment:

https://github.com/Expensify/expensify-common/pull/797/files#r1814977410

• [@ZhenjaHorbach] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion:

NA

• [@ZhenjaHorbach] Determine if we should create a regression test for this bug.
• [@ZhenjaHorbach] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.

• Open any chat
• Press + and select Assign task
• Enter any title
• Enter an inline code block markdown with spaces only as the content for the description ( )
• Press Next
• Press Description to edit it
• Add a space after the inline code block
• Save it
• Verify the description is still an inline code block

[ZhenjaHorbach]

@ZhenjaHorbach, can you complete the checklist above so that I can issue payment today? Thanks!

@abekkala

Thanks for the reminder !
Done !