[HOLD for payment 2023-09-29] [$1000] Android - Login - Linkified email from external app is pasted as linkified email in email field

[lanitochka17]

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. Copy a linkified email address from external app (like gmail).
2. Launch New Expensify app.
3. Long tap on the email field.
4. Select Paste.

Expected Result:

Email is not linkified

Actual Result:

Email is linkified

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.3.31.2

Reproducible in staging?: Yes

Reproducible in production?: Yes

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: Any additional supporting documentation

Bug6103773_Screen_Recording_20230623_181250_New_Expensify.mp4

Expensify/Expensify Issue URL:

Issue reported by: Applause - Internal Team

Slack conversation:

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01fec00bf5a9d050cf
• Upwork Job ID: 1673491584987435008
• Last Price Increase: 2023-06-27

[melvin-bot]

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

[melvin-bot]

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

[cubuspl42]

Proposal

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

When pasting rich text to the e-mail field, the formatting is preserved, although we'd like the text to be plain text.

What is the root cause of that problem?

On Android, TextInput for some reason is rich text input by default. It can contain initial formatted text, for example:

<TextInput>
    Hello, <Text style={{color:'red'}}>World</Text>
</TextInput>

Also, formatted text can be pasted by the user.

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

Surprisingly, there is no way to disable the rich text properties of the TextInput.

There is a workaround, though. One can heuristically detect when the text is pasted and change it to some other text (so the internal formatting markers are cleared), but immediately change it back.

We could introduce these helpers:

const zeroWidthSpace = '\u8203';

/**
 * A very simple heuristic for detecting whether the text was pasted
 *
 * @param {String} oldText
 * @param {String} newText
 * @return {Boolean}
 */
function isTextPasted(oldText, newText) {
    return Math.abs(newText.length - oldText.length) > 1;
}

/**
 * A helper for setting a TextInput-managed state, which ensures that the
 * text keeps being plain, even when formatted text is pasted by the user.
 *
 * @param {Object} component
 * @param {Function} buildState
 * @param {String} oldText
 * @param {String} newText
 */
function setPlainTextInputState(component, buildState, oldText, newText) {
    if (isTextPasted(oldText, newText)) {
        // Set a similar but different text, so TextInput clears formatting markers
        component.setState(buildState(
            `${zeroWidthSpace}${newText}${zeroWidthSpace}`,
        ), () => {
            // Change back to the pasted text (in the plain text)
            component.setState(buildState(newText));
        });
    } else {
        // If it doesn't look like a paste, set the state directly to minimize flickering
        component.setState(buildState(newText));
    }
}

And then use them in LoginForm's onTextInput method:

    /**
     * Handle text input and clear formError upon text change
     *
     * @param {String} newText
     */
    onTextInput(newText) {
        setPlainTextInputState(
            this,
            (text) => ({login: text}),
            this.state.login,
            newText,
        );

        this.setState({
            formError: null,
        });

        // ...
    }

[Christinadobrzyn]

I'm not able to replicate this - reaching out in Slack to get more specific directions on how the email is copied.

[cubuspl42]

@Christinadobrzyn

You can copy any link, for what it's worth. Let's say from here. When I copied from Chrome, I had to use text selection.

RichText-1-compressed.mp4

[Christinadobrzyn]

I might not be able to get this to work on Browserstack but it seems like enough people can reproduce it so I'll send it to External.

[melvin-bot]

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

[melvin-bot]

Triggered auto assignment to @Gonals (Engineering), see https://stackoverflow.com/c/expensify/questions/4319 for more details.

[melvin-bot]

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

[melvin-bot]

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

[fabOnReact]

Proposal

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

Text is copy pasted as rich text on Android TextInput instead of Plain Text.

What is the root cause of that problem?

Android EditText and iOS UITextField/UITextView have different copy/paste behavior.

• Android TextInput copies/pastes rich text
• iOS UITextField copies/pastes plain text.

iOS (react-native)	Android (react-native)
Simulator.Screen.Recording.-.iPhone.14.Pro.-.2023-06-27.at.21.41.04.mp4	repro_android.mov

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

The issue is a bug in react-native facebook/react-native#31442:

1. The JavaScript TextInput and ReactEditText Android state are not in sync
2. The TextInput Android Native state over-rides the JavaScript state.
3. onChangeText passes a plain text string to JavaScript, not rich text (text with spans and styles).

More info at #21411 (comment)

The solution consists of:

• over-riding onTextContextMenuItem in ReactEditText to copy/paste plain text instead of rich-text (https://stackoverflow.com/a/45319485/7295772).
• removing the Paste as plaintext option from the insert and selection context menu ([HOLD for payment 2023-09-29] [$1000] Android - Login - Linkified email from external app is pasted as linkified email in email field #21411 (comment))

I have included below the test performed in an Android App.

Reproducing the issue on Android

2023-06-27.22-28-06.mp4

Fixing the issue on Android

Sourcecode https://github.com/fabriziobertoglio1987/text-input-cursor-flickering-android/blob/fix-copy-paste/app/src/main/java/com/example/myapplication/CustomEditText.java

2023-06-28.21-26-53.mp4

Testing the solution on React Native

2023-06-29.17-49-09.mp4

[cubuspl42]

@fabriziobertoglio1987 Just for the sake of clarity, there's an open issue in React Native. It's not confirmed yet whether it's considered a bug or intended behavior.

[s77rt]

@cubuspl42 Thanks for the proposal. Your RCA makes sense but I don't think we should follow the workaround solution especially since this is broken at a lower level.

[s77rt]

@fabriziobertoglio1987 Thanks for the proposal. Your RCA makes sense. I like the simplicity of the solution treating "Paste" as "Paste as plaintext" but then we will have two options that do the same thing, not really a blocker, but can we control the context menu options to show?

[fabOnReact]

Thanks @s77rt. This is my update for today.

Thanks for the proposal. Your RCA makes sense. I like the simplicity of the solution treating "Paste" as "Paste as plaintext" but then we will have two options that do the same thing, not really a blocker, but can we control the context menu options to show?

I removed the Paste as plaintext option from the insert and selection:

Removed the option in insert mode with setCustomInsertionActionModeCallback

The user opens the context menu with the long press action on the cursor.

Before applying the Fix

Result:

• Text is pasted as rich text
• paste as plain text option is available in the context menu while in insert mode

2023-06-28.21-14-46.mp4

After applying the Fix

Result:

• Text is pasted as plain text
• paste as plain text option is not available in the context menu while in insert mode

Sourcecode https://github.com/fabriziobertoglio1987/text-input-cursor-flickering-android/blob/fix-copy-paste/app/src/main/java/com/example/myapplication/CustomEditText.java

2023-06-28.21-26-53.mp4

Removed the option in selection mode with setCustomSelectionActionModeCallback

The user opens the context menu with the long press action on the selected text.

Before applying the Fix

Result:

• Text is pasted as rich text
• paste as plain text option is available in the context menu while in selection mode

2023-06-28.21-24-37.mp4

After applying the Fix

Result:

• Text is pasted as plain text
• paste as plain text option is not available in the context menu while in selection mode

Sourcecode: https://user-images.githubusercontent.com/24992535/249484884-44e4577c-3ca7-4ec2-9ecd-364c7c2ee610.mp4

2023-06-28.21-27-15.mp4

[fabOnReact]

It's not confirmed yet whether it's considered a bug or intended behavior.

Native and JavaScript state are not in sync. The EditText Native style changes the style of a JavaScript Controlled TextInput.

https://snack.expo.dev/@fabrizio.bertoglio/controlled-component-style-not-working

• The JavaScript TextInput and ReactEditText Android state are not in sync
• The TextInput Native state over-rides the JavaScript state.

Detailed steps to reproduce the issue:

1. The user copy pastes rich text in a controlled component
2. The pasted rich text is a link and applies highlight color of blue and underline
3. The TextInput controlled component applies custom style color of red to the entire text and no other style. The text is supposed to have red color and no underline.
4. The rich text facebook/react-native#380 does not change color. Deleting the text, changes the color from blue to red. The new typed text does not have the correct color and the correct underline style.

Conclusion:

1. The style set in JavaScript TextInput component does not over-ride the native Android style
2. Copy Pasting rich text in the TextInput creates two different states between JavaScript and Android.

• The JavaScript TextInput internal state is

<Text style={{color: "red"}}>attached screenshot. https://githddd</Text>

• Android ReactEditText internal state is

<Text style={{color: "red"}}>attached screenshot. 
   <Text style={{color: "blue", textDecorationColor: "blue", textDecorationLine: "underline"}}>https://gith</Text>
   <Text style={{color: "red", textDecorationColor: "red", textDecorationLine: "underline" }}>ddd</Text>
</Text>

On Android different Span will expand to the left/right characters, and not respect the style defined in JavaScript.

3. onChangeText passes to JavaScript callback the plain text instead of rich text

2023-06-28.21-56-29.mp4

[s77rt]

@fabriziobertoglio1987 Thanks for the update. I think we can start with treating "Paste" as "Paste as plaintext" (make a PR for that). Then we can make a follow up PR to hide the redundant "Paste as plaintext" if needed.

🎀 👀 🎀 C+ reviewed

BTW, can you please use heading styles instead of <details>. The details tag does not seem to support indent and this makes it hard to read the proposal. Heading levels are easier to read I think.

[melvin-bot]

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

[fabOnReact]

Update 10/09

I prepared a branch for the PR. I tested the branch and did not detect issue, but most of the patches do not work on 0.72.4.

I will try to complete the task by the 13th of September, as I will be away for some time from the 14th of September.

Branch https://github.com/Expensify/App/compare/main...fabriziobertoglio1987:App:upgrade-react-native-0.72.4?expand=1

Patches that can be deleted because already merged in the Upstream branch. May require updating dependency.

• react-navigation material-top already merged to upstream (patch, upstream PR, changelog, release). Needs dependency update.

Patches that have been reapplied for 0.72.4.

• Image manipulator (changing compile and targetSdk to 30) (upstream (Expensify fork))
• react-native-sdk not merged in upstream (patch, upstream).
• NumberOfLines (1000+ lines) https://github.com/Expensify/App/blob/main/patches/react-native%2B0.72.3%2B002%2BNumberOfLines.patch (more info in this comment)
• VerticalScrollBarPosition (few lines) https://github.com/Expensify/App/blob/main/patches/react-native%2B0.72.3%2B003%2BVerticalScrollBarPosition.patch

Patches that had no issues with 0.72.4:

• react-navigation-native (300 lines, patch, upstream). Needs dependency update. They are currently working on updating the patch with this Issue (PR), I will contact the author to apply it on 0.72.4.
• react-navigation+stack+6.3.16+001+initial
• react-navigation+stack+6.3.16+002+dontDetachScreen
• react-native+0.72.4+001+initial
• react-native ModalKeyboardFlashing (patch)
• react-native-fast-image https://github.com/Expensify/App/blob/main/patches/react-native-fast-image%2B8.6.3.patch
• react-native-modal (few lines) https://github.com/Expensify/App/blob/main/patches/react-native-modal+13.0.1.patch
• Vision-camera (few lines patch)
• React-native-web-lottie (1 line patch)

Issue ds300/patch-package#487 https://expensify.slack.com/archives/C01GTK53T8Q/p1692709363724909

[fabOnReact]

Update 12/09

Update to react-native 0.72.4
No issues updating to react-native 0.72.4. All patches work except NumberOfLines and VerticalScrollBarPosition. I did not detect issues in the Android functionalities.

Issues with patch NumberOfLines
Re-applying NumberOfLines patch fails because of the missing binary ReactAndroid/hermes-engine/build/hermes/API/hermes/hermes.framework/hermes, but the project builds/runs without issues on Android. The problem was reported in the patch-package repo with issue #445. More info on the slack discussion. I manually verified the diff and there are no errors. Probably patch-package needs a hermes binary to run javascript with hermes.

https://hermesengine.dev/docs/building-and-running/#executing-javascript-with-hermes

This tool compiles JavaScript to Hermes bytecode. It can also execute JavaScript, from source or bytecode or be used as a REPL.

Maybe patch-package needs to compile the JavaScript changes to bytecode using the hermes binary?
It is possible that some steps are missing with npm run-android compare to building react-native from source

https://github.com/fabriziobertoglio1987/react-native/blob/abbd60ed0cf1ef753a8c0bf27894b5bd87f7e21d/packages/react-native/ReactAndroid/hermes-engine/build.gradle#L235

Tomorrow I will verify if the hermes binary is present under ReactAndroid/hermes-engine/build/hermes/API/hermes/hermes.framework/Versions/Current when building react-native from source.

[fabOnReact]

Update 13/09

Published DRAFT pull request #27320 to upgrade to react-native 0.72.4

[fabOnReact]

Update 15/09

Completed DRAFT pull request #27320 to upgrade to react-native 0.72.4. I will move the PR to ready to review this weekend.

[fabOnReact]

Update 16/09

Moved the PR #27320 to ready to review.

[melvin-bot]

Based on my calculations, the pull request did not get merged within 3 working days of assignment. Please, check out my computations here:

• when @fabriziobertoglio1987 got assigned: 2023-07-04 07:59:57 Z
• when the PR got merged: 2023-09-20 03:24:54 UTC
• days elapsed: 55

On to the next one 🚀

[melvin-bot]

Reviewing label has been removed, please complete the "BugZero Checklist".

[melvin-bot]

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.3.72-11 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• Upgrade react native to 0.72.4 #27320

If no regressions arise, payment will be issued on 2023-09-29. 🎊

After the hold period is over and BZ checklist items are completed, please complete any of the applicable payments for this issue, and check them off once done.

• External issue reporter
• Contributor that fixed the issue
• Contributor+ that helped on the issue and/or PR

For reference, here are some details about the assignees on this issue:

• @s77rt requires payment
• @fabriziobertoglio1987 requires payment

As a reminder, here are the bonuses/penalties that should be applied for any External issue:

• Merged PR within 3 business days of assignment - 50% bonus
• Merged PR more than 9 business days after assignment - 50% penalty

[melvin-bot]

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

• [@s77rt] The PR that introduced the bug has been identified. Link to the PR:
• [@s77rt] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment:
• [@s77rt] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion:
• [@s77rt] Determine if we should create a regression test for this bug.
• [@s77rt] 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.
• [@Christinadobrzyn] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[s77rt]

• The PR that introduced the bug has been identified: N/A This was a bug in RN
• The offending PR has been commented on: N/A
• A discussion in #expensify-bugs has been started: N/A
• Determine if we should create a regression test for this bug: No

[Christinadobrzyn]

Payouts due:

Issue Reporter: NA
Contributor: $1000 @fabriziobertoglio1987
Contributor+: $1000 @s77rt

Eligible for 50% #urgency bonus? N based on #21411 (comment) but also it looks like the PR was created Sept 13th & merged Sept 19th - does that sound right?

Upwork job is here.

[s77rt]

@Christinadobrzyn Can you please double check the Upwork job. It seems unavailable.

[melvin-bot]

@s77rt, @Gonals, @fabriziobertoglio1987, @Christinadobrzyn Whoops! This issue is 2 days overdue. Let's get this updated quick!

[Christinadobrzyn]

Hi @s77rt the Upwork job is closed, sorry I didn't realise that meant you couldn't see it. You and @fabriziobertoglio1987 were automatically hired so I can pay you through the closed job still.

Will you confirm the payment details here are good and I'll pay this? #21411 (comment)

[s77rt]

@Christinadobrzyn The payout summary looks correct 👍

[Christinadobrzyn]

Perfect! Thanks for confirming @s77rt!

I have paid both you and @fabriziobertoglio1987 based on the payment structure here

I'll close this out but please let me know if you need anything else!