[$250] mWeb - Chat-mweb-Copy to clipboard [] 4 + image, pasted& sent content goes out of screen

[IuliiaHerets]

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: v9. 0.35-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: https://expensify.testrail.io/index.php?/tests/view/4968019 Issue reported by: Applause Internal Team

Action Performed:

1. Go to https://staging.new.expensify.com/
2. Open a chat
3. Enter [] 4 + upload a image
4. Send the message
5. Long press the message sent and select copy to clipboard
6. Paste the content in compose box
7. Send the message
8. Note the content above error message, goes out of screen and not visible fully.

Expected Result:

Copy to clipboard [] 4 + image, pasted& sent content must not go out of screen and must be visible fully.

Actual Result:

Copy to clipboard [] 4 + image, pasted& sent content goes out of screen and not visible fully.

Workaround:

Unknown

Platforms:

• [ ] Android: Native
• [x] Android: mWeb Chrome
• [ ] iOS: Native
• [ ] iOS: mWeb Safari
• [ ] MacOS: Chrome / Safari
• [ ] MacOS: Desktop

Screenshots/Videos

https://github.com/user-attachments/assets/0201856a-4099-46d2-b45c-f5686ed594bc

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~021836484063080797102
• Upwork Job ID: 1836484063080797102
• Last Price Increase: 2024-09-25

Issue Owner

Current Issue Owner: @alitoshmatov

[melvin-bot[bot]]

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

[IuliiaHerets]

@greg-schroeder 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

[greg-schroeder]

Pretty niche bug to encounter but it does seem broken for sure

[skg0525]

Edited by proposal-police: This proposal was edited at 2024-09-19 01:10:50 UTC.

Proposal

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

mWeb - Chat-mweb-Copy to clipboard [] 4 + image, pasted& sent content goes out of screen

What is the root cause of that problem?

onSubmitComment method invokes handleCreateTask which checks the string for task and since task strings starts with "[] " the square brackets in orginal message are mistaken for a task

https://github.com/Expensify/App/blob/539327dc2d0585624e5e0b990ca0d79f4e66e81b/src/pages/home/report/ReportFooter.tsx#L127-L137

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

We need to modify the regex so it checks if the pasted string contains a attachment, i.e. "[]

!(https://www.expensify.com/chat-attachments/..........1024.jpg)"

then it does not try to create a task and rather treat it as a comment with attachments

What alternative solutions did you explore? (Optional)

[melvin-bot[bot]]

📣 @skg0525! 📣 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>

[skg0525]

Contributor details Your Expensify account email: iamskg7@gmail.com Upwork Profile Link: https://www.upwork.com/freelancers/~01f0c31b30aaa4d002

[melvin-bot[bot]]

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

[melvin-bot[bot]]

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

[melvin-bot[bot]]

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

[greg-schroeder]

Next up is proposal review

[klajdipaja]

Edited by proposal-police: This proposal was edited at 2024-09-20 10:14:42 UTC.

Proposal

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

When copying a text that starts with [] but also has attachment markdown syntax behind that in the chat an error is shown that the task failed to create.

What is the root cause of that problem?

There are two things to consider here:

1. The text that is copied is a valid syntax for creating a Task since it starts with []. What comes after that would be the title of the task. That should not be considered a bug IMO as it is the users responsibility to know what they're pasting in the keyboard.
2. The actual root of the error. Since the syntax is for a task the pasted link is considered to be the title of the task but we are not applying any size limitations to it as we do when a task is created using the Create Task form. In the tested example we get an error from the backend the Task title is too long. We would get the same error if you pasted this long text into it: [] asjksjdjksajdsakjd kaskjd sakd sadaskjdkjsakjkjdskj kjsadkj akjsdkjsa kjdsa kjdkjsa dsad sa dsakjd kjsakj dkjsakjd kjsa dsakjakjd kaskjd sakd sadaskjdkjsakjkjdskj kjsadkj akjsdkjsa kjdsa kjdkjsa dsad sa dsakjd kjsakj dkjsakjd kjsa dsakjakjd kaskjd sakd sadaskjdkjsakjkjdskj kjsadkj akjsdkjsa kjdsa kjdkjsa dsad sa dsakjd kjsakj dkjsakjd kjsa dsakjakjd kaskjd sakd sadaskjdkjsakjkjdskj kjsadkj akjsdkjsa kjdsa kjdkjsa dsad sa dsakjd kjsakj dkjsakjd kjsa dsakj

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

Option A. We should add a validation for the task title in https://github.com/Expensify/App/blob/main/src/pages/home/report/ReportFooter.tsx # handleCreateTask and show an error to the user for the use case as in the reported scenario.

My suggestion is to add a state for the error message:

const [error, setError] = useState<Errors>(null);

And in the handleCreateTask right after we check if the title is not empty we would add a check for the max length of the title right after that block: https://github.com/Expensify/App/blob/d009d8ed4830c7d0562c524c991b8a688e2b4869/src/pages/home/report/ReportFooter.tsx#L139-L141 As below:

            if (title.length > CONST.TASK_TITLE_MAX_LENGTH) {
                setError({taskCreationError: translate('violations.taskTitleLength')} as Errors);
                return false;
            }

I would actually like to split that callback in two functions with separated concerns

1. One that parses the message into TaskRequest object with the following property: isTask,title,asigne,mention. This one would also have the logic to validate the title length.
2. One that handles the creation of the task as the one currently.

That would improve the readability of those functions and of the onSubmit which would look like this:

  const onSubmitComment = useCallback(
        (text: string) => {
            const taskRequest= parseTask(text);
            if (taskRequest.isTask) {
                handleCreateTask(taskRequest);
            } else
                Report.addComment(report.reportID, text);
        },
        // eslint-disable-next-line react-compiler/react-compiler, react-hooks/exhaustive-deps
        [report.reportID, handleCreateTask, error],
    );

And inside the SwipableView to add the message that will show the error:

                        {error && (
                            <DotIndicatorMessage
                                style={[styles.mh4, styles.mv3]}
                                messages={error}
                                type="error"
                            />
                        )}

The output of this would be:

We should probably check about the error message with someone from a product/UX perspective because till now the composer did not have error messages.

Option B.

We can add the validation of the task in ReportActionCompose in a similar way that we do for maxCommentLength during onValueChange:

https://github.com/Expensify/App/blob/d009d8ed4830c7d0562c524c991b8a688e2b4869/src/pages/home/report/ReportActionCompose/ReportActionCompose.tsx#L512-L517

We can update that block and add a validateComment instead of validateCommentMaxLength(value, {reportID});.

What validateComment would do is use the refactoring I mentioned in point A and validate the text as the user types:

           const taskRequest= parseTask(text);
            if (taskRequest.isTask) {
                validateTask();
            } else
                validateCommentMaxLength(value, {reportID});;
        },

the validateTask function would set a state on the task isTask and an error taskValidationError which we would use to show the message. We would replace the following line: https://github.com/Expensify/App/blob/d009d8ed4830c7d0562c524c991b8a688e2b4869/src/pages/home/report/ReportActionCompose/ReportActionCompose.tsx#L564

With this block:

 {hasExceededMaxCommentLength && <CommentValidationError error={translate('composer.commentExceededMaxLength', {formattedMaxLength: numberFormat(CONST.MAX_COMMENT_LENGTH)})}/>}
 {isTask && !taskValidationError <CommentValidationError error={translate('composer.titleExceededMaxLength', {formattedMaxLength: numberFormat(CONST.MAX_COMMENT_LENGTH)}}}/>}

CommentValidationError is a new component that would refactor the existing ExceededCommentLength into a reusable component. Option C. To solve the problem of option A loosing the text after submit. Add a validation step before onSubmit to the composer component that we can use to do validations before the text is submited

Option A is easier but does not retain the text after it is submited but the title was too long, this will be annoying if the user is actually typing a long text. That is solved by options B and C but they both are more complex.

What alternative solutions did you explore? (Optional)

[klajdipaja]

Updated Proposal https://github.com/Expensify/App/issues/49290#issuecomment-2363356092 to add two more options.

[greg-schroeder]

@alitoshmatov will review proposals shortly!

[melvin-bot[bot]]

@greg-schroeder, @alitoshmatov Eep! 4 days overdue now. Issues have feelings too...

[alitoshmatov]

@skg0525 Thank you for your proposal. When we paste it it is being pasted as url link which should work as normal(you can test it by typing url manually).

[alitoshmatov]

@klajdipaja Thank you for proposal. You are right about root cause of the error it is showing, but the main issue in OP is still present even if we meet length requirement.

[melvin-bot[bot]]

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

[klajdipaja]

@alitoshmatov yes you are right.

We also need to check onSubmit that a user is pasting syntax that represents images or videos and if they do to consider that syntax instead of the Task. To do that we need to use the regex's from: https://github.com/Expensify/expensify-common/blob/75869597915c75812e85880c686294f12c112698/lib/ExpensiMark.ts#L55-60 And if the message matches one of them we do not create a task. From my proposal isTaskwould be false ifMARKDOWN_IMAGE_REGEX.test(text)|| MARKDOWN_VIDEO_REGEX.matches(text) and on submit would not call handleCreateTask:

  const onSubmitComment = useCallback(
        (text: string) => {
            const taskRequest= parseTask(text);
            if (taskRequest.isTask) {
                handleCreateTask(taskRequest);
            } else
                Report.addComment(report.reportID, text);
        },
        // eslint-disable-next-line react-compiler/react-compiler, react-hooks/exhaustive-deps
        [report.reportID, handleCreateTask, error],
    );

The regex matching would be great if we could expose as functions from ExpensiMark parser extractImagesInMarkdownComment and extractVideosInMarkdownComment similarly to extractLinksInMarkdownComment

[skg0525]

@skg0525 Thank you for your proposal. When we paste it it is being pasted as url link which should work as normal(you can test it by typing url manually).

@alitoshmatov, I didn't paste the exact test strings, but I believe with some adjustments to the regex, we can meet all the requirements without overcomplicating things. The issue stems from how the regex currently treats anything that starts with '[]' as a task. Right now, it splits the string into three parts:

It checks for a task. It identifies mentions or email addresses. The remaining part is treated as the task title. However, it's not accounting for cases where the third part should not be an attachment. When an attachment is long-pressed and copied, it takes the form of '' or '!(URL)'.

I've updated the regex with an explicit check to ensure URL attachments aren't considered as the third part (the task title).

Improved Regex for Task Rule Matching: `const taskRegex = /^[([^]])]\s+(?:@([^\s@]+(?:@\w+.\w+)?))?\s([^!]*)/;

// Match the string against the regex const match = taskString.match(taskRegex);

// Check if there's a match and group 3 (title) exists if (!match || !match[3]) { return false; }

// Check if group 3 contains disallowed URLs const disallowedUrls = [ "!([w|https].)", // Matches URLs starting with "!" or "https" "!\[([^\]])\]\(.*\)", // Matches image markdown with links ];

for (const urlRegex of disallowedUrls) { if (new RegExp(urlRegex).test(match[3])) { return false; } }`

I have tested this code with multiple different inputs, some of them are as follows:

`const task1 = "[] This is a valid task"; --- This should VALID TASK

const task2 = " @[email protected] This is another NOT a task"; ---NOT A TASK

const task3 = "[] !([https://www.expensify.com/invalid-url])"; // Invalid - URL starting with ![]() ---NOT A TASK

const task4 = "[] This is valid"; // Invalid - Image URL after title ---NOT A TASK `

[alitoshmatov]

@klajdipaja Still, I think the main issue here should be that long texts are not correctly wrapped in android web, you can try similar thing on ios and it wraps correctly without overflowing.

I think we should solve this issue first. Then we can discuss if we should prevent creating tasks with invalid titles

[alitoshmatov]

Still, I think the main issue here should be that long texts are not correctly wrapped in android web, you can try similar thing on ios and it wraps correctly without overflowing.

@skg0525 You should also consider my point. The issue here is not that user is trying to create a task with invalid title.

[klajdipaja]

@alitoshmatov Uh sorry, focused too much on the error and missed the actual expected result from the reporter. That is a simple style change in the https://github.com/Expensify/App/blob/main/src/components/ReportActionItem/TaskPreview.tsx component. In this line we need to add styles for word-wrap and change it to this one:

style={[styles.alignSelfCenter, styles.flex1, styles.flexRow, styles.breakWord, styles.preWrap]}

And we also need to align the arrow vertically by wrapping it in a view and adding styles.alignSelfCenter to it's style.

<View style={[styles.alignSelfCenter]}>
                    <Icon
                        src={Expensicons.ArrowRight}
                        fill={StyleUtils.getIconFillColor(getButtonState(isHovered))}
                    />
                </View>

The output is this:

[klajdipaja]

@alitoshmatov my latest comment would be my proposal on how to solve the reported issue. Should I create another formal proposal for that? I feel like my initial proposal contains valuable information on the error that is seen in the video, unless there's another issue open for that already

[melvin-bot[bot]]

@greg-schroeder @alitoshmatov 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!

[greg-schroeder]

vip-vsb is now fully paused/closed, so I'm going to close this as it doesn't really fit in #newdot-quality or any other active project.