Web - Chat - Pasted message not always displays no hyperlink format when paste as plain text
If you havenβt already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!
Version Number: undefined Reproducible in staging?: Yes Reproducible in production?: Yes If this was caught on HybridApp, is this reproducible on New Expensify Standalone?: undefined If this was caught during regression testing, add the test name, ID and link from TestRail: undefined Email or phone of affected tester (no customers): undefined Issue reported by: Applause Internal Team
Action Performed:
- Open https://staging.new.expensify.com/
- At any chat, type hello and send message
- Click three dots of message action menu
- Click "Copy to clipboard"
- At compose box, right click mouse and select "Paste as plain text"
- Send message
- Repeat 3 times from step 2 to step 6
Expected Result:
When choosing "Paste as plain text", the message is pasted without any hyperlink format
Actual Result:
The first time paste as plain text, the message is pasted with hyperlink format, the second time it is pasted without hyperlink format as expected
Workaround:
Unknown
Platforms:
- [ ] Android: Standalone
- [ ] Android: HybridApp
- [x] Android: mWeb Chrome
- [ ] iOS: Standalone
- [ ] iOS: HybridApp
- [ ] iOS: mWeb Safari
- [x] MacOS: Chrome / Safari
- [ ] MacOS: Desktop
Screenshots/Videos
https://github.com/user-attachments/assets/20a184cf-013e-4762-8fe6-f158863bf971
Triggered auto assignment to @alexpensify (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.
Edited by proposal-police: This proposal was edited at 2024-12-06 21:28:37 UTC.
Proposal
Please re-state the problem that we are trying to solve in this issue.
Web - Chat - Pasted message not always displays no hyperlink format when paste as plain text
What is the root cause of that problem?
We are setting the plain text to a markdown value if it is anchor tag and plain text value if it is not https://github.com/Expensify/App/blob/48b80f65e80d304965cbeb3573d7d6678fb9bd3a/src/pages/home/report/ContextMenu/ContextMenuActions.tsx#L52 but anchor regex test passes intermittently for the same value as explained here because:
When the RegExp test method is run with global flag (/g), the regex keeps internally the state of the search. Therefore at each invocation the regular exception will be run from the last index that was found previously.
What changes do you think we should make in order to solve the problem?
This isAnchorRegex check is introduced in https://github.com/Expensify/App/pull/42147 to achieve copy paste consistency across web and native platforms but to achieve it the correct way to do it is to copy the mardown content to the plain clipboard here https://github.com/Expensify/App/blob/476f3979f1ae6b89eee5b853f7368603d50403b0/src/pages/home/report/ContextMenu/ContextMenuActions.tsx#L47-L52
Clipboard.setHtml(content, Parser.htmlToMarkdown(content));
then convert from markdown > text here for web https://github.com/Expensify/App/blob/476f3979f1ae6b89eee5b853f7368603d50403b0/src/hooks/useHtmlPaste/index.ts#L93-L98
const markdownText = event.clipboardData?.getData('text/plain');
if (markdownText) {
const pastedHTML = Parser.replace(markdownText);
paste(Parser.htmlToText(pastedHTML));
}
What specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?
We can make a unit test for setClipboardMessage passing the same anchor link content (mocking canSetHtml to return true) and asserting the text argument it is passing to setHtml is the same (markdown not plain text version) for consecutive calls of the function.
What alternative solutions did you explore? (Optional)
Job added to Upwork: https://www.upwork.com/jobs/~021866332344495527627
Triggered auto assignment to Contributor-plus team member for initial proposal review - @Pujan92 (External)
@Pujan92 - Can you please review and confirm if one of these proposals will fix the issue? Thanks!
Please re-state the problem that we are trying to solve in this issue.
Web - Chat - Pasted message not always displays no hyperlink format when paste as plain text
What is the root cause of that problem?
ContextMenuActions.tsx (lines 45β55)
The root cause is that the plain text fallback wasn't being reliably set as the primary clipboard content before the HTML, leading to inconsistent behavior during the first "Paste as plain text" operation.
What changes do you think we should make in order to solve the problem?
After:
const plainText = isAnchorTag ? Parser.htmlToMarkdown(content) : Parser.htmlToText(content);
// First, set the plain text to the clipboard to ensure it is prioritized
Clipboard.setString(plainText);
// Then, set the HTML content with plain text fallback
Clipboard.setHtml(content, plainText);
What specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?
To prevent reintroducing this issue in the future, automated tests should cover a range of scenarios to verify clipboard behavior and ensure consistency across environments. Plain Text Fallback: Ensure that if Clipboard.canSetHtml() returns false, the plain text content is set correctly. HTML Content and Plain Text: Verify that when Clipboard.canSetHtml() is true, both HTML and plain text versions of the content are set as expected.
What alternative solutions did you explore? (Optional)
π£ @AK-web! π£ Hey, it seems we donβt have your contributor details yet! You'll only have to do this once, and this is how we'll hire you on Upwork. Please follow these steps:
- Make sure you've read and understood the contributing guidelines.
- Get the email address used to login to your Expensify account. If you don't already have an Expensify account, create one here. If you have multiple accounts (e.g. one for testing), please use your main account email.
- Get the link to your Upwork profile. It's necessary because we only pay via Upwork. You can access it by logging in, and then clicking on your name. It'll look like this. If you don't already have an account, sign up for one here.
- Copy the format below and paste it in a comment on this issue. Replace the placeholder text with your actual details.
Format:
Contributor details
Your Expensify account email: <REPLACE EMAIL HERE>
Upwork Profile Link: <REPLACE LINK HERE>
Edited by proposal-police: This proposal was edited at 2024-12-10 15:20:03 UTC.
Proposal
Please re-state the problem that we are trying to solve in this issue.
Chat - Pasted message is not always displayed without hyperlink format when paste as plain text
What is the root cause of that problem?
There are two problems here
- During copy to clipboard, we have inconsistent behavior observed when using the regex CONST.REGEX_LINK_IN_ANCHOR as it has global flag ('g') which is stateful and the regex lastIndex is not reset to zero before using it again. So alternatively, markdown and plain text gets copied. In the condition for isAnchorTag, only markdown should be copied to clipboard
https://github.com/Expensify/App/blob/df7ab48f101094316aab164dfed9213914713d4f/src/pages/home/report/ContextMenu/ContextMenuActions.tsx#L50-L52
- If we solve root cause (1), since markdown will be copied to clipboard as plain text, in useHTMLPaste hook when handlePastePlainText is called, we don't have the text extraction of the markdown so instead of "hello" we will be pasting
[hello](<link>)which is not the expected behaviour
https://github.com/Expensify/App/blob/9fe5511a6c9502c289c129543a09a4fa5f33f65e/src/hooks/useHtmlPaste/index.ts#L79-L81
What changes do you think we should make in order to solve the problem?
We should create a new instance of RegExp. The updated code in https://github.com/Expensify/App/blob/df7ab48f101094316aab164dfed9213914713d4f/src/pages/home/report/ContextMenu/ContextMenuActions.tsx#L50 will be
const anchorRegex = new RegExp(CONST.REGEX_LINK_IN_ANCHOR);
To handle root cause (2), we should check and convert the markdown to plain text in https://github.com/Expensify/App/blob/9fe5511a6c9502c289c129543a09a4fa5f33f65e/src/hooks/useHtmlPaste/index.ts#L81
What specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?
None
What alternative solutions did you explore? (Optional)
@alexpensify, @Pujan92 Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!
Thanks for the proposals.
@FitseTLT and @prakashbask's proposal of regex new instance won't solve the problem as it will only make it consistent(isAnchorTag gets true) and paste will be always the link format.
@AK-web's RCA and solution don't look correct to me.
- If we solve root cause (1), since markdown will be copied to clipboard as plain text, in useHTMLPaste hook when handlePastePlainText is called, we don't have the text extraction of the markdown so instead of "hello" we will be pasting hello which is not the expected behaviour
I agree with the @prakashbask RCA but why do we need the text extraction within the useHTMLPaste? Isn't the right plainText should be passed from ContextMenuActions Clipboard.setHtml?
Thanks for the proposals.
@FitseTLT and @prakashbask's proposal of regex new instance won't solve the problem as it will only make it consistent(isAnchorTag gets true) and paste will be always the link format.
@Pujan92 You have misunderstood the problem. The problem is that isAnchorTag is being inconsistent and the solution is to make it consistent. You have to first understand the purposes of isAnchorTag check please refer to https://github.com/Expensify/App/pull/42147 for more context where they have implemented to make the copy of anchor to be always the markdown text for both plain and HTML clipboard. The purpose is to make it consistent whether anchor link are copied to clipboard from web or native. π
@FitseTLT The problem isn't only making isAnchorTag consistent, it also should paste correct text without the link when the user chooses "Paste as plain text".
@FitseTLT The problem isn't only making
isAnchorTagconsistent, it also should paste correct text without the link when the user chooses "Paste as plain text".
We will cause regression on https://github.com/Expensify/App/pull/42147 please read the PR for more context.
You have to first understand the purposes of isAnchorTag check please refer to https://github.com/Expensify/App/pull/42147 for more context where they have implemented to make the copy of anchor to be always the markdown text for both plain and HTML clipboard.
As a hint, I would like to suggest checking whether https://github.com/Expensify/App/issues/40564 is an actual issue or correct behavior.
You have to first understand the purposes of isAnchorTag check please refer to #42147 for more context where they have implemented to make the copy of anchor to be always the markdown text for both plain and HTML clipboard.
As a hint, I would like to suggest checking whether #40564 is an actual issue or correct behavior.
That's a nice idea. My first proposal was like your suggestions but after digging deep I found out the PR and that's why I proposed the fix for the RCA of the inconsistency.
In case your expectation is accepted I have Updated to incorporate it to my alt solution
Ok @Pujan92 Let's ask for the expected behaviour π
When choosing "Paste as plain text", the message is pasted without any hyperlink format
@alexpensify Could you plz confirm the expected behavior for this issue as @FitseTLT is requesting? To me, it looks correct.
When choosing "Paste as plain text", the message is pasted without any hyperlink format
@alexpensify Could you plz confirm the expected behavior for this issue as @FitseTLT is requesting? To me, it looks correct.
@Pujan92 @alexpensify We will need to club this 53832 issue also to discuss the expected behaviour
@alexpensify To make it clear for you. The problem here is inconsistency in the paste behaviour as mentioned in the Actual Result
Actual Result: The first time paste as plain text, the message is pasted with hyperlink format, the second time it is pasted without hyperlink format as expected
We have no doubt on fixing the inconsistency. What we want from you is to confirm on the behaviour of paste as a plain text for links. what should be pasted? A. plain text B. link
In all other cases for past as plain we paste the text without the formating, for instance, if it was a bold text markdown when we paste as plain we paste only the raw text without formatting but the problem is they have made an exception for anchor links in this PR to fix an issue That change made links to be pasted with the link for both normal paste and paste as plain text options. WDYT is the expected behaviour? Should we revert the change by that PR or fix the inconsistency based on the expectations from that PR.
@Pujan92 @alexpensify We will need to club this 53832 issue also to discuss the expected behaviour
Agree @prakashbask, both can be clubbed. The only platform that differs is Android as it doesn't support HTML paste and we need to consider it a special case for deriving plainText. cc @ZhenjaHorbach
As a hint, I would like to suggest checking whether https://github.com/Expensify/App/issues/40564 is an actual issue or correct behavior.
I might be wrong here @FitseTLT, I think that issue needs to be generic to render all markdowns on Android instead of plain text.
The only platform that differs is Android as it doesn't support HTML paste and we need to consider it a special case for deriving plainText.
I think both IOS and Android https://github.com/Expensify/App/blob/b2dca65a743aea0c09cbb5b711e832f2fc9a6e1a/src/libs/Clipboard/index.native.ts#L11-L12
I might be wrong here @FitseTLT, I think that issue needs to be generic to render all markdowns on Android instead of plain text.
So where do you think we should handle it here or there?
Heads up, I will be offline until Wednesday, December 18, 2024, and will not actively watch over this GitHub during that period.
I believe there is an updated plan here, but if this GitHub requires an urgent update, please ask for help in the #expensify-open-source Slack Room. If the inquiry can wait, I'll review it when I return online.
@Pujan92 I have updated based on the expectation here
I might be wrong here @FitseTLT, I think that issue needs to be generic to render all markdowns on Android instead of plain text.
@alexpensify, @Pujan92 Whoops! This issue is 2 days overdue. Let's get this updated quick!
π£ It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? πΈ
@alexpensify, @Pujan92 Eep! 4 days overdue now. Issues have feelings too...
@Pujan92 can you please review the update and see if it will fix the issue? Thanks!
@alexpensify @Pujan92 this issue was created 2 weeks ago. Are we close to approving a proposal? If not, what's blocking us from getting this issue assigned? Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!
@alexpensify, @Pujan92 8 days overdue is a lot. Should this be a Weekly issue? If so, feel free to change it!
Setting the plainText as htmlText seems logically wrong to me, I think instead the react native android should support the text/html if the passed html string worked with type text/plain.
Setting the plainText as htmlText in the clipboard seems logically wrong to me, but this only looks like the option(setting markdown text in text/plain option) for android. As I am not sure, let me ask for inputs from react-native-live-markdown team.
Issue: When we paste the clipboard content in android which is copied from the other web platform, markdown isn't rendered as text/html type isn't supported in android.
@Skalakid @BartoszGrajdek Could you plz give a look and provide your suggestion?
Also adding a stamp to include an internal engineer.
πππ C+ reviewed