Skip to content

Commit

Permalink
Fix issue#11068 of duplicating characters when replacing letters to l…
Browse files Browse the repository at this point in the history
…owercase or uppercase in TextInput (facebook#35929)

Summary:
These changes are intended to resolve facebook#11068.

## Changelog:

[Android] [Fixed] - Fix letters duplication when using autoCapitalize

Pull Request resolved: facebook#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
  • Loading branch information
fknives authored and juniorklawa committed Jul 20, 2023
1 parent 57b17b6 commit 5eee615
Showing 1 changed file with 48 additions and 7 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -632,10 +632,13 @@ public void maybeSetText(ReactTextUpdate reactTextUpdate) {
if (reactTextUpdate.getText().length() == 0) {
setText(null);
} else {
// When we update text, we trigger onChangeText code that will
// try to update state if the wrapper is available. Temporarily disable
// to prevent an infinite loop.
boolean shouldRestoreComposingSpans = length() == spannableStringBuilder.length();

getText().replace(0, length(), spannableStringBuilder);

if (shouldRestoreComposingSpans) {
restoreComposingSpansToTextFrom(spannableStringBuilder);
}
}
mDisableTextDiffing = false;

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

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

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

// We keep a copy of Composing spans
if (isComposing) {
spannableStringBuilder.setSpan(span, spanStart, spanEnd, spanFlags);
continue;
}

// Make sure the span is removed from existing text, otherwise the spans we set will be
// ignored or it will cover text that has changed.
getText().removeSpan(span);
Expand Down Expand Up @@ -803,6 +816,34 @@ private void addSpansFromStyleAttributes(SpannableStringBuilder workingText) {
}
}

/**
* Attaches the {@link Spanned#SPAN_COMPOSING} from {@param spannableStringBuilder} to {@link
* ReactEditText#getText}
*
* <p>See {@link ReactEditText#manageSpans} for more details. Also
* https://github.com/facebook/react-native/issues/11068
*/
private void restoreComposingSpansToTextFrom(SpannableStringBuilder spannableStringBuilder) {
Editable text = getText();
if (text == null) {
return;
}
Object[] spans = spannableStringBuilder.getSpans(0, length(), Object.class);
for (Object span : spans) {
int spanFlags = spannableStringBuilder.getSpanFlags(span);
boolean isComposing = (spanFlags & Spanned.SPAN_COMPOSING) == Spanned.SPAN_COMPOSING;

if (!isComposing) {
continue;
}

final int spanStart = spannableStringBuilder.getSpanStart(span);
final int spanEnd = spannableStringBuilder.getSpanEnd(span);

text.setSpan(span, spanStart, spanEnd, spanFlags);
}
}

private static boolean sameTextForSpan(
final Editable oldText,
final SpannableStringBuilder newText,
Expand Down

0 comments on commit 5eee615

Please sign in to comment.