Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rich text can be pasted into input box on Android #4660

Open
WesleyAC opened this issue Apr 14, 2021 · 10 comments
Open

Rich text can be pasted into input box on Android #4660

WesleyAC opened this issue Apr 14, 2021 · 10 comments
Labels
a-Android a-compose/send Compose box, autocomplete, camera/upload, outbox, sending bug help wanted upstream: RN Issues related to an issue in React Native

Comments

@WesleyAC
Copy link
Contributor

When a user copies and pastes formatted text (for instance, from a web browser), the rich text is pasted into the compose box. This is incorrect, we should strip this formatting instead.

See this CZO thread for details.

The next step here is to look at what we get from onChangeText when rich text is pasted in, since that seems like the most likely route to fix this.

@WesleyAC WesleyAC added bug help wanted a-Android a-compose/send Compose box, autocomplete, camera/upload, outbox, sending labels Apr 14, 2021
@vaibhavs2
Copy link
Contributor

  • simple texts are being stored in state,

  • Only the \n next Line is retained by TextInput
    state

@chrisbobbe
Copy link
Contributor

chrisbobbe commented Apr 16, 2021

@vaibhavs2, it looks like Wesley is specifically asking about what gets passed to onChangeText; could you log all the arguments it's getting called with when you paste rich text in?

@vaibhavs2
Copy link
Contributor

The function has single argument that get passed to onChangeText and from the argument state is getting data that's why I directly comment with state data.
log

I tried
<TextInput onChangeText={(value)=>{this.setState({message:value})}} multiline style={{borderWidth:2, borderColor:'red',}}>
<Text style={{ fontWeight:'bold'}}>hey therer</Text>
<Text style = {{color:'red'}}> I don't know what to write</Text>
<Text style = {{fontWeight:'bold', fontSize:20}}> I don't know what to write</Text>
</TextInput>

Rich text do visible in TextInput but `onChangeText` doesn't return rich text, it gives simple text and that is being stored in state.

simulate

I think TextInput is doing something same under the hood when we paste rich text from somewhere and return us simple text

@gnprice
Copy link
Member

gnprice commented Apr 16, 2021

Thanks @vaibhavs2 !

So the answer to what onChangeText gives us is that it's just the plain text.

Which agrees with the RN docs, which say the type it passes to that callback is just string. So that fits, at least.

Given that RN is only actually giving us the plain text, not any of the formatting, it seems like an outright bug in RN that it accepts formatting on paste and shows the formatting in the input.

Though from what we saw in that chat thread linked above, it seems like Android may not give RN an easy way to fix this bug.

I think the next step is to search the RN issue tracker and see if there's an existing issue. If there is, then link to it here.

If there isn't, then the step after that would be to file a good bug report on RN. The key work to do to make a good bug report is that it shouldn't be about Zulip; instead it should have a simple repro, where you link to a tiny new RN app that demonstrates the bug and does nothing else. That way someone interested in fixing the RN bug can focus on that minimal repro.

@gnprice gnprice added the upstream: RN Issues related to an issue in React Native label Apr 16, 2021
@vaibhavs2
Copy link
Contributor

Issue Created
facebook/react-native#31442

@alya
Copy link
Collaborator

alya commented Oct 25, 2021

As described on this CZO thread, this issue can lead to a very awkward user experience.

Screen_Recording_20211023-061041_Zulip.mp4

@gnprice
Copy link
Member

gnprice commented Aug 2, 2022

For how to go about resolving this issue:

At the Android level, there's a handy solution that @WesleyAC found in a Stack Overflow answer:

A perfect and easy way: Override the EditText's onTextContextMenuItem and intercept the android.R.id.paste to be android.R.id.pasteAsPlainText

This works on Android >= M, which covers all the Android versions we support.

Then to apply that in Zulip:

  • I think the ideal solution is that React Native upstream should apply that fix in the implementation of the TextInput component.

    (It's already the case that the only data TextInput actually exposes to the app is the plain text -- so the fact that it lets the user paste formatted text, and shows that formatting back, is basically a bug in RN because there's no way the app can end up respecting that formatting.)

  • Alternatively, without any changes to RN, I think it should be possible to make our own component with a name like FixedTextInput, using inheritance in the native code to avoid needing to duplicate most of the existing implementation.

    OTOH TextInput has a more complicated implementation than most components, so this may not be simple. The relevant subclass of Android EditText is in ReactAndroid/src/main/java/com/facebook/react/views/textinput/ReactEditText.java, and we should be able to make a pretty simple subclass of that that just adds the needed behavior by overriding onTextContextMenuItem… but then I'm not sure what other scaffolding is needed around that in order to get a React component that can be used in place of the upstream TextInput.

So one route to a fix would be to try the second approach, with a subclass, and then when it's working send a PR upstream to React Native to fix TextInput itself. This would be good, provided that it doesn't end up involving too much extra scaffolding, because it'd mean a straightforward way for us to have the fix immediately while the fix works its way through getting merged upstream and then going into a RN release we upgrade to.

Alternatively, we could go straight for the upstream fix. Then there'd be a lag of a few months before we're on an RN release that has the fix. We could simply accept that lag, or we'd also have the option of switching to our own patched version of RN with the fix applied.

@gnprice
Copy link
Member

gnprice commented Jul 31, 2023

The upstream issue facebook/react-native#31442 has apparently been fixed! Thanks @vaibhavs2 for filing it 🙂

@gnprice
Copy link
Member

gnprice commented Jul 31, 2023

(And from the PR thread — at facebook/react-native#38189 (comment) and facebook/react-native#38189 (review) — it's clear that our discussion here was key in getting this fixed, by making it clear why the existing behavior was a bug. So good work everyone.)

@fabOnReact
Copy link

I sent a cherry pick request for this commit in 0.72.4, if they decline the cherry-pick request, we can ask again in the next release 0.73. Thanks a lot for the support! 🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a-Android a-compose/send Compose box, autocomplete, camera/upload, outbox, sending bug help wanted upstream: RN Issues related to an issue in React Native
Projects
None yet
Development

No branches or pull requests

6 participants