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

@twisterdotcom

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: 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:

Launch app
Open a room chat
Tap header -- room description
Enter and tap save
Tap description
Just add a space after description and tap save
Note code block markdown can be seen
Tap plus icon -- assign task
Enter title
Enter description and tap next
Tap description and add a space and save
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?

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

@abekkala

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.

@abekkala

@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

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('&nbsp;', ' ')}\``,

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");

@abekkala

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

@abekkala

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

@abekkala

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

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

@ZhenjaHorbach

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

@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:
[@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:

@twisterdotcom

I'm back from ooo - unassigning @twisterdotcom

@bernhardoj

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

Requested in ND.

@ZhenjaHorbach

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

@ZhenjaHorbach

@ZhenjaHorbach payment sent and contrct ended - thank you! 🎉

@bernhardoj

Pasting summary once more as it got a little buried above:

PAYMENT SUMMARY FOR OCT 24

Fix: @bernhardoj [$250] payment via NewDot
PR Review: @ZhenjaHorbach [$250] - paid 🎉

@carlosmiceli

@carlosmiceli @abekkala Be sure to fill out the Contact List!

@bernhardoj

$250 approved for @bernhardoj

Navigation Menu

Repository navigation

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

Action Performed:

Expected Result:

Actual Result:

Workaround:

Platforms:

Screenshots/Videos

Proposal

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

What is the root cause of that problem?

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

What alternative solutions did you explore? (Optional)

29 remaining items

PAYMENT SUMMARY FOR OCT 24

PAYMENT SUMMARY FOR OCT 24

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

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

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

	const submitForm = useCallback(() => {
	const previousValue = report?.description ?? '';
	const newValue = description.trim();

	Report.updateDescription(report.reportID, previousValue, newValue);
	goBack();
	}, [report.reportID, report.description, description, goBack]);

	// 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');

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

Description

Action Performed:

Expected Result:

Actual Result:

Workaround:

Platforms:

Screenshots/Videos

Activity

melvin-bot commented on Sep 28, 2024

lanitochka17 commented on Sep 28, 2024

bernhardoj commented on Sep 30, 2024

Proposal

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

What is the root cause of that problem?

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

What alternative solutions did you explore? (Optional)

melvin-bot commented on Oct 2, 2024

melvin-bot commented on Oct 4, 2024

melvin-bot commented on Oct 8, 2024

melvin-bot commented on Oct 8, 2024

melvin-bot commented on Oct 8, 2024

29 remaining items

melvin-bot commented on Oct 17, 2024

abekkala commented on Oct 21, 2024

abekkala commented on Oct 21, 2024

PAYMENT SUMMARY FOR OCT 24

bernhardoj commented on Oct 21, 2024

abekkala commented on Oct 24, 2024

ZhenjaHorbach commented on Oct 24, 2024

ZhenjaHorbach commented on Oct 24, 2024

abekkala commented on Oct 24, 2024

abekkala commented on Oct 24, 2024

PAYMENT SUMMARY FOR OCT 24

melvin-bot commented on Oct 24, 2024

JmillsExpensify commented on Oct 24, 2024

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions