Skip to content

Commit ab3c00d

Browse files
fknivesfacebook-github-bot
authored andcommitted
Fix issue#11068 of duplicating characters when replacing letters to lowercase or uppercase in TextInput (#35929)
Summary: These changes are intended to resolve #11068. ## Changelog: [Android] [Fixed] - Fix letters duplication when using autoCapitalize Pull Request resolved: #35929 Test Plan: I took the `RewriteExample` from `TextInputSharedExamples.js` duplicated it, updated the labels, attached to the same screen. Modified its `onChangeText` function, from `text = text.replace(/ /g, '_');` to `text = text.toLowerCase();` then tested via rn-tester as shown in the video: - No duplicate characters - Characters are updated to be lowercase - Long pressing delete properly deletes, doesn’t stop after deleting one character - Suggestions (selected from keyboard) work and are updated to lowercase when it becomes part of the input text - Moving the cursor and typing works, cursor position is kept as it should - Moving the cursor and deleting works - Selection portion and deleting it works, cursor position is kept as it should https://user-images.githubusercontent.com/14225329/213890296-2f194e21-2cf9-493f-a516-5e0212ed070e.mp4 Note: I have tested manually with 0.67.4, because later versions failed on my machine with cmake and argument errors when building the rn-tester from Android Studio to any device. Help regarding that would be appreciated. ## Possible Future Improvements As it can be seen the video, the letter duplication is resolved, however since the lowercase modification happens on the Javascript side, it takes a couple milliseconds and the Uppercase can still be shown momentarily while typing. ## Technical details, why the solution works I've debugged a simple AppCompatEditText with calling the same `getText().replace` in `doAfterTextChanged` with a bit of delay and noticed a difference to the `ReactEditText`. The ReactEditText removes the `android.view.inputmethod.ComposingText` Span in `manageSpans` before calling replace (ComposingText is `Spanned.SPAN_EXCLUSIVE_EXCLUSIVE`). That `ComposingText` Span is used in `android.view.inputmethod.BaseInputConnection` `private replaceText` to find from what point the text should be replaced from when applying suggestions or typing new letters. Without that Span, it defaults to the selection position, which is usually the end of the text causing duplication of the old "word". **In simple terms, while typing with suggestions on the keyboard, each new letter is handled similarly as clicking a suggestion would be, aka replacing the current "word" with the new "word". (let's say "Ar" word with "Are" word)** Another way to describe it: While typing with suggestions, with the ComposingText Span the TextView keeps track of what word completions are suggested for on the keyboard UI. When receiving a new letter input, it replaces the current "word" with a new "word", and without the Span, it replaces nothing at the end (selection point) with the new word which results in character duplication. It also seems to make sense then why without suggestions (like password-visible and secureTextEntry) the issue hasn't occurred. ### Examples How the issue happened: > - User types: A (ComposingText Span becomes (0,1), composingWord: "A") > - Javascript replaced A with a, ComposingText Span was removed from getText() > - User types a new character: r (ComposingText, defaulting to selection, from selection, empty string is replaced with word "Ar") > => Complete text: aAr => letter duplication. How it works with the changes applied: > - User types: A (ComposingText Span becomes (0,1), composingWord: "A") > - Javascript replaces A with a, (ComposingText Span (0,1) is re-set after replace) > - User types a new character: r (ComposingText (0,1), "a" word is replaced with word "Ar". ComposingText becomes (0,2) "Ar") > - Javascript replaced Ar with ar, (ComposingText Span (0,2) is re-set after replace) > => Complete text: ar => no letter duplication > - User selects suggestion "Are" (ComposingText Span (0,2) "ar" is replaced with new word and space "Are ") > - CompleteText: "Are " > - Javascript replaces "Are " with "are " (ComposingText Span doesn't exist, no string after space " ") Note: the Editable.replace also removes the ComposingText, if it doesn't cover the whole text, that's why we need to re-setSpan them even if we wouldn't remove them in `manageSpans`. ## Note This is my first attempt to contribute so if I missed something or messed up, please point out and I will try to adapt. Reviewed By: NickGerleman Differential Revision: D47243817 Pulled By: lunaleaps fbshipit-source-id: 5f3551d17466f2c7cd1aa89ffb09af50e065b52e
1 parent d839e4a commit ab3c00d

File tree

1 file changed

+48
-7
lines changed
  • packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/textinput

1 file changed

+48
-7
lines changed

packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/textinput/ReactEditText.java

Lines changed: 48 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -632,10 +632,13 @@ public void maybeSetText(ReactTextUpdate reactTextUpdate) {
632632
if (reactTextUpdate.getText().length() == 0) {
633633
setText(null);
634634
} else {
635-
// When we update text, we trigger onChangeText code that will
636-
// try to update state if the wrapper is available. Temporarily disable
637-
// to prevent an infinite loop.
635+
boolean shouldRestoreComposingSpans = length() == spannableStringBuilder.length();
636+
638637
getText().replace(0, length(), spannableStringBuilder);
638+
639+
if (shouldRestoreComposingSpans) {
640+
restoreComposingSpansToTextFrom(spannableStringBuilder);
641+
}
639642
}
640643
mDisableTextDiffing = false;
641644

@@ -650,10 +653,13 @@ public void maybeSetText(ReactTextUpdate reactTextUpdate) {
650653
}
651654

652655
/**
653-
* Remove and/or add {@link Spanned.SPAN_EXCLUSIVE_EXCLUSIVE} spans, since they should only exist
654-
* as long as the text they cover is the same. All other spans will remain the same, since they
655-
* will adapt to the new text, hence why {@link SpannableStringBuilder#replace} never removes
656-
* them.
656+
* Remove and/or add {@link Spanned#SPAN_EXCLUSIVE_EXCLUSIVE} spans, since they should only exist
657+
* as long as the text they cover is the same unless they are {@link Spanned#SPAN_COMPOSING}. All
658+
* other spans will remain the same, since they will adapt to the new text, hence why {@link
659+
* SpannableStringBuilder#replace} never removes them. Keep copy of {@link Spanned#SPAN_COMPOSING}
660+
* Spans in {@param spannableStringBuilder}, because they are important for keyboard suggestions.
661+
* Without keeping these Spans, suggestions default to be put after the current selection
662+
* position, possibly resulting in letter duplication.
657663
*/
658664
private void manageSpans(SpannableStringBuilder spannableStringBuilder) {
659665
Object[] spans = getText().getSpans(0, length(), Object.class);
@@ -662,6 +668,7 @@ private void manageSpans(SpannableStringBuilder spannableStringBuilder) {
662668
int spanFlags = getText().getSpanFlags(span);
663669
boolean isExclusiveExclusive =
664670
(spanFlags & Spanned.SPAN_EXCLUSIVE_EXCLUSIVE) == Spanned.SPAN_EXCLUSIVE_EXCLUSIVE;
671+
boolean isComposing = (spanFlags & Spanned.SPAN_COMPOSING) == Spanned.SPAN_COMPOSING;
665672

666673
// Remove all styling spans we might have previously set
667674
if (span instanceof ReactSpan) {
@@ -676,6 +683,12 @@ private void manageSpans(SpannableStringBuilder spannableStringBuilder) {
676683
final int spanStart = getText().getSpanStart(span);
677684
final int spanEnd = getText().getSpanEnd(span);
678685

686+
// We keep a copy of Composing spans
687+
if (isComposing) {
688+
spannableStringBuilder.setSpan(span, spanStart, spanEnd, spanFlags);
689+
continue;
690+
}
691+
679692
// Make sure the span is removed from existing text, otherwise the spans we set will be
680693
// ignored or it will cover text that has changed.
681694
getText().removeSpan(span);
@@ -803,6 +816,34 @@ private void addSpansFromStyleAttributes(SpannableStringBuilder workingText) {
803816
}
804817
}
805818

819+
/**
820+
* Attaches the {@link Spanned#SPAN_COMPOSING} from {@param spannableStringBuilder} to {@link
821+
* ReactEditText#getText}
822+
*
823+
* <p>See {@link ReactEditText#manageSpans} for more details. Also
824+
* https://github.com/facebook/react-native/issues/11068
825+
*/
826+
private void restoreComposingSpansToTextFrom(SpannableStringBuilder spannableStringBuilder) {
827+
Editable text = getText();
828+
if (text == null) {
829+
return;
830+
}
831+
Object[] spans = spannableStringBuilder.getSpans(0, length(), Object.class);
832+
for (Object span : spans) {
833+
int spanFlags = spannableStringBuilder.getSpanFlags(span);
834+
boolean isComposing = (spanFlags & Spanned.SPAN_COMPOSING) == Spanned.SPAN_COMPOSING;
835+
836+
if (!isComposing) {
837+
continue;
838+
}
839+
840+
final int spanStart = spannableStringBuilder.getSpanStart(span);
841+
final int spanEnd = spannableStringBuilder.getSpanEnd(span);
842+
843+
text.setSpan(span, spanStart, spanEnd, spanFlags);
844+
}
845+
}
846+
806847
private static boolean sameTextForSpan(
807848
final Editable oldText,
808849
final SpannableStringBuilder newText,

0 commit comments

Comments
 (0)