[HOLD for payment 2022-05-20][$2000] Copying / pasting code between < and /> seems broken

[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. Type something like and send the message
2. Try to select, copy, then paste into a new message

Expected Result:

Exactly what you copied should be pasted

Actual Result:

The code gets stripped
Issue happens when selecting with a mouse, not when using the Copy button

Workaround:

unknown

Platform:

Where is this issue occurring?

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

Version Number: 1.1.40-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

Screen.Recording.2022-02-14.at.6.31.29.PM.mov

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

View all open jobs on GitHub

[MelvinBot]

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

[MelvinBot]

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

[mateusbra]

PROPOSAL

We have to change how we strip html when copying to clipboard, at the moment we are only replacing everything within <> for an empty string here:
https://github.com/Expensify/expensify-common/blob/f77bb4710c13d01153716df7fb087b637ba3b8bd/lib/str.js#L306-L318

[adelekennedy]

internal
external

[MelvinBot]

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

[MelvinBot]

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

[parasharrajat]

Thanks for the proposal @mateusbra. Could you please root cause details to your proposal? and tell us how did you reach your solution.

[dennismunene]

PROPOSAL

We need to change how we handle pasted HTML here.

Change this

https://github.com/Expensify/expensify-common/blob/f77bb4710c13d01153716df7fb087b637ba3b8bd/lib/str.js#L312-L318

To

Only strip off valid html tags

    /**
     * Gets the textual value of the given string.
     *
     * @param {String} str The string to fetch the text value from.
     * @return {String} The text from within the HTML string.
     */
    stripHTML(str) {
        if (!this.isString(str)) {
            return '';
        }

        return str.replace(/<\/?(a|abbr|acronym|address|applet|area|article|aside|audio|b|base|basefont|bdi|bdo|bgsound|big|blink|blockquote|body|br|button|canvas|caption|center|cite|code|col|colgroup|data|datalist|dd|del|details|dfn|dir|div|dl|dt|em|embed|fieldset|figcaption|figure|font|footer|form|frame|frameset|h1|h2|h3|h4|h5|h6|head|header|hgroup|hr|html|i|iframe|img|input|ins|isindex|kbd|keygen|label|legend|li|link|listing|main|map|mark|marquee|menu|menuitem|meta|meter|nav|nobr|noframes|noscript|object|ol|optgroup|option|output|p|param|plaintext|pre|progress|q|rp|rt|ruby|s|samp|script|section|select|small|source|spacer|span|strike|strong|style|sub|summary|sup|table|tbody|td|textarea|tfoot|th|thead|time|title|tr|track|tt|u|ul|var|video|wbr|xmp|path|svg|...)\b[^<>]*>/g, '');
    },

SCREEN RECORDING

Screen.Recording.2022-02-27.at.11.43.17.mov

[parasharrajat]

Thanks for the proposal @dennismunene. Did you try?

 `<someComponent />` 

as message.

[dennismunene]

@parasharrajat yes I have , works as expected.

[parasharrajat]

Ok. Thanks but I don't consider this a proper approach.

const parser = new ExpensiMark(); 
     const markdownText = parser.htmlToMarkdown(html); 

This step is important for the app.

[dennismunene]

@parasharrajat updated my proposal.

[parasharrajat]

Thanks, @dennismunene but I still think we can do better.

1. StripHTML is doing its job of removing HTML but the question is why are getting HTML at this stage? HTML should have been converted to MD by now.

Now there are two things to compare.

1. Copy to clipboard and Selection copy via Ctrl+C.

Copy to clipboard works. But the selection does not. So it must have given you the idea by now what is wrong.

[mateusbra]

Thanks for the proposal @mateusbra. Could you please root cause details to your proposal? and tell us how did you reach your solution.

@parasharrajat thank you for the review. I think this is gonna be more complex than I thought, please ignore my proposal by now

[melvin-bot]

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.1.55-2 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• Fix: Copying / pasting code between < and /> #8449

If no regressions arise, payment will be issued on 2022-04-27. 🎊

[parasharrajat]

@gladiator-1 I am facing the following error on the console. Sorry, it got missed in the PR review. Could you please send a PR to fix it?

Steps:

1. Go to any chat.
2. Select any string from the message on the web.
3. Right-click to open the menu. But It throws the error on console and ContextMenu does not open.

[gladiator-1]

@parasharrajat I'm so sorry for this error, PR #8741

[adelekennedy]

@parasharrajat do we still need to wait on payment or are we good to go with the fix in #8741?

[parasharrajat]

Yes, we have to wait. The new PR is not merged.

[melvin-bot]

@parasharrajat, @marcaaron, @adelekennedy, @gladiator-1 Eep! 4 days overdue now. Issues have feelings too...

[adelekennedy]

it's okay Melvin - we're waiting on the new PR

[marcaaron]

Unassigning as I'm going OOO for a week. I think we are pretty much good here and just need to assign a new reviewer with PullerBear for whatever PR we are waiting on.

[melvin-bot]

@parasharrajat, @adelekennedy, @gladiator-1 Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[adelekennedy]

checking here - it looks like #8741 has been merged, are we good for payment @parasharrajat?

[parasharrajat]

Unfortunately not yet. PR is not yet deployed to PRDO.

[melvin-bot]

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.1.57-17 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• Fix error message belong to issue #7912 #8741

If no regressions arise, payment will be issued on 2022-05-20. 🎊

[adelekennedy]

paid and paid!