[$250] Web - Composer - Cursor positioned at the start if pasting text which contains a new line

[kbecciv]

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: 1.4.74-0
Reproducible in staging?: y
Reproducible in production?: y
Issue reported by: Applause - Internal Team

Action Performed:

1. Navigate to staging.new.expensify.com
2. Open a chat and type a one line text and create an empty new line
3. Select all using keyboard ctrl + A and cut the text
4. Paste it inside the compose box

Expected Result:

The cursor gets positioned at the end of the pasted text, no console error

Actual Result:

The cursor gets positioned at the start of the pasted text if the pasted text contains a new empty line, console error is showing

Workaround:

n/a

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

Bug6481716_1715782332852.bandicam_2024-05-15_17-07-29-894.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01cc45b59ee9c4ae02
• Upwork Job ID: 1792451156946927616
• Last Price Increase: 2024-05-20

[melvin-bot]

Triggered auto assignment to @Christinadobrzyn (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.

[kbecciv]

We think that this bug might be related to #wave-collect - Release 1

[kbecciv]

@Christinadobrzyn 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.

[Christinadobrzyn]

Very similar to #42112 - asking if the fix will be the same

[moonstar-95515]

Proposal

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

Web - Composer - Cursor positioned at the start if pasting text which contains a new line

What is the root cause of that problem?

We are setting cursor position in below part when pasting.

App/src/hooks/useHtmlPaste/index.ts

Lines 19 to 20 in fb14d5a

	range.setStart(node, node.length);
	range.setEnd(node, node.length);

When pasted text contains empty line, the value of node.length is greater than it's actually length because the length of line break at the end(\n\n) will be caculated as 2.
So console error occurs and cursor is not set because we are trying to set cursor to non-exist position.

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

We have to set range with node.length - 1 when it contains empty line at the end.

What alternative solutions did you explore? (Optional)

[melvin-bot]

📣 @moonstar-95515! 📣
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. Make sure you've read and understood the contributing guidelines.
2. 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.
3. 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.
4. 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>

[Christinadobrzyn]

Doesn't seem to be related to #42112 as that is resolved. I can still reproduce this - adding external

[melvin-bot]

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

[melvin-bot]

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

[rakhaxor]

Proposal

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

Web - Composer - Cursor positioned at the start if pasting text which contains a new line

What is the root cause of that problem?

In the text param, newline character(s) at the end (if present) are wrongly interpreted by the cursorUtils library because node.length is calculated including the \n but \n is not inserted in the document node which makes the range go out of bounds.

App/src/hooks/useHtmlPaste/index.ts

Line 10 in e6c9f06

const insertAtCaret = (target: HTMLElement, text: string) => {

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

Creating a new text variable with stripped newline characters at the end or modifying reusing the same text would work fine.

const plainText = text.replace(/\n+$/, '');
And use this in all the places we are using text:

const node = document.createTextNode(text);
insertByCommand(text);

What alternative solutions did you explore? (Optional)

None

[RickDeb2004]

Proposal

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

when pasting text containing a new empty line into the chat compose box, the cursor incorrectly positions itself at the start of the text instead of the end and a console error occurs.

What is the root cause of that problem?

The root cause of the problem is that the cursor positioning logic in the insertAtCaret function incorrectly handles the insertion of text containing new lines. Specifically, the cursor is not being moved to the correct position (the end of the newly inserted text) when the text includes empty new lines, which causes the cursor to appear at the start of the text. Additionally, there is no proper error handling to catch and log any potential issues during the paste operation, leading to console errors.

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

a)
original code :

range.setStart(node, node.length);
       range.setEnd(node, node.length);
       selection.removeAllRanges();
       selection.addRange(range);

modified code:

 range.setStartAfter(node);
        range.setEndAfter(node);
        selection.removeAllRanges();
        selection.addRange(range);
 

b)
original code :

target.dispatchEvent(new Event('paste', {bubbles: true}));
       // Dispatch input event to trigger Markdown Input to parse the new text
       target.dispatchEvent(new Event('input', {bubbles: true}));
   } else {
       insertByCommand(text);
   };

modified code:

 target.dispatchEvent(new Event('input', {bubbles: true}));
    } else {
        insertByCommand(text);
    }
 

c)
original code :

const paste = useCallback((text: string) => {
       try {
           const textInputHTMLElement = textInputRef.current as HTMLElement;
           if (textInputHTMLElement?.hasAttribute('contenteditable')) {
               insertAtCaret(textInputHTMLElement, text);
           } else {
               insertByCommand(text);
           }

           // Pointer will go out of sight when a large paragraph is pasted on the web. Refocusing the input keeps the cursor in view.
           textInputRef.current?.blur();
           textInputRef.current?.focus();
           // eslint-disable-next-line no-empty
       } catch (e) {}
       // We only need to set the callback once.
       // eslint-disable-next-line react-hooks/exhaustive-deps
   }, []);

modified code:

 const paste = useCallback((text: string) => {
        try {
            const textInputHTMLElement = textInputRef.current as HTMLElement;
            if (textInputHTMLElement?.hasAttribute('contenteditable')) {
                insertAtCaret(textInputHTMLElement, text);
            } else {
                insertByCommand(text);
            }

            // Pointer will go out of sight when a large paragraph is pasted on the web. Refocusing the input keeps the cursor in view.
            textInputRef.current?.blur();
            textInputRef.current?.focus();
        } catch (e) {
            // ** Added console error log here **
            console.error('Error during paste handling:', e);
        }
    }, []);

 

Summary of Changes

1.Cursor Positioning Fix:

Updated the insertAtCaret function to use range.setStartAfter(node) and range.setEndAfter(node) to correctly position the cursor at the end of the newly inserted text, especially when new lines are involved.

2.Error Handling:

Added console.error logging within the paste function to handle and log any exceptions that occur during the paste operation, making it easier to debug issues.
These changes ensure that the cursor is positioned correctly after pasting text with new lines and improve error visibility during paste operations.

What alternative solutions did you explore? (Optional)

None

Contributor details
Your Expensify account email: debanjanrick04@gmail.com
Upwork Profile Link: https://www.upwork.com/freelancers/~017738fb94de003574

[melvin-bot]

📣 @RickDeb2004! 📣
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. Make sure you've read and understood the contributing guidelines.
2. 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.
3. 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.
4. 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>

[Christinadobrzyn]

hi @eVoloshchak can you take a peek at these proposals when you have a moment? TY!

[eVoloshchak]

We have to set range with node.length - 1 when it contains empty line at the end.
@moonstar-95515, that works for this specific case of testing steps, but wouldn't work if pasted text end with 3 (or more) new lines

I think we should proceed with @rakhaxor's proposal, it's clean and straightforward. It does cut off all of the new lines at the end of the pasted text, but that's what the backend does when you send the message, so this can be considered a small improvement

🎀👀🎀 C+ reviewed!

[melvin-bot]

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

[melvin-bot]

📣 @rakhaxor You have been assigned to this job!
Please apply to the Upwork job and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
Once you apply to this job, your Upwork ID will be stored and you will be automatically hired for future jobs!
Keep in mind: Code of Conduct | Contributing 📖

[moonstar-95515]

@eVoloshchak

@moonstar-95515, that works for this specific case of testing steps, but wouldn't work if pasted text end with 3 (or more) new lines

My proposal works when it has 3 or more new lines at the end, as the text will be hello 111\n\n\n\n when it has 3 new lines.
Only last new line is presented with two \n.

[moonstar-95515]

@eVoloshchak cc: @Christinadobrzyn @AndrewGable

It does cut off all of the new lines at the end of the pasted text, but that's what the backend does when you send the message, so this can be considered a small improvement

I can't sure if I understood correctly, but I think only new lines at the end of the text will be cut on backend side.
If we copy text with new lines in the middle of the compose box and paste it again, new lines will be deleted. I think this is not what we want.

test.mp4

[rakhaxor]

@eVoloshchak Thank you for accepting my proposal! This is my first time contributing and I just applied to the Upwork job like 3-4 hours ago. Do I need to do some further action before my upwork proposal is accepted and do I need to wait for it to be accepted before I could raise my PR?

[moonstar-95515]

@eVoloshchak cc: @Christinadobrzyn @AndrewGable

It does cut off all of the new lines at the end of the pasted text, but that's what the backend does when you send the message, so this can be considered a small improvement

I can't sure if I understood correctly, but I think only new lines at the end of the text will be cut on backend side.
If we copy text with new lines in the middle of the compose box and paste it again, new lines will be deleted. I think this is not what we want.

What do you think about my suggestion? Could you please check? Thank you.

[Christinadobrzyn]

Dmd @eVoloshchak to see if he can take a peek at the recent comments. Thanks!

[eVoloshchak]

@moonstar-95515, thank you for #42216 (comment), good catch, this is a case I've missed and it's definitely not the desired behavior.

I think we should proceed with @moonstar-95515's proposal
It's simpler and more efficient (since we don't need to iterate over the whole string with replace, we just need to check if the line ends with a line break)

Apologies for the confusion folks
cc: @AndrewGable

[rakhaxor]

@moonstar-95515, thank you for #42216 (comment), good catch, this is a case I've missed and it's definitely not the desired behavior.

I think we should proceed with @moonstar-95515's proposal It's simpler and more efficient (since we don't need to iterate over the whole string with replace, we just need to check if the line ends with a line break)

Apologies for the confusion folks cc: @AndrewGable

Hello @eVoloshchak, @AndrewGable! I have already sent proposal on Upwork and about to raise a PR. I think this is not a good way since I have put a lot of my time on it setting up everything. And my solution works as needed. Also for the moonstar's proposal, I believe he posted a video where he got "hi" text below the text he is writing, so it is working but I am not sure it would work with multiple lines in case there are newlines after the last character.

[moonstar-95515]

@rakhaxor

Also for the moonstar's proposal, I believe he posted a video where he got "hi" text below the text he is writing, so it is working but I am not sure it would work with multiple lines in case there are newlines after the last character.

I've tested my proposal with multiple lines at the end and it's working properly.

[moonstar-95515]

Hi, @AndrewGable
Could you please assign me so that I can raise PR? Thanks.

[melvin-bot]

@AndrewGable @eVoloshchak @rakhaxor @Christinadobrzyn 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!

[Christinadobrzyn]

Double-checking with @AndrewGable that we're good with #42216 (comment)

[melvin-bot]

The BZ member will need to manually hire moonstar-95515 for the Contributor role. Please store your Upwork details and apply to our Upwork job so this process is automatic in the future!

[kpadmanabhan]

@eVoloshchak : There is already a PR in progress for this issue.
#41265

I think this is duplicate.

[moonstar-95515]

Hi, @eVoloshchak
PR is ready for review. I will upload videos asap.