[HOLD for payment 2022-05-20][$250] When we enter 4 backticks and send it sends as two empty block, and when switched the chat, it disappears - reported by @mdneyazahmad

[mvtglobally]

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. Send 4 back ticks as message.
2. Switch to another chat
3. Back to the same chat

Expected Result:

The sent message disappears.

Actual Result:

Either we should not allow to send empty message or keep it as it is sent.

Workaround:

unknown

Platform:

Where is this issue occurring?

• Web
• iOS
• Android
• Desktop App
• Mobile Web

Version Number: 1.1.41-0
Reproducible in staging?: Y
Reproducible in production?: Y
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos: Any additional supporting documentation
Potentially related to #7965

Screen.Recording.2022-02-17.at.2.15.36.PM.mov

Record_2022-02-21-18-53-48.mp4

Expensify/Expensify Issue URL:
Issue reported by: @mdneyazahmad
Slack conversation: https://expensify.slack.com/archives/C01GTK53T8Q/p1645088209701899

View all open jobs on GitHub

[MelvinBot]

Triggered auto assignment to @lschurr (AutoAssignerTriage), see https://stackoverflow.com/c/expensify/questions/4749 for more details.

[MelvinBot]

Triggered auto assignment to @bondydaa (Engineering), see https://stackoverflow.com/c/expensify/questions/4319 for more details.

[lschurr]

Not sure if this is a high priority.. adding eng to investigate.

[bondydaa]

hmm yeah probably something with the markdown parser, I'd guess this is fine to work on for external

[MelvinBot]

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

[MelvinBot]

Triggered auto assignment to Contributor-plus team member for initial proposal review - @Santhosh-Sellavel (Exported)

[MelvinBot]

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

[K4tsuki]

proposal

The problem is in Report_AddComment Api request, when the content of the backtick is empty or white space only, the API request fail with message No content to add.

My solution, In:

https://github.com/Expensify/expensify-common/blob/f77bb4710c13d01153716df7fb087b637ba3b8bd/lib/ExpensiMark.js#L48-L53

Add check for $2, whether it is match(/^\s*$/) => whether content is empty or white space only.
If the match true we could prepend the $2 with zero-width-space character ​ This character will not change the visual display of the message.

Result after sending the API request:

ss2.mp4

Another solution for white space only characters inside backtick is to replace matching \s regex with corresponding html hex entities. List of white space characters:

https://stackoverflow.com/a/46637343

[mdneyazahmad]

Proposal

https://github.com/Expensify/expensify-common/blob/f77bb4710c13d01153716df7fb087b637ba3b8bd/lib/ExpensiMark.js#L52

When number of back tick is in 2n. It has 2n pair of <code></code> and text content for this html will be empty string. When we send this message it is optimistically added to the message list and the api request is made that produces an error. The reason it removes when tab is switched is that removeOptimisticActions is called when we switch the chat and it removes any pending messages.

Solution:

Do not consider the text as code until it is strictly a single back tick start and close and it has some text inside.

Example: `` will not be consider as inline code unless it has some content. Also, when 3, 4, or 5 backticks. Only `back tick` followed by some text content then `back tick` will be valid.

Or else the text will be send as plain text without format.

-                regex: /(\B|_)&#x60;(.*?)&#x60;(\B|_)(?![^<]*<\/pre>)/g,
+                regex: /(\B|_)(?<!&#x60;)&#x60;(?!&#x60;)(.+)(?<!&#x60;)&#x60;(?!&#x60;)(\B|_)(?![^<]*<\/pre>)/g,

[bfitzexpensify]

@Santhosh-Sellavel please take a look at these ^ proposals when you get a chance - thanks!

[Santhosh-Sellavel]

@AndrewGable

@K4tsuki proposal #7971 (comment)
Solves the issue but this looks like a hack or workaround to me.

@mdneyazahmad proposal also has addresses the issue, but lookbehinds were used in regex.
I see here in the comment look behinds are not supported in mobile platforms.

Please post your thoughts here, Thanks!

[frenkield]

Proposal (updated)

If I understand correctly, the goal is to address this issue in the front end code by disabling the send button for certain problematic messages containing only backticks and whitespace.

Task - Disable send button for problematic backtick sequences

The situation is a little different from Slack where triple-backticks elicit the creation of a secondary text editor. So on Slack a message containing only triple-backticks and whitespace is not valid. However, other sequences of ticks (e.g. 2 backticks and whitespace) are considered valid. On Expensify, on the other hand, a message containing just 2 backticks and whitespace should not be considered valid, as it causes an error on the back end and some weird message rendering.

So a straightforward solution is to update ReportActionCompose.updateComment() so that a comment is considered invalid if it contains only backticks and whitespace.

Note that this is more restrictive than Slack. This validation code could go in ReportActionCompose or in ExpensiMark or in some utility class.

The quickest way to resolve the issue is by changing this line of code:

App/src/pages/home/report/ReportActionCompose.js

Line 307 in ad637d3

isCommentEmpty: newComment.trim().length === 0,

This line is executed after any update to the comment text field. And when isCommentEmpty is true the send button is disabled. At the moment isCommentEmpty evaluates to true when the comment contains only whitespace (i.e. when isCommentEmpty.trim() is true).

So instead of trim() we can use a regular expression with the match() function to determine if the comment contains only whitespace and backticks:

isCommentEmpty: !!newComment.match(/^(\s|`)*$/)

[Santhosh-Sellavel]

@frenkield
The task 2 is already being handled in another PR, it will be fixed soon.

Can you elaborate more on the fix for this issue. What changes are you proposing?

[frenkield]

Ok thanks, @Santhosh-Sellavel. I'll remove task 2 from the proposal.

The issue can be resolved by changing this line of code:

App/src/pages/home/report/ReportActionCompose.js

Line 307 in ad637d3

isCommentEmpty: newComment.trim().length === 0,

This line is executed after any update to the comment text field. And when isCommentEmpty is true the send button is disabled. At the moment isCommentEmpty evaluates to true when the comment contains only whitespace (i.e. when isCommentEmpty.trim() is true).

So instead of trim() we can use a regular expression with the match() function to determine if the comment contains only whitespace and backticks:

isCommentEmpty: !!newComment.match(/^(\s|`)*$/)

[Santhosh-Sellavel]

Will review this tomorrow!

[Santhosh-Sellavel]

Discussing this one!

[Santhosh-Sellavel]

@AndrewGable
@frenkield proposal LGTM!
🎀👀🎀 C+ reviewed

Thanks!

[melvin-bot]

📣 @frenkield You have been assigned to this job by @AndrewGable!
Please apply to this job in Upwork and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
Keep in mind: Code of Conduct | Contributing 📖

[frenkield]

Thanks for assigning me this ticket.

I submitted the proposal on Upwork (https://www.upwork.com/jobs/~012ef02def10429641). I'm planning to submit the pull request by Monday, May 2nd.

And sorry, I put the wrong delivery date in the Upwork proposal. It should be Monday, May 2nd.

[Santhosh-Sellavel]

Cool, thanks!

[frenkield]

Hi All. I created a draft pull request. Can I submit it for final review or do I need to wait for approval?

[Santhosh-Sellavel]

You need to submit your PR to get reviewed @frenkield!

[Santhosh-Sellavel]

@bfitzexpensify Any update on payment?

[bfitzexpensify]

Paid out @frenkield - @Santhosh-Sellavel, once you accept the offer here I'll wrap that up too. Same for @mdneyazahmad.

[bfitzexpensify]

OK great, all done here!

[tgolen]

I found this GH while researching #13854.

It looks like the solution was to disable the sending of the messages based on Slack functionality (@AndrewGable ):

Ok I think the ideal behavior is to copy slack here, when the code block is empty the send button should be disabled.

However, I just checked Slack, and it let's me send a message with empty code blocks as well as one and two backticks (any number). So, I think I'm going to look into reverting this change and looking into the backend issues instead

[Santhosh-Sellavel]

@tgolen I can't send empty blocks still on slack

Screen.Recording.2023-01-03.at.4.17.04.AM.mov

[tgolen]

Ah yes, I was mistaken about the full code blocks, but you can send empty inline code blocks (ie. two backticks) as well as single backticks.