App: [HOLD for payment 2023-06-09] [$1000] Composer input is empty when editing the word `[block]` in chat

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. Log into staging.new.expensify.com
  2. Click into any chat
  3. Type the word [block] and send it as a message
  4. Click the pencil icon to edit the comment
  5. Notice the composer input is empty

Expected Result:

The composer input should be text [block] in editing mode

Actual Result:

The composer input is empty

Platforms:

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

  • Android / native
  • Android / Chrome
  • iOS / native
  • iOS / Safari
  • MacOS / Chrome / Safari
  • MacOS / Desktop

Version Number: 1.3.3 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: Any additional supporting documentation

https://user-images.githubusercontent.com/43996225/233535465-20d83e6a-1279-4ddd-8a77-aaab3ebd04b6.mp4

https://user-images.githubusercontent.com/43996225/233535477-bc59a995-b10c-4ad7-9b52-88df4ffadbcb.mov

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

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~01a0c0a4619ff68e29
  • Upwork Job ID: 1650565300774252544
  • Last Price Increase: 2023-04-24

About this issue

  • Original URL
  • State: closed
  • Created a year ago
  • Comments: 89 (67 by maintainers)

Most upvoted comments

@cead22 @rushatgabhane Actually I find this way which can save us all the trouble of inventing a random string.

The [block] is currently used basically to represent a group of HTML tags that, when converted to markdown, should add a new line, see https://github.com/Expensify/expensify-common/blob/afe46976cbc7c5966118a32ae2e3737109d849b4/lib/ExpensiMark.js#L545. And then, we split the htmlString by [block] in here https://github.com/Expensify/expensify-common/blob/afe46976cbc7c5966118a32ae2e3737109d849b4/lib/ExpensiMark.js#L493 for replacing with new lines.

We can remove [block] completely as an intermediary, and split the string directly by using the regex in https://github.com/Expensify/expensify-common/blob/afe46976cbc7c5966118a32ae2e3737109d849b4/lib/ExpensiMark.js#L545 There’ll be a few more small changes required like updating the h1 replacing logic, stripHTML for an array of strings that are already split, but it will work.

@cead22 @rushatgabhane if there’s interest in this solution, I’m happy to create a full proposal for it. It might take more changes but will solve the problem fully and avoid any random string to be used.

This is paid by Anu, and can be closed.

Hey @maddylewis , Anu is setting up global payments through expensify for us. It might take another week or two, and we’ll have the clarity of next steps for payment.

Until then, we could make this a weekly issue? I hope this is okay 😃

https://expensify.slack.com/archives/C02NK2DQWUX/p1687451792686509?thread_ts=1686871517.189609&cid=C02NK2DQWUX

Checklist

  1. The PR that introduced the bug has been identified. Link to the PR: Not a bug but a known drawback of the implementation.

  2. [@rushatgabhane] 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: not a bug.

  3. [@rushatgabhane] 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: [@rushatgabhane]: NA

4.Determine if we should create a regression test for this bug. NO

@rushatgabhane can you please complete the list here so that I can issue your payment? Thanks!

apologies! sending offers now!

The PR on expensify-common is merged. The PR in App should be ready soon, it just needs to be re-tested on the latest commit of expensify-common

communicating here that i am OOO May 11 and 12. i am adding the BZ label in case this issue needs to be actioned while I’m away, but will keep myself assigned / remove the other assignee once i return from OOO.

I found another possible solution to fix this issue. We can generate a id as the block separator instead of having a fixed one.

To achieve this we can initialise a separator this.BLOCK_SEPARATOR inside the constructor of ExpensiMark, like

export default class ExpensiMark {
    constructor() {
        /**
         * We generate a special id as the separator between markdown block elements to avoid 
         * conflicting with user input.
         */
        this.BLOCK_SEPARATOR = Str.guid('block');

And then replace all [block] with this.BLOCK_SEPARATOR.

The BLOCK_SEPARATOR will look like blockc41de41b-2767-617b-b78d-49ca52a0fb0c which won’t conflict with user input.

@cead22 @rushatgabhane I’ll be happy to hear your feedback about this new solution.

@maddylewis yes this can be fixed externally, so you can apply the label. Can you update the issue description to make it extra clear what the steps to reproduce should be, and remove any boilerplate info from there?