App: [HOLD for payment 2023-03-31] [$4000] Adding the 2 line break in message that is inside the code block with header text is hiding the edited messaged

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


Action Performed:

  1. Go to any chat
  2. Send a code block message with h1 text inside (e.g. # hello)
  3. Edit the above message and add 2 line break in the message
  4. Check the Message and the Observe the result

Expected Result:

Message should not be hidden and it should display edited message

Actual Result:

Message will be hidden after a second

Workaround:

unknown

Platforms:

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

  • iOS / native

Version Number: 1.2.74-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 Notes/Photos/Videos:

https://user-images.githubusercontent.com/43996225/220152930-bd1234a7-3ea3-44a7-b6e8-124eedd74e75.mp4

https://user-images.githubusercontent.com/43996225/220152932-ed5b9a3a-c049-48cf-b9a7-4a7e560bf2dd.mp4

https://user-images.githubusercontent.com/43996225/220152944-819f42d3-0990-4d5f-ae53-c06811438308.mov

Expensify/Expensify Issue URL: Issue reported by: @harshad2711 Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1676710142677019

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~01a0e20796efbb701d
  • Upwork Job ID: 1628571831573925888
  • Last Price Increase: 2023-03-09

About this issue

  • Original URL
  • State: closed
  • Created a year ago
  • Comments: 57 (29 by maintainers)

Most upvoted comments

@ntdiary Sorry I got confused because there were no explanations about your code diff.

This is your proposal regex And I thought this was same as before https://github.com/Expensify/expensify-common/pull/496

This is latest code in expensify-common repo: regex

Original: /<(pre)(?:"[^"]*"|'[^']*'|[^'">])*>([\s\S]*?)<\/\1>(?![^<]*(<\/pre>|<\/code>))/gi, After https://github.com/Expensify/expensify-common/pull/496: /<(pre)(?:"[^"]*"|'[^']*'|[^'">])*>(\n?)([\s\S]*?)(\n?)<\/\1>(?![^<]*(<\/pre>|<\/code>))/gi, Your proposal: /<(pre)(?:"[^"]*"|'[^']*'|[^'">])*>([\s\S]*?)(\n?)<\/\1>(?![^<]*(<\/pre>|<\/code>))/gi,

So what I got confused is that Original = Your proposal because it partially reverts https://github.com/Expensify/expensify-common/pull/496 and I thought you proposed based on old codebase before https://github.com/Expensify/expensify-common/pull/496

This works fine on production. Also, your regex is deprecated. You are not supposed to update regex in your proposal, right?

@aimane-chnaif, I don’t understand what you mean. Why my regex is deprecated? and I shouldn’t update the regex in my proposal? What I recommend in my proposal is the rule from this comment.

In subsequent communications, @deetergp said that the extra line break after the text is eliminated when he edited the message again with my proposal. Then I uploaded a video to show that it worked and hoped he would share his case. Before he replied again, It occurred to me that it was probably because he had only modified the replacement and not the regex. I thought that event though I had listed the links to that comment and PR, I should take responsibility for not make it clear directly that the regex should be reverted to the version in that comment. So I said sorry to him and provided the regex directly.

IMO, the merged PR should not eliminate the line break after the <pre>. Because both the original message (in the input box) and the html rendered from it(after sending) obviously had extra line break before the text. So I said that his original proposal is fine (The original is meant to be relative to the version in his subsequent PR, not the first version of that comment).

Personally, I think that this issue is just that the conversion rules are complicated. maybe I’m wrong : ). If you feel that the other proposals are better or more sensible, please feel free to choose them. I just hope there are no misunderstandings in our communication (I’m still learning to communicate in English, I’ve learned a lot in this community over the past year, and also thanks for DeepL and Google Translator).

I am checking & testing various cases now. Will update soon

@deetergp thank you very much, have updated my proposal.🙂

I don’t know why it is acting differently today, since what I did yesterday is essentially what you just linked @aimane-chnaif, but today it’s behaving the way described here in this GH. In any event…

I’m not sure if the backend acts for any other purpose, maybe we could push the same content instead of empty data if the comment remains the same?

I think that’s probably true, @ntdiary, since simply returning early here blanks out the content of the comment, which is not the desired outcome, but I think that is also a separate bug.

when we save changes , the above condition is false, so the code isn’t interrupted, and then backend will push a empty html data to frontend. (parsedOriginalCommentHTML is <pre>#&#32;test<br /><br /></pre>, and htmlForNewComment.trim() is <pre><br />#&#32;test<br /><br /></pre>)

But isn’t the issue here that htmlForNewComment.trim() ought to be <pre><br /><br />#&#32;test<br /><br /></pre> if we are indeed intending on rendering what the client intends to display? And in this instance, shouldn’t parsedOriginalCommentHTML be <pre><br />#&#32;test<br /><br /></pre>?

I think issue description needs updated a bit. This happens on all text (not only header text) inside code block.

The weird thing in this video is that when refresh Report page (either go to another tab or to switch chat and come back), disappeared message shows again.

https://user-images.githubusercontent.com/96077027/221652256-45ab606d-b60d-4241-895f-fd2d0110cd3c.mov