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:
- Log into staging.new.expensify.com
- Click into any chat
- Type the word
[block]and send it as a message - Click the pencil icon to edit the comment
- 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
Expensify/Expensify Issue URL: Issue reported by: @eh2077 Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1681920140787049
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)
@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 theh1replacing 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
The PR that introduced the bug has been identified. Link to the PR: Not a bug but a known drawback of the implementation.
[@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.
[@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_SEPARATORinside the constructor of ExpensiMark, likeAnd then replace all
[block]withthis.BLOCK_SEPARATOR.The
BLOCK_SEPARATORwill look likeblockc41de41b-2767-617b-b78d-49ca52a0fb0cwhich 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?