[$1000] BUG: The text inside the composer input and the menu on the settings page got highlighted when triple-click on the last report chat reported by @mollfpr

[kavimuru]

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!

---

Action Performed:

1. Go to the composer input and type something
2. Triple-click to highlight the text of the last report chat
3. Click the account avatar to open the settings modal in the rightDocked side modal

Expected Result:

Only the intended text selection is highlighted.

Actual Result:

The intended text select, text inside the composer input, and the menu on the settings page are highlighted.

Workaround:

unknown

Platform:

Where is this issue occurring?

• Web
• Desktop App

Version Number: 1.2.18-2 Reproducible in staging?: y Reproducible in production?: y Email or phone of affected tester (no customers): Logs: https://stackoverflow.com/c/expensify/questions/4856 Notes/Photos/Videos:

https://user-images.githubusercontent.com/43996225/196834834-73f86df0-db5a-4c75-a57f-ae420bfcd880.mp4

https://user-images.githubusercontent.com/43996225/196834880-bf4a8f77-4043-4eea-9907-1555d75afe27.mov

Upwork URL: https://www.upwork.com/jobs/~019c6881ba683194f2 Issue reported by: @mollfpr Slack conversation: https://expensify.slack.com/archives/C01GTK53T8Q/p1666188981167469

View all open jobs on GitHub

[melvin-bot[bot]]

Triggered auto assignment to @trjExpensify (AutoAssignerTriage), see https://stackoverflow.com/c/expensify/questions/4749 for more details.

[melvin-bot[bot]]

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

[melvin-bot[bot]]

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

[melvin-bot[bot]]

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

[trjExpensify]

I can reproduce this reliably on v1.2.18-4. Marking as external.

[trjExpensify]

Job on Upwork here: https://www.upwork.com/jobs/~019c6881ba683194f2

[hungvu193]

Proposal Disable the text selection highlighting can help us fix this problem.

Solution Apply this style to the ScreenWrapper of InitialSettingPage:

- <ScreenWrapper >
+  <ScreenWrapper style={[styles.settingsPageWrapper]}>
                <HeaderWithCloseButton
                    title={this.props.translate('common.settings')}
                    onCloseButtonPress={() => Navigation.dismissModal(true)}
                />

+settingsPageWrapper: {
+      MozUserSelect: "none",
+      WebkitUserSelect: "none",
+      MsUserSelect: "none",
+}

We also need to update this style to other screen in this page too (Workspace, profile, Preference....)

https://user-images.githubusercontent.com/16502320/197338308-ce43ee26-2e06-4f19-a751-e5f53996a138.mov

[melvin-bot[bot]]

Looks like something related to react-navigation may have been mentioned in this issue discussion.

As a reminder, please make sure that all proposals are not workarounds and that any and all attempt to fix the issue holistically have been made before proceeding with a solution. Proposals to change our DeprecatedCustomActions.js files should not be accepted.

Feel free to drop a note in #expensify-open-source with any questions.

[melvin-bot[bot]]

@chiragsalian, @trjExpensify, @thesahindia Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[melvin-bot[bot]]

@chiragsalian, @trjExpensify, @thesahindia Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[thesahindia]

@hungvu193, the text inside the composer still gets selected. I think your solution is a workaround. We need to find the root cause.

[wildan-m]

Proposal

Root cause

I think it's related to how chrome behaves with a triple click. They have a related bug with status won't fix https://bugs.chromium.org/p/chromium/issues/detail?id=575136

Workaround

My proposed solution is also a workaround, but let me know if it's acceptable.

The idea is to override the selection with the desired contents.

We can wrap ReportActionItemFragment with a div and listen triple-click events to programmatically re-select the text contents.

git diff src/pages/home/report/ReportActionItemMessage.js
diff --git a/src/pages/home/report/ReportActionItemMessage.js b/src/pages/home/report/ReportActionItemMessage.js
index d226ac9ced..af88ac4e5e 100644
--- a/src/pages/home/report/ReportActionItemMessage.js
+++ b/src/pages/home/report/ReportActionItemMessage.js
@@ -26,18 +26,33 @@ const defaultProps = {
     style: [],
 };
 
+const selectElementContents = (el) => {
+    const sel = window.getSelection();
+    sel.removeAllRanges();
+    const range = document.createRange();
+    range.selectNodeContents(el);
+    sel.addRange(range);
+};
+
+const onClick = (e) => {
+    if (e.detail !== 3) { return; }
+    selectElementContents(e.target);
+};
+
 const ReportActionItemMessage = props => (
     <View style={[styles.chatItemMessage, ...props.style]}>
         {_.map(_.compact(props.action.previousMessage || props.action.message), (fragment, index) => (
-            <ReportActionItemFragment
-                key={`actionFragment-${props.action.sequenceNumber}-${index}`}
-                fragment={fragment}
-                isAttachment={props.action.isAttachment}
-                attachmentInfo={props.action.attachmentInfo}
-                source={lodashGet(props.action, 'originalMessage.source')}
-                loading={props.action.isLoading}
-                style={props.style}
-            />
+            <div role="presentation" onClick={onClick}>
+                <ReportActionItemFragment
+                    key={`actionFragment-${props.action.sequenceNumber}-${index}`}
+                    fragment={fragment}
+                    isAttachment={props.action.isAttachment}
+                    attachmentInfo={props.action.attachmentInfo}
+                    source={lodashGet(props.action, 'originalMessage.source')}
+                    loading={props.action.isLoading}
+                    style={props.style}
+                />
+            </div>
         ))}
     </View>
 );

I know this solution will still have the wrong selection at first, but it will immediately correct the text selection. Please have a look at the video.

https://user-images.githubusercontent.com/11847279/197930547-a9dd6a91-dcdf-4c3e-a32c-53ad4ec999f8.mp4

[thesahindia]

I think it's related to how chrome behaves with a triple click. They have a related bug with status won't fix

@wildan-m, the issue can be seen on other browsers as well.

Waiting for more proposals.

[wildan-m]

Proposal (Updated)

@thesahindia, you are right, I can also reproduce it in Safari.

I still believe that it's caused by the different triple-click behavior of browsers At this time, behavior of default action of triple-click isn't specified the W3C spec Other repos also experience similar issues: https://github.com/facebook/lexical/issues/2660 https://github.com/ckeditor/ckeditor4/issues/484 https://github.com/ckeditor/ckeditor4/issues/3118

I've resolved glitches in my previous solution by changing the onClick event to onMouseDown and adding preventDefault if the subsequent click is more than two.

https://github.com/Expensify/App/blob/73922083c9bc4f2b1f050406b6e3b24c6ba98fc7/src/pages/home/report/ReportActionItemMessage/index.web.js#L10-L40

These changes will be implemented for the web and desktop only.

Complete comparison: https://github.com/Expensify/App/commit/73922083c9bc4f2b1f050406b6e3b24c6ba98fc7

No more flickering:

https://user-images.githubusercontent.com/11847279/198174311-e405f8b6-6ac9-4d6f-909c-0b350869832a.mp4

[thesahindia]

I don't prefer adding another wrapper, I wanna wait to see if someone can find the root cause and propose a more simpler solution.

[trjExpensify]

@thesahindia presumably you believe this is a regression, right?

I still believe that it's caused by the different triple-click behavior of browsers

@wildan-m, we see the issue on the desktop app as well (not just in the Web browser), so how can that be the case?

[trjExpensify]

Job price doubled to $500 on Upwork: https://www.upwork.com/jobs/~019c6881ba683194f2

[wildan-m]

@trjExpensify The desktop uses electron and electron use chromium, so I think it will behave the same with chrome browser

[trjExpensify]

Ah, I see. So @thesahindia, to confirm there's a root cause for this regression in App, have you gone back like 10 versions or something to see if it's there?

[trjExpensify]

Outside of that, the code highlighted in this comment was added back in Sept by @marcaaron in this PR released in 1.2.2-0, so maybe there are some clues there?

[thesahindia]

I randomly tested this at v1.2.13-4 and wasn't able to repro, so It's definitely a regression. I think https://github.com/Expensify/App/issues/11921 is similar and I wasn't able to repro it at v1.2.13-4 I think we should merge them?

[trjExpensify]

Aren't they only similar insofar as it's a bug when triple clicking? It depends really if the root cause is the same?

I randomly tested this at v1.2.13-4 and wasn't able to repro, so It's definitely a regression.

Nice! That's a good start. This was reported in 1.2.18-2 so we have a window.

[thesahindia]

It depends really if the root cause is the same?

I think the root cause is probably the same but let's wait as I am not 100% sure

[trjExpensify]

Yep!

@wildan-m, are you up for taking a look at the releases in between v1.2.13-4 and 1.2.18-2 to narrow down what the probable cause was? Something we changed introduced this, so starting there to inform the proposal seems like a good idea.

[wildan-m]

@trjExpensify sure, I'll check it

[osama256]

Proposal

Using drawer legacy Implementation will fix this issue and this as well.

this issue happened because we were using @react-navigation/drawer version below 6.3.2, in these versions the drawer use useLegacyImplementation={true} by default so we didn't notice this issue but in higher versions of @react-navigation/drawer the part of code that causes useLegacyImplementation={true} always true has been removed so we need to make it manually.

in src/libs/Navigation/AppNavigator/BaseDrawerNavigator :

        const content = (
            <Drawer.Navigator
+               useLegacyImplementation
                backBehavior="none"
                key="BaseDrawerNavigator"
                defaultStatus={this.state.defaultStatus}
                sceneContainerStyle={styles.navigationSceneContainer}
                drawerContent={this.props.drawerContent}
                screenOptions={{

Result:

https://user-images.githubusercontent.com/51829206/199309238-8dd9b046-f590-4d30-8d94-228cb4372729.mp4

[trjExpensify]

Hey @osama256, looks promising! Can you type something in the compose box to illustrate the fix as per the repro steps in the OP? Thanks!

[osama256]

Can you type something in the compose box to illustrate the fix as per the repro steps in the OP?

@trjExpensify Sure.

https://user-images.githubusercontent.com/51829206/199314486-58a67380-d467-4ff8-87ce-045278f783a1.mp4

[thesahindia]

@osama256, I am getting error when testing this

[osama256]

I am getting error when testing this

Ops, i was testing on @react-navigation/drawerversion 6.3.2 it has the same issue but it seems it using a lower version of react-native-reanimated.

This is so interesting, in @react-navigation/drawer below 6.3.2 useLegacyImplementation is always true, but in the current version useLegacyImplementation is always false because they are using a higher version of react-native-reanimated that doesn't allow useLegacyImplementation.

Maybe we can downgrade to 6.3.1 until react-native-reanimated make it possible to use useLegacyImplementation.