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

The library conflicts with the react-native-text-input-mask since version 1.10.0. #324

Closed
devoren opened this issue Jan 11, 2024 · 6 comments · Fixed by #326
Closed

The library conflicts with the react-native-text-input-mask since version 1.10.0. #324

devoren opened this issue Jan 11, 2024 · 6 comments · Fixed by #326
Assignees
Labels
🤖 android Android specific 🐛 bug Something isn't working focused input 📝 Anything about focused input functionality

Comments

@devoren
Copy link

devoren commented Jan 11, 2024

Describe the bug
Hello! I know that this problem is not serious and in many cases does not occur. Sorry for the increase in issues, but I hope someone can help 🙏 . The library conflicts with the react-native-text-input-mask since version 1.10.0, so the keyboard does not open the first time. I think this is related to the FocusedInputTextChangedEvent. The latest version that works without errors is 1.9.5 without FocusedInputTextChangedEvent.

Code snippet

<TextInputMask
  mask="+1 ([000]) [000] [00] [00]"
  onChangeText={(formatted, extracted) => {
    console.log(formatted); // +1 (123) 456-78-90
    setPhone(extracted ?? ''); // 1234567890
  }}
  keyboardType="phone-pad"
  placeholder="+1 (___) ___ __ __"
  style={{
    width: '100%',
    backgroundColor: '#e2e8f0',
    borderRadius: 4,
    paddingHorizontal: 12,
  }}
/>

Repo for reproducing
You can change the RNKC version and see the difference.
https://github.com/devoren/TestRNKC732

To Reproduce
Steps to reproduce the behavior:

  1. Install react-native-keyboard-controller and react-native-text-input-mask
  2. Add Mask input and add mask prop
  3. Press input and see that the keyboard does not open the first time
  4. See logcat from android studio

Expected behavior
Keyboard opens without any problem in first press

Screenshots
image

In the video i press input 2 times

mask.mp4

Smartphone (please complete the following information):

  • Desktop OS: [Windows 10]
  • Device: [Pixel 4 Emulator]
  • OS: [Android 12]
  • RN version: [0.73.2]
  • RN architecture: [old]
  • JS engine: [Hermes]
  • Library version: [1.10.1]

Additional context
The problem only occurs if I add a mask prop to TextInputMask.

@kirillzyusko
Copy link
Owner

Nice catch @devoren

I'll try to have a look today on it 👀

@kirillzyusko kirillzyusko added 🐛 bug Something isn't working 🤖 android Android specific labels Jan 11, 2024
@kirillzyusko
Copy link
Owner

Just a quick update - the issue is reproducible on my end in 100% cases.

Basically this exception happens when you iterate over and collection gets modified, so this code will trigger this exception:

List<String> myList = new ArrayList<>();
Iterator<String> iterator = myList.iterator();
while (iterator.hasNext()) {
    String element = iterator.next();
    myList.add("newElement"); // This will throw ConcurrentModificationException
}

There is some issues because in different calls I see different amount of listeners.

3 listeners 2 listeners
image image

I've tried naive things like adding listener after 100ms or 1s, but it will produce crash when you type a symbol 🤷‍♂️

I'll search for new solutions 👀

@kirillzyusko
Copy link
Owner

I think this is the most interesting issue I've ever discovered in my life 👀

So basically crash happens in this code:

public void afterTextChanged(Editable s) {
    if (!ReactEditText.this.mIsSettingTextFromJS && ReactEditText.this.mListeners != null) {
        Iterator var2 = ReactEditText.this.mListeners.iterator();

        while(var2.hasNext()) {
            TextWatcher listener = (TextWatcher)var2.next();
            listener.afterTextChanged(s);
        }
    }

}

On every afterTextChanged input-mask-android removes the listener and adds it back.

Now let's consider oversimplified example (you can run this code in https://www.jdoodle.com/online-java-compiler):

import java.util.ArrayList;
import java.util.Iterator;

public class MyClass {
    public static void main(String args[]) {
        ArrayList<Integer> mListeners = new ArrayList<>();
        mListeners.add(0);
        mListeners.add(1);
        // mListeners.add(2);

        Iterator<Integer> iterator = mListeners.iterator();

        while (iterator.hasNext()) {
            Integer listener = iterator.next();

            // Check if the listener is equal to 1
            // 1 is OnlyChangeIfRequiredMaskedTextChangedListener and we simulate the behavior of this class
            if (listener == 1) {
                int i = mListeners.indexOf(listener);

                if (i >= 0) {
                    mListeners.remove(i);
                }

                // Add the removed element at the end
                mListeners.add(listener);
            }
        }

        // Print the modified list
        System.out.println(mListeners);
    }
}

This code will be executed without any issues, and will produce expected [0, 1] output.

But if we uncomment mListeners.add(2); - we'll get our exception:

Exception in thread "main" java.util.ConcurrentModificationException
	at java.base/java.util.ArrayList$Itr.checkForComodification(ArrayList.java:1095)
	at java.base/java.util.ArrayList$Itr.next(ArrayList.java:1049)
	at MyClass.main(MyClass.java:14)

These 3 array elements are real elements in RN app:

  • 0 is EmojiTextWatcher (seems like it's added by OS);
  • 1 is OnlyChangeIfRequiredMaskedTextChangedListener (added by react-native-text-input-mask);
  • 2 is a listener that I'm attaching in RNKC.

So here we get a situation, where react-native code works for 2 listeners, but doesn't work for 3. To make this code working again with 3 listeners we just can change the order - if the order is [2, 0, 1] or [0, 2, 1] it will work, because removal/insertion operation will be the last one and java will not throw an exception.

Or before iterating we can create a copy of array and iterate over this copy (but we'll have to modify react-native source code). For example Android code iterates over mListeners as:

if (mListeners != null) {
    final ArrayList<TextWatcher> list = mListeners;
    final int count = list.size();
    for (int i = 0; i < count; i++) {
        list.get(i).afterTextChanged(text);
    }
}

Oversimplified version with new iteration approach that works may look like this:

import java.util.ArrayList;
import java.util.Iterator;

public class MyClass {
    public static void main(String args[]) {
        ArrayList<Integer> mListeners = new ArrayList<>();
        mListeners.add(0);
        mListeners.add(1);
        mListeners.add(2);
        
        final ArrayList<Integer> list = mListeners;
        final int count = list.size();
        for (int i = 0; i < count; i++) {
            Integer listener = list.get(i);
            if (listener == 1) {
                mListeners.remove(i);


                // Add the removed element at the end
                mListeners.add(listener);
            }
        }

        // Print the modified list
        System.out.println(mListeners);
    }
}

So now we have two options on how to fix this issue:

  • insert RNKC listener before MaskedTextChangedListener;
  • change how react-native iterates over array.

Both solutions are not good enough because:

  • we don't have an access to private variable of ReactEditText and can not insert to the beginning (actually we can use reflection and I already experimented with it - it fixes the problem, but everyone knows that reflection should be used as a last resort);
  • that means that only new version of RN (let's say 0.74) will support RNKC 1.10. And such compatibility range is not good enough for general purpose library.

I will think about other solutions too, but if you @devoren have any ideas - please, share them 😊

@devoren
Copy link
Author

devoren commented Jan 11, 2024

To be honest, I'm not good at the native side. But I understand the problem, so is it because of react-native limitations? Then I don't want to take up too much of your time😅. For now i will use version 1.9.5 without any problems

@kirillzyusko
Copy link
Owner

I think I will go with reflection approach (it should work pretty reliably) and most likely will submit a PR to react-native repo :)

I think it'll be fixed in 1.10.2 🤞

kirillzyusko added a commit that referenced this issue Jan 12, 2024
## 📜 Description

Fixed `ConcurrentModificationException` when user touches TextInput from
`react-native-text-input-mask`.

## 💡 Motivation and Context

I described problem in
#324 (comment)

Here I'd like to focus on a solution. The most obvious one is to match
old behavior and attach `TextWatcher` from
`react-native-text-input-mask` as a last one. Unfortunately we don't
have an API to control a position of watcher, so I had to use
reflection.

I think I wrote reflection quite carefully. I'm checking, that
`mListeners` field is present, I'm checking that it's `ArrayList`, I'm
verifying that all elements belongs to `TextWatcher` and only after that
I'm adding RNKC listener as first element in an array.

Closes
#324

## 📢 Changelog

### JS

- added example with `react-native-text-input-mask`;

### Android

- use reflection to set RNKC `TextWatcher` into first position of array;

## 🤔 How Has This Been Tested?

Tested manually on Pixel 3a (emulator, Android 13).

## 📸 Screenshots (if appropriate):

<img width="245" alt="Screenshot 2024-01-12 at 22 37 41"
src="https://github.com/kirillzyusko/react-native-keyboard-controller/assets/22820318/5783a44b-82f6-428a-82a6-99a4c4f8816e">

## 📝 Checklist

- [x] CI successfully passed
- [x] I added new mocks and corresponding unit-tests if library API was
changed
@kirillzyusko kirillzyusko added the focused input 📝 Anything about focused input functionality label Jan 12, 2024
@4yki
Copy link

4yki commented Jan 15, 2024

Just've tried new version. The issue was fixed for me. Huge thx for your work @kirillzyusko

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🤖 android Android specific 🐛 bug Something isn't working focused input 📝 Anything about focused input functionality
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants