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

[Android][TextInput] onSelectionChange can have end < start #18579

Closed
3 tasks done
akalin-keybase opened this issue Mar 27, 2018 · 10 comments
Closed
3 tasks done

[Android][TextInput] onSelectionChange can have end < start #18579

akalin-keybase opened this issue Mar 27, 2018 · 10 comments
Labels
Bug Component: TextInput Related to the TextInput component. Good first issue Interested in collaborating? Take a stab at fixing one of these issues. Platform: Android Android applications. Resolution: Locked This issue was locked by the bot.

Comments

@akalin-keybase
Copy link

If one has a TextInput and selects from right to left, on Android the selection on onSelectionChange has end < start. On iOS, start <= end consistently.

Environment

Environment:
OS: macOS High Sierra 10.13.3
Node: 9.9.0
Yarn: 1.5.1
npm: 5.8.0
Watchman: 4.9.0
Xcode: Xcode 9.2 Build version 9C40b
Android Studio: Not Found

Packages: (wanted => installed)
react: 16.3.0-alpha.1 => 16.3.0-alpha.1
react-native: 0.54.2 => 0.54.2

Steps to Reproduce

See https://snack.expo.io/BJUeRfw5M which is for another bug. On a regular simulator you can do shift-left to repro, but that doesn't seem to work on Snack. Instead, click and drag a word with your mouse from right to left. From the console logs, you'll see that end < start on Android. Alas, iOS on Snack doesn't seem to let you select from right to left, so you'll have to fire up the simulator for that. See https://github.com/akalin-keybase/rn-text-input-bugs instead.

Expected Behavior

start <= end consistently.

Actual Behavior

On Android the selection on onSelectionChange has end < start.

@react-native-bot react-native-bot added Platform: Android Android applications. Component: TextInput Related to the TextInput component. labels Mar 27, 2018
@stale
Copy link

stale bot commented Jun 25, 2018

Hey there, it looks like there has been no activity on this issue recently. Has the issue been fixed, or does it still require the community's attention? This issue may be closed if no further activity occurs. You may also label this issue as "For Discussion" or "Good first issue" and I will leave it open. Thank you for your contributions.

@stale stale bot added the Stale There has been a lack of activity on this issue and it may be closed soon. label Jun 25, 2018
@akalin-keybase
Copy link
Author

Not fixed AFAIK

@stale stale bot removed the Stale There has been a lack of activity on this issue and it may be closed soon. label Jun 25, 2018
@stale
Copy link

stale bot commented Dec 21, 2018

Hey there, it looks like there has been no activity on this issue recently. Has the issue been fixed, or does it still require the community's attention? This issue may be closed if no further activity occurs. You may also label this issue as "For Discussion" or "Good first issue" and I will leave it open. Thank you for your contributions.

@stale stale bot added the Stale There has been a lack of activity on this issue and it may be closed soon. label Dec 21, 2018
@akalin-keybase
Copy link
Author

Not fixed AFAIK

@stale stale bot removed the Stale There has been a lack of activity on this issue and it may be closed soon. label Dec 21, 2018
@hramos hramos removed the Bug Report label Feb 6, 2019
@elicwhite
Copy link
Member

Using this code I was able to repro the issue on 0.59:

import React from 'react';
import { StyleSheet, Text, TextInput, View } from 'react-native';

export default class App extends React.Component {
  constructor(props) {
    super(props)
    this.state = { start: 0, end: 0 }
  }

  _onSelectionChange = (event) => {
    const selection = event.nativeEvent.selection
    this.setState({start: selection.start, end: selection.end})
  }

  render() {
    return (
      <View style={styles.container}>
        <Text>{`Start: ${this.state.start}, End: ${this.state.end}`}</Text>
        <TextInput style={styles.textInput} onSelectionChange={this._onSelectionChange} value="initial text" multiline={true} />
      </View>
    );
  }
}

const styles = StyleSheet.create({
  container: {
    flex: 1,
    backgroundColor: '#fff',
    alignItems: 'center',
    justifyContent: 'center',
  },
  textInput: {
    height: 40,
    width: 150,
    borderColor: 'black',
    borderWidth: 1,
  }
});

textselection

@elicwhite elicwhite changed the title TextInput selection can have end < start on Android [Android][TextInput] onSelectionChange can have end < start Mar 19, 2019
@elicwhite
Copy link
Member

This issue can probably be fixed with this change. I don't have my build environment set up so if someone wants to create a PR and test this change, that would be great:

--- a/react-native/ReactAndroid/src/main/java/com/facebook/react/views/textinput/ReactTextInputManager.java
+++ b/react-native/ReactAndroid/src/main/java/com/facebook/react/views/textinput/ReactTextInputManager.java
@@ -958,16 +958,22 @@
       // Android will call us back for both the SELECTION_START span and SELECTION_END span in text
       // To prevent double calling back into js we cache the result of the previous call and only
       // forward it on if we have new values
-      if (mPreviousSelectionStart != start || mPreviousSelectionEnd != end) {
+
+      // Apparently Android might call this with an end value that is less than the start value
+      // Lets normalize them. See https://github.com/facebook/react-native/issues/18579
+      int realStart = Math.min(start, end);
+      int realEnd = Math.max(start, end);
+
+      if (mPreviousSelectionStart != realStart || mPreviousSelectionEnd != realEnd) {
         mEventDispatcher.dispatchEvent(
             new ReactTextInputSelectionEvent(
                 mReactEditText.getId(),
-                start,
-                end
+                realStart,
+                realEnd
             ));

-        mPreviousSelectionStart = start;
-        mPreviousSelectionEnd = end;
+        mPreviousSelectionStart = realStart;
+        mPreviousSelectionEnd = realEnd;
       }
     }
   }

@cpojer cpojer added the Good first issue Interested in collaborating? Take a stab at fixing one of these issues. label May 9, 2019
@DanielHH
Copy link

DanielHH commented May 9, 2019

I am a new to open source contributing and would like to work on this issue.

@DanielHH
Copy link

I am attempting to set up a build environment but am getting the following problem during the build: 'org.gradle.process.internal.ExecException: Process 'command 'C:\Users\Daniel\AppData\Local\Android\android-ndk-r19c\ndk-build.cmd'' finished with non-zero exit value 2' which appears similar to this issue from 2015: #3107.
Is there still a problem building from source on windows?

@Conviley
Copy link

+1

@uqmessias
Copy link
Contributor

@TheSavior
Since this a bug that is there for a while, I'd like to try fixing this issue.
Tonight, I'll set up my dev environment in order to debug and test your diff, if it doesn't solve the issue I'll try something else. Either way, eventually I'll open a PR with the fix as soon as I fix it.

M-i-k-e-l pushed a commit to M-i-k-e-l/react-native that referenced this issue Mar 10, 2020
Summary:
Fixes facebook#18579

Normalize `start` and `end` arguments when `onSelectionChange` is
dispatched on Android.

It just applies a [fix](facebook#18579 (comment)) sent by TheSavior (Thanks, by the way 😄)

## Changelog

[Android] [Fixed] - fix(android): Normalize start and end args
Pull Request resolved: facebook#24938

Differential Revision: D15412005

Pulled By: cpojer

fbshipit-source-id: bb132313cfb8877a682f3865a5f9e48d45ac20ac
@facebook facebook locked as resolved and limited conversation to collaborators May 20, 2020
@react-native-bot react-native-bot added the Resolution: Locked This issue was locked by the bot. label May 20, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Bug Component: TextInput Related to the TextInput component. Good first issue Interested in collaborating? Take a stab at fixing one of these issues. Platform: Android Android applications. Resolution: Locked This issue was locked by the bot.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants