[Pay meow][$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
• [x] MacOS: Chrome / Safari
• [ ] MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

https://github.com/Expensify/App/assets/93399543/776b5c51-77c3-4fdd-94bc-9f33b61804ff

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-08-08

[melvin-bot[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 https://github.com/Expensify/App/issues/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. https://github.com/Expensify/App/blob/fb14d5a4970374a2377483e70eb5f974354570b5/src/hooks/useHtmlPaste/index.ts#L19-L20

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[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 https://github.com/Expensify/App/issues/42112 as that is resolved. I can still reproduce this - adding external

[melvin-bot[bot]]

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

[melvin-bot[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. https://github.com/Expensify/App/blob/e6c9f06a0952434f3f8ff9ace267c0e3ca026e8e/src/hooks/useHtmlPaste/index.ts#L10

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[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[bot]]

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

[melvin-bot[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.

https://github.com/Expensify/App/assets/170007025/067a681e-9ce0-4747-bc4d-34b17bfaceee

[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 https://github.com/Expensify/App/issues/42216#issuecomment-2123510932, 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[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 https://github.com/Expensify/App/issues/42216#issuecomment-2133401204

[melvin-bot[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. https://github.com/Expensify/App/pull/41265

I think this is duplicate.

[moonstar-95515]

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

[Christinadobrzyn]

Melvbot update - watching PR - https://github.com/Expensify/App/pull/42892

[Christinadobrzyn]

watching - https://github.com/Expensify/App/pull/42892

Just a heads up - I'm going to be ooo until June 24th. I'm not going to assign anyone new but if you need a new BZ teammate for any reason please feel free to ask for one to be assigned here.

[Christinadobrzyn]

watching - https://github.com/Expensify/App/pull/42892

[mallenexpensify]

@eVoloshchak @AndrewGable should we look at having another contributor, or internal engineer, get this over the finish line? It's been ~25 days since the contributor was assigned. Thx

[kpadmanabhan]

@mallenexpensify / @AndrewGable / @eVoloshchak :

this is completely unprofessional and unacceptable to have multiple people working on the same issue and keeping one wait from proceeding with original issue for this long. expensify must bring in some sort of sanitization of process in recognizing duplicate issues before C+ reviewing and assigning these to multiple people. at the moment, I monitor issues and blow whistle in each of them like this for instance.

in fact, this is the very first instance of reporting of this bug and the related PR for the issue.

if acceptable, i am ready to proceed with my PR fixing all related issues and regressions that I have kept track of, but with an increased bug bounty.

CC: @jayeshmangwani (C+ for original issue)

[mallenexpensify]

@kpadmanabhan from CONTRIBUTING.md

Daily updates on weekdays are highly recommended. If you know you won’t be able to provide updates within 48 hours, please comment on the PR or issue stating how long you plan to be out so that we may plan accordingly. We understand everyone needs a little vacation here and there. Any issue that doesn't receive an update for 5 days (including weekend days) may be considered abandoned and the original contract terminated.

I see almost no updates on the issue and only one comment on the PR over a 10 day period.

@moonstar-95515 our interest is fixing bugs quickly. When do you foresee the PR being ready to be reviewed by @eVoloshchak ?

[moonstar-95515]

@mallenexpensify Sorry for delay. I've completed everything but test video for android. Now I'm trying to build android version but I have some problems. I will complete PR asap.

[Christinadobrzyn]

Hi @moonstar-95515 checking on the PR - can you provide an update for the team? Thank you!

[moonstar-95515]

PR completed for review.

[Christinadobrzyn]

Watching PR - https://github.com/Expensify/App/pull/42892

[mallenexpensify]

Reported a bug I found while testing this https://expensify.slack.com/archives/C049HHMV9SM/p1720744680218859

[Christinadobrzyn]

Watching PR - https://github.com/Expensify/App/pull/42892

[mallenexpensify]

I have another 'cursor position isn't in right place issue`. If it's related to this or, or if this might fix it, plz comment on the issue

• https://github.com/Expensify/App/issues/45885

[Christinadobrzyn]

@eVoloshchak can you check on this question when you have a moment? https://github.com/Expensify/App/issues/42216#issuecomment-2243843021

[eVoloshchak]

@Christinadobrzyn, @mallenexpensify, I doesn't look like https://github.com/Expensify/App/issues/45885 is related to this issue

I cannot reproduce this issue on staging (or when building latest main), what should be our next steps here before closing?

[Christinadobrzyn]

Sorry for the delay @eVoloshchak - I'm not actually sure where we are with this one. Did we decide not to continue this PR? Is there anyone one?

[kpadmanabhan]

@Christinadobrzyn / @mallenexpensify / @eVoloshchak : #45885 is not related to this issue and PR. It has a different root cause of not using useHtmlPaste.

@Christinadobrzyn : This issue, for the Composer field, is already fixed in main/HEAD through other PRs. Also, I do not see any decisions made w.r.t this in PR or in public channel in Slack. In fact @eVoloshchak is asking here on how to proceed with a closure to this issue. May be @mallenexpensify might be able to guide.

[mallenexpensify]

oooof, this one is a bit confusing.

Are the next steps

1. Plan to close the issue since it isn't reproducible.
2. Before closing, allow contributors to comment if they think they're due compensation?

[moonstar-95515]

Hi @mallenexpensify,

I’d like to provide an overview of the PR for this issue:

PR Created: May 30 PR Completed (excluding Android video): June 12 Video Uploaded: July 4 I completed everything except for the Android screen recordings by June 12. I acknowledge that the delay in uploading the test video was my responsibility due to various challenges. However, I want to assure you that I did my best; in fact, I even purchased a new Mac to facilitate this work.

Could you please let me know if I could receive payment in this situation?

Thank you for your consideration. Best regards.

[mallenexpensify]

Thanks @moonstar-95515 for the breakdown!

This is a lil complex because... generally, our process is

If a contributor has been hired for a job and we decide to close the job before it is successfully completed, full payment is due for C+ and the contributor. One caveat here might be if the hired contributor wasn't working on their PR with urgency.

What makes this complex is One caveat here might be if the hired contributor wasn't working on their PR with urgency.

I propose we compensate 50% here.