Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[$250] Mark down - The sent message containing two > does not display double quote mark down #45154

Open
6 tasks done
lanitochka17 opened this issue Jul 10, 2024 · 48 comments
Open
6 tasks done
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 Engineering External Added to denote the issue can be worked on by a contributor

Comments

@lanitochka17
Copy link

lanitochka17 commented Jul 10, 2024

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.6-0
Reproducible in staging?: Y
Reproducible in production?: N
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:

  1. Go to staging.new.expensify.com
  2. Go to chat
  3. Type >> hello
  4. Note that the live mark down displays the nested quote
  5. Send the message

Expected Result:

The sent message containing two > will display double quote mark down

Actual Result:

The sent message containing two > does not display double quote mark down

Workaround:

Unknown

Platforms:

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

  • Android: Native
  • Android: mWeb Chrome
  • iOS: Native
  • iOS: mWeb Safari
  • MacOS: Chrome / Safari
  • MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

Bug6537871_1720617437950.double_quote.mp4

View all open jobs on GitHub

Issue OwnerCurrent Issue Owner: @Skalakid
Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~021856771063028418492
  • Upwork Job ID: 1856771063028418492
  • Last Price Increase: 2024-11-13
@lanitochka17 lanitochka17 added DeployBlockerCash This issue or pull request should block deployment DeployBlocker Indicates it should block deploying the API labels Jul 10, 2024
Copy link

melvin-bot bot commented Jul 10, 2024

Triggered auto assignment to @pecanoro (DeployBlockerCash), see https://stackoverflowteams.com/c/expensify/questions/9980/ for more details.

Copy link
Contributor

👋 Friendly reminder that deploy blockers are time-sensitive ⏱ issues! Check out the open `StagingDeployCash` deploy checklist to see the list of PRs included in this release, then work quickly to do one of the following:

  1. Identify the pull request that introduced this issue and revert it.
  2. Find someone who can quickly fix the issue.
  3. Fix the issue yourself.

@lanitochka17
Copy link
Author

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

@lanitochka17
Copy link
Author

We think that this bug might be related to #vip-vsp

@devguest07
Copy link
Contributor

Caused by #44262

@pecanoro
Copy link
Contributor

Confirmed and posted in the PR, we probably need to assign the people to fix it!

@pecanoro
Copy link
Contributor

Oh, but maybe we don't need to hold on this as it only happens when you do >> which it's an edge case

@mountiny
Copy link
Contributor

Yeah lets not hold on this, we have to get to higher version of the markdown library and this does not seem that big issue

Please feel free to reassign to me, happy to handle this with swm or community upstream

@mountiny mountiny added Daily KSv2 and removed DeployBlockerCash This issue or pull request should block deployment Hourly KSv2 DeployBlocker Indicates it should block deploying the API labels Jul 10, 2024
@mountiny
Copy link
Contributor

cc @Skalakid @BrtqKr @BartoszGrajdek @tomekzaw

@BartoszGrajdek
Copy link
Contributor

I think @BrtqKr will be the best person to help you here since he was the one working on block quotes 🙌🏻

@BrtqKr
Copy link
Contributor

BrtqKr commented Jul 11, 2024

Blockquote requires a space following it. If there are two blockquotes, they still need to be separated with a space. This was meant to allow empty lines to be treated as a part of the blockquote and prevent some of the issues with treating '>' as a blockquote unintentionally. I think this should go the other way - the input shouldn't show that this would result in a blockquote.

@pecanoro
Copy link
Contributor

@BrtqKr Let me know if I am following correctly, the problem here is that the composer should not show the preview of the markdown, right?

@pecanoro pecanoro added the Bug Something is broken. Auto assigns a BugZero manager. label Jul 11, 2024
Copy link

melvin-bot bot commented Nov 19, 2024

@JmillsExpensify, @pecanoro, @Pujan92, @BrtqKr Eep! 4 days overdue now. Issues have feelings too...

@melvin-bot melvin-bot bot added the Overdue label Nov 19, 2024
@Skalakid
Copy link
Contributor

Hello, Here is the draft PR with the fix. I changed the blockquote parsing logic and now I'm working on changing html to markdown conversion algorithm

@melvin-bot melvin-bot bot removed Help Wanted Apply this label when an issue is open to proposals by contributors Overdue labels Nov 19, 2024
@Skalakid
Copy link
Contributor

Hello, today I continued fixing HTML to markdown parsing logic. Tomorrow I will clean up the code and push the changes to the PR so we can start merging it

@melvin-bot melvin-bot bot added the Overdue label Nov 21, 2024
@pecanoro pecanoro removed the Overdue label Nov 22, 2024
@melvin-bot melvin-bot bot added the Overdue label Nov 22, 2024
Copy link

melvin-bot bot commented Nov 25, 2024

@JmillsExpensify, @pecanoro, @Pujan92, @Skalakid, @BrtqKr Huh... This is 4 days overdue. Who can take care of this?

@pecanoro
Copy link
Contributor

@Skalakid Any recent updates?

@Skalakid
Copy link
Contributor

Hi, yes, I'm adding last touches to my PR and will handle it to the review today

@melvin-bot melvin-bot bot removed the Overdue label Nov 26, 2024
@Skalakid
Copy link
Contributor

The PR is ready! Can you assign a reviewer there, please?

@pecanoro
Copy link
Contributor

@Pujan92 Do you want to review the PR? Since you were assigned here originally as C+

@pecanoro
Copy link
Contributor

@Pujan92 It is not letting me assigned you so you probably need to comment in the PR

@Pujan92
Copy link
Contributor

Pujan92 commented Nov 28, 2024

@pecanoro Yes, I will review it tomorrow.

Copy link

melvin-bot bot commented Dec 2, 2024

@JmillsExpensify, @pecanoro, @Pujan92, @Skalakid, @BrtqKr Huh... This is 4 days overdue. Who can take care of this?

@melvin-bot melvin-bot bot added the Overdue label Dec 2, 2024
@Skalakid
Copy link
Contributor

Skalakid commented Dec 2, 2024

PR is waiting for the C+ review

@melvin-bot melvin-bot bot removed the Overdue label Dec 2, 2024
Copy link

melvin-bot bot commented Dec 5, 2024

@JmillsExpensify, @pecanoro, @Pujan92, @Skalakid, @BrtqKr Whoops! This issue is 2 days overdue. Let's get this updated quick!

@melvin-bot melvin-bot bot added the Overdue label Dec 5, 2024
@JmillsExpensify
Copy link

PR going through the review process, as mentioned above.

Copy link

melvin-bot bot commented Dec 9, 2024

@JmillsExpensify, @pecanoro, @Pujan92, @Skalakid, @BrtqKr Still overdue 6 days?! Let's take care of this!

@Skalakid
Copy link
Contributor

Skalakid commented Dec 9, 2024

The PR has been merged. We will bump the Expensify-common together with the next Live Markdown bump since some new breaking changes were introduced there

@melvin-bot melvin-bot bot removed the Overdue label Dec 9, 2024
@Skalakid
Copy link
Contributor

Skalakid commented Dec 11, 2024

Waiting for this PR to be merged, since it bumps the Live Markdown and expensify-common libraries

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 Engineering External Added to denote the issue can be worked on by a contributor
Projects
Status: MEDIUM
Development

No branches or pull requests