[HOLD for payment 2023-04-20] [$1000] Inconsistent link copy between manual copy and "Copy to Clipboard"

[kavimuru]

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. Open staging.new.expensify.com
2. Send a hyperlink with a text, for example test
3. Copy the link manually (with Ctrl+C and Ctrl+V) and paste it on the chat textbox
4. Copy the link by selecting "Copy to Clipboard" from the message options and paste it on the chat textbox
   Examine the difference

Expected Result:

All three copying methods retains the hyperlink format when copied in the chat textbox

Actual Result:

While link copied with "Copy to Clipboard" retains its hyperlink format, link copied manually isn't retain it and just pasted regular text (without link)

Workaround:

unknown

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.2.89-0
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:

Link.Copy.mp4

Recording.59.mp4

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

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01ee21b7899295084b
• Upwork Job ID: 1641536680230211584
• Last Price Increase: 2023-04-06

[MelvinBot]

Triggered auto assignment to @anmurali (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

[MelvinBot]

Bug0 Triage Checklist (Main S/O)

• This "bug" occurs on a supported platform (ensure Platforms in OP are ✅)
• This bug is not a duplicate report (check E/App issues and #expensify-bugs)
  • If it is, comment with a link to the original report, close the issue and add any novel details to the original issue instead
• This bug is reproducible using the reproduction steps in the OP. S/O
  • If the reproduction steps are clear and you're unable to reproduce the bug, check with the reporter and QA first, then close the issue.
  • If the reproduction steps aren't clear and you determine the correct steps, please update the OP.
• This issue is filled out as thoroughly and clearly as possible
  • Pay special attention to the title, results, platforms where the bug occurs, and if the bug happens on staging/production.
• I have reviewed and subscribed to the linked Slack conversation to ensure Slack/Github stay in sync

[MelvinBot]

@anmurali Whoops! This issue is 2 days overdue. Let's get this updated quick!

[MelvinBot]

Job added to Upwork: https://www.upwork.com/jobs/~01ee21b7899295084b

[MelvinBot]

Current assignee @anmurali is eligible for the External assigner, not assigning anyone new.

[MelvinBot]

Triggered auto assignment to Contributor-plus team member for initial proposal review - @Santhosh-Sellavel (External)

[MelvinBot]

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

[anmurali]

Created a job for external contributors.

[alexxxwork]

Proposal

Please re-state the problem that we are trying to solve in this issue.

While link copied with "Copy to Clipboard" retains its hyperlink format, link copied manually doesn't retain it and just copies regular text (without link)

What is the root cause of that problem?

The root cause of the problem is that when you select text the browser selects only text. Copy button in menu copies the underlying markdown using ExpesniMark object to convert html to text or markdown.
The right-click menu has different intended behaviour and shows option applicable to the post content. The "Copy URL to clipboard" action is different from "Copy to clipboard" action. If you have a link and text in a post - right click menu will be different for different regions of the post (link or text).

What changes do you think we should make in order to solve the problem?

We have a keyboard listener for copy to clipboard keys in a separate component here:

App/src/components/CopySelectionHelper.js

Line 12 in 2398744

this.unsubscribeCopyShortcut = KeyboardShortcut.subscribe(

and this component is mounted in ReportActionView:

App/src/pages/home/report/ReportActionsView.js

Line 357 in 2398744

<CopySelectionHelper />

It converts html to markdown and doesn't do it when you have only link in a post

App/src/components/CopySelectionHelper.js

Line 36 in 2398744

Clipboard.setString(parser.htmlToMarkdown(selection));

we could alter this to capture single links:
Clipboard.setString(parser.htmlToMarkdown(parser.htmlToMarkdown(parser.replace(selection,{filterRules:['autolink']}))));
(this is weird, we'd better write another rule in ExpensiMark)

I think we shouldn't alter "Copy URL" and "Copy" behaviour as they are intended.
If we want to alter the right menu behaviour we should edit this function or select different action to show in right-click menu.

App/src/pages/home/report/ContextMenu/ContextMenuActions.js

Lines 113 to 121 in 2398744

	textTranslateKey: 'reportActionContextMenu.copyURLToClipboard',
	icon: Expensicons.Clipboard,
	successTextTranslateKey: 'reportActionContextMenu.copied',
	successIcon: Expensicons.Checkmark,
	shouldShow: type => type === CONTEXT_MENU_TYPES.LINK,
	onPress: (closePopover, {selection}) => {
	Clipboard.setString(selection);
	hideContextMenu(true, ReportActionComposeFocusManager.focus);
	},

What alternative solutions did you explore? (Optional)

Honestly, I think it isn't right to copy to clipboard something different than you see. So maybe we should alter the "Copy" function here:

App/src/pages/home/report/ContextMenu/ContextMenuActions.js

Line 160 in 2398744

Clipboard.setString(parser.htmlToMarkdown(content));

change to
Clipboard.setString(parser.htmlToText(content));

[hoangzinh]

Proposal

Please re-state the problem that we are trying to solve in this issue.

Link copied manually isn't retain it and just pasted regular text (without link)

What is the root cause of that problem?

Because when user select a text that starts and ends within a HTML tag, in the method SelectionScraper.getHTMLOfSelection, the window.getSelection() API only returns content of selection, which is string. It doesn't know that there is an anchor tag outside.

What changes do you think we should make in order to solve the problem?

We can add more code in this line so that if the selection starts and ends within same text node, we do more check if it's textContent same as parentNode's textContent, we assume user selected whole content in the parent node => use the parentNode instead of itself => it will ensure we can copy the anchor tag so that we can parse it as an url later

Sample code how does it look like:

const parentNode = range.commonAncestorContainer.parentNode;
if (parentNode.textContent === clonedSelection.textContent) {
    clonedSelection = range.createContextualFragment(parentNode.outerHTML);
}

node = parentNode.closest(`[${tagAttribute}]`);

[dianaferreira1]

Proposal

Please re-state the problem that we are trying to solve in this issue.

When the link is copied manually it loses the hyperlink ( and appears as regular text) , however it retains it when copied with "Copy to Clipboard"

What is the root cause of that problem?

The main issue stems from the fact that when text is selected, the browser only selects the text itself. However, the "Copy" button in the menu copies the markdown that underlies the selected HTML, using the ExpesniMark object to convert it to either text or markdown format.

The right-click menu is designed to have different options that are relevant to the content of the post. Therefore, the "Copy URL to clipboard" action is separate from the "Copy to clipboard" action. Additionally, if there is both a link and text in a post, the right-click menu will differ depending on which part of the post is selected (either the link or the text).

What changes do you think we should make in order to solve the problem?

Select the link element that contains the URL to be copied, and then add an event listener to the "Copy to Clipboard" button. When the button is clicked, create a new temporary input element, set its value to the URL of the link element, and add it to the document. Then select the contents of the temporary input element and execute the "copy" command to copy the URL to the clipboard. Finally, Remove the temporary input element from the document.

Also add an event listener to the link element itself, which handles manual copying. When the link is clicked, use the navigator.clipboard.writeText() method to copy the URL to the clipboard directly.

What alternative solutions did you explore? (Optional)

Reminder: Please use plain English, be brief and avoid jargon. Feel free to use images, charts or pseudo-code if necessary. Do not post large multi-line diffs or write walls of text. Do not create PRs unless you have been hired for this job.

[MelvinBot]

📣 @dianaferreira1! 📣

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:

1. 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.
2. 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.
3. 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>

[MelvinBot]

✅ Contributor details stored successfully. Thank you for contributing to Expensify!

[dianaferreira1]

Contributor details
Your Expensify account email: concierge@expensify.com
Upwork Profile Link: https://www.upwork.com/freelancers/~0154add7fc91591af5

[MelvinBot]

⚠️ Missing/invalid email or upwork profile link. Please make sure you add both your Expensify email and Upwork profile link in the format specified.

[dianaferreira1]

Contributor details
Your Expensify account email: concierge@expensify.com
Upwork Profile Link: https://www.upwork.com/freelancers/dianaf4

[MelvinBot]

⚠️ Missing/invalid email or upwork profile link. Please make sure you add both your Expensify email and Upwork profile link in the format specified.

[bernhardoj]

Proposal

Please re-state the problem that we are trying to solve in this issue.

When we copy the link manually (ctrl+c) and paste it to composer, the formatting/markdown is lost.

What is the root cause of that problem?

We should understand first what happen when we copy a chat manually. Copying a chat manually will select the HTML from what we currently selecting (the blue highlight).

App/src/libs/SelectionScraper/index.js

Lines 133 to 139 in 2f922c4

	const getCurrentSelection = () => {
	const domRepresentation = parseDocument(getHTMLOfSelection());
	domRepresentation.children = _.map(domRepresentation.children, replaceNodes);
	const newHtml = render(domRepresentation);
	return newHtml || '';
	};

For example, if the chat is a bold message, the selected HTML will be <strong>text</strong>. But if we take a look at how the bolded text is rendered (image below), the element uses is not strong, but a div.

How can it's converted to strong and also know which HTML to select? It's all thanks to data-testid attribute as you may noticed from the screenshot above (data-testid=strong). getHTMLOfSelection is responsible to select the HTML by selecting the closest node that has a data-testid attribute. In our example case, the closest node is the bold text node itself.

App/src/libs/SelectionScraper/index.js

Lines 63 to 67 in 2f922c4

	if (range.commonAncestorContainer instanceof HTMLElement) {
	node = range.commonAncestorContainer.closest(`[${tagAttribute}]`);
	} else {
	node = range.commonAncestorContainer.parentNode.closest(`[${tagAttribute}]`);
	}

And then later converted to it's data-testid value by replaceNodes.

Now, why it does not work for anchor tag? The data-testid attributes is set from react-native-render-html by taking the element tagName as it's value. However, our anchor tag uses a custom renderer.

https://github.com/meliorence/react-native-render-html/blob/a02be7119faf00f2a45a3c87e851a2ea4b22128f/packages/render-html/src/helpers/getNativePropsForTNode.ts#L73

App/src/components/HTMLEngineProvider/HTMLRenderers/AnchorRenderer.js

Lines 86 to 105 in 2f922c4

	return (
	<AnchorForCommentsOnly
	href={attrHref}
	// Unless otherwise specified open all links in
	// a new window. On Desktop this means that we will
	// skip the default Save As... download prompt
	// and defer to whatever browser the user has.
	// eslint-disable-next-line react/jsx-props-no-multi-spaces
	target={htmlAttribs.target || '_blank'}
	rel={htmlAttribs.rel || 'noopener noreferrer'}
	style={{...props.style, ...parentStyle}}
	key={props.key}
	displayName={displayName}
	// Only pass the press handler for internal links, for public links fallback to default link handling
	onPress={(internalNewExpensifyPath || internalExpensifyPath) ? navigateToLink : undefined}
	>
	<TNodeChildrenRenderer tnode={props.tnode} />
	</AnchorForCommentsOnly>

This makes the data-testid is not applied to the anchor tag and the closest node for anchor tag will be div with data-testid=comment.

So, when we copy a link, this is what we get

<comment>link</comment>

NOTE: There are some tag that uses custom renderer too (e.g., code, pre) that still works because both of them use the TDefaultRenderer, while AnchorRenderer is not.

What changes do you think we should make in order to solve the problem?

Because we use a custom renderer, we need to set the data-testid manually to our anchor tag here

App/src/components/AnchorForCommentsOnly/BaseAnchorForCommentsOnly.js

Lines 69 to 82 in 2f922c4

	<Text
	ref={el => linkRef = el}
	style={StyleSheet.flatten([props.style, defaultTextStyle])}
	accessibilityRole="link"
	hrefAttrs={{
	rel: props.rel,
	target: props.target,
	}}
	href={linkProps.href}
	// eslint-disable-next-line react/jsx-props-no-spreading
	{...rest}
	>
	{props.children}
	</Text>

testID='a'

[Santhosh-Sellavel]

@luacmartins

@bernhardoj proposal here seems like a right approach to me, let me know your thoughts please, thanks!

[hoangzinh]

@Santhosh-Sellavel there is a behavior that you might need to consider with @bernhardoj proposal above. It's if we only select a part of link, For example: only select expensify in https://staging.new.expensify.com/, it will be copied as a link too, which will change current behavior of "Copy to clipboard".
Or even they just select and copy a "." in the text, it will be copied as a link too.

[luacmartins]

Just waiting on tests

[Santhosh-Sellavel]

Is this waiting for me or @anmurali

[luacmartins]

I see these items unchecked in the checklist:

[@anmurali] Determine if we should create a regression test for this bug.
[@Santhosh-Sellavel] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.
[@anmurali] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[Santhosh-Sellavel]

@anmurali

I'm sure the applause team is aware of the ways to copy the messages keyboard method.
Here are the regression steps for this PR will let them decide and update where it's required

Regression steps:

1. Open any chat
2. Send a message with a link, for example link
3. Select the message you just sent and copy (ctrl/cmd+c)
4. Paste it to composer
5. Verify the pasted text is link

👍 or 👎

cc: @luacmartins

[luacmartins]

@Santhosh-Sellavel test steps make sense!

[luacmartins]

@anmurali can we add these test steps to TestRail?

[luacmartins]

waiting on tests

[anmurali]

https://github.com/Expensify/Expensify/issues/282800

[anmurali]

Paid

[kerupuksambel]

Hi there, I am sorry to asking to this closed issue, but it seems that I, as the issue reporter, hasn't got the offer and also hasn't been paid. Is it okay to ask for help on this matter? Thanks in advance.

[kadiealexander]

@kerupuksambel could you please apply here on Upwork, so we can pay you?

[kerupuksambel]

@kadiealexander I am sorry, but it seems that I can't apply there due to 403 error in Upwork. Could you please send me a publicly available offer for me to apply at or accept? Thanks beforehand

[kadiealexander]

Could you please try this one?

[kerupuksambel]

@kadiealexander Thank you for the offer, I have applied to the job.

[kadiealexander]

@kerupuksambel I've sent an offer, please accept and I'll issue payment :)

[kerupuksambel]

@kadiealexander Thanks! Offer has been accepted now.

[kadiealexander]

Thanks! All paid :)