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

Keyboard state is wrong after picking @-mention or stream from autocomplete #5072

Open
gnprice opened this issue Oct 23, 2021 · 1 comment
Open
Labels
a-Android a-compose/send Compose box, autocomplete, camera/upload, outbox, sending upstream: RN Issues related to an issue in React Native

Comments

@gnprice
Copy link
Member

gnprice commented Oct 23, 2021

This issue appears on Android devices using certain keyboards. It does not occur with Gboard, so it does not occur on a typical Pixel device or emulator image.

I just observed this on v27.173 (the current alpha), on a Samsung J5 (model SM-J510FN) running Android 6.0.1. That's the only non-Google Android device I have lying around, but I expect it will repro on current Samsung devices, and likely LG and other vendors too.

Repro steps:

  • Start composing a message. Optionally type some random text.
  • Type an @, to start an @-mention.
  • Type a few more letters. Observe the list of predicted/suggested next words, at the top of the keyboard, update to reflect what you've typed.
    • For example, you might type "mobi", and the list of predictions might read "mobi", "mini", "omni".
  • Select a person or group from the Zulip autocomplete popup above the input area.
  • Expected: An @-mention of the chosen person or group gets inserted, followed by a space. The list of suggestions resets to a list that'd make sense for a new word. Moreover if I then type some other letter, I get that letter.
    • This is indeed what I see when I try this using Gboard, on a Pixel 4 running Android 11.
  • Actual: An @-mention of the chosen person or group gets inserted, followed by a space. The list of suggestions does not change; it behaves as if I was still in the middle of typing the last word. And if I type some other letter, I get all the letters I'd just typed, again, followed by the new letter.
    • In the example above, the list of predictions would still read "mobi", "mini", "omni".
    • Then if I type "h", I get "mobih".
    • Or if I hit the spacebar, the top word in the prediction list gets inserted: "mobi".

The exact same behavior happens if instead of an @, you type a # and choose a stream from the list, too.

  • So for example if I type "#tes", the predictions are "tes", "Test" highlighted, and "yes". If I tap "test here" from the Zulip autocomplete popup, it inserts "#test here ".
    • If I then hit "h", it inserts "tesh", and I end with "#test here tesh".
    • Or if instead I hit space, I end up with "#test here Test".

This is on Samsung keyboard 1.5.46 (as found from the gear icon at the bottom of the keyboard > About Samsung keyboard.)


I discovered this while studying #4239, in #4239 (comment) and the next two comments. In short, React Native has a bug in the implementation of TextInput where exactly this kind of symptom happens when you update the contents of a TextInput. The bug has been worked around for the specific case of clearing the contents, but for any other update it's still live.

The situation is that the Android docs say that when you're updating the contents of a text input, you should call a certain method to let the input method -- that's the keyboard -- know it should refresh its state. Specifically the documentation for InputMethodManager#restartInput reads:

If the input method is currently connected to the given view, restart it with its new contents. You should call this when the text within your view changes outside of the normal input method or key input flow, such as when an application calls TextView.setText().

And React Native's TextInput does not follow that instruction.

There was a PR sent for this back in 2017: facebook/react-native#12462 The RN maintainers in 2019 were amenable (facebook/react-native#12462 (review)), but didn't hear from anyone who would test it out. (At that point in the thread, it was thought that the issue was limited to a few older devices.) So they didn't merge the fix.

Also in 2019, a change was merged in RN (facebook/react-native#18859) which operated in the case where the text was being cleared, i.e. updated to be empty. That change doesn't call restartInput -- it calls setText(null) -- but reportedly that was enough to resolve the issue on some Samsung and LG devices, in the case of clearing the text.

Much later, in 2020-09, a bug was reported in another RN app (status-im/status-mobile#11216) which had similar symptoms: the keyboard's state doesn't get refreshed when you insert something by picking it from the app's own autocomplete UI. (The details are a bit different; but also the app appears to be doing something more interesting with the text, styling the @-mention with a highlight color.) That issue was observed on Android 8 and Android 9. And it was fixed by… calling restartInput.


Probably the best path to fix this is:

  • Reproduce the issue on a recent device, to confirm it's not limited to devices as ancient as the one Samsung device I have around.
  • Then, reproduce it in a tiny fresh React Native app.
  • Modify React Native with a patch similar to Android bugfixes: TextInput issues facebook/react-native#12462.
  • Confirm that that patched React Native fixes the issue, both in the tiny test app and in Zulip.
  • Send that as a PR to React Native.
  • Once such a PR is merged, we'll get the benefit of it in our next RN upgrade.
@gnprice gnprice added a-Android upstream: RN Issues related to an issue in React Native a-compose/send Compose box, autocomplete, camera/upload, outbox, sending labels Oct 23, 2021
@gnprice
Copy link
Member Author

gnprice commented Oct 23, 2021

One other note: It may be possible to reproduce this issue on other Android devices, including the emulator, by installing Samsung Keyboard.

I can't find Samsung Keyboard in either the Play Store nor Samsung's own Galaxy Store. But it does appear here:
https://www.apkmirror.com/apk/samsung-electronics-co-ltd/samsung-keyboard/
including version 4.9.00.8, just a few months old.

gnprice added a commit to gnprice/zulip-mobile that referenced this issue Oct 25, 2021
This library was a workaround for a bug in React Native's TextInput
component: it doesn't tell the keyboard to refresh its state after
the app changes the text contents programmatically.

The workaround only operates in the case where we're clearing the
contents entirely.  When we update it in any other way, such as for
autocomplete, we're still exposed to this RN bug; that's zulip#5072.

And then it turns out that since RN v0.60.0, this workaround has been
superseded by one in RN which also operates only in the case of
completely clearing the contents:
  zulip#4239 (comment)

So we can happily drop it.

I tested this manually on a device which I know is affected by the
underlying RN bug, because it reproduces zulip#5072: a Samsung J5 (model
SM-J510FN) running Android 6.0.1 and Samsung Keyboard 1.5.46.  (From
other reports I believe these issues appear on recent Samsung devices
too; that's just the one I have on hand.)  I sent a message, causing
us to clear the input.  The keyboard state refreshed as it should:
typing a new letter inserted only that letter, not also the last word
typed in the previous message.
gnprice added a commit to gnprice/zulip-mobile that referenced this issue Oct 25, 2021
This library was a workaround for a bug in React Native's TextInput
component: it doesn't tell the keyboard to refresh its state after
the app changes the text contents programmatically.

The workaround only operates in the case where we're clearing the
contents entirely.  When we update it in any other way, such as for
autocomplete, we're still exposed to this RN bug; that's zulip#5072.

And then it turns out that since RN v0.60.0, this workaround has been
superseded by one in RN which also operates only in the case of
completely clearing the contents:
  zulip#4239 (comment)

So we can happily drop it.

I tested this manually on a device which I know is affected by the
underlying RN bug, because it reproduces zulip#5072: a Samsung J5 (model
SM-J510FN) running Android 6.0.1 and Samsung Keyboard 1.5.46.  (From
other reports I believe these issues appear on recent Samsung devices
too; that's just the one I have on hand.)  I sent a message, causing
us to clear the input.  The keyboard state refreshed as it should:
typing a new letter inserted only that letter, not also the last word
typed in the previous message.

Fixes: zulip#4239
gnprice added a commit to gnprice/zulip-mobile that referenced this issue Oct 26, 2021
This library was a workaround for a bug in React Native's TextInput
component: it doesn't tell the keyboard to refresh its state after
the app changes the text contents programmatically.

The workaround only operates in the case where we're clearing the
contents entirely.  When we update it in any other way, such as for
autocomplete, we're still exposed to this RN bug; that's zulip#5072.

And then it turns out that since RN v0.60.0, this workaround has been
superseded by one in RN which also operates only in the case of
completely clearing the contents:
  zulip#4239 (comment)

So we can happily drop it.

I tested this manually on a device which I know is affected by the
underlying RN bug, because it reproduces zulip#5072: a Samsung J5 (model
SM-J510FN) running Android 6.0.1 and Samsung Keyboard 1.5.46.  (From
other reports I believe these issues appear on recent Samsung devices
too; that's just the one I have on hand.)  I sent a message, causing
us to clear the input.  The keyboard state refreshed as it should:
typing a new letter inserted only that letter, not also the last word
typed in the previous message.

Fixes: zulip#4239
chrisbobbe pushed a commit to gnprice/zulip-mobile that referenced this issue Oct 26, 2021
This library was a workaround for a bug in React Native's TextInput
component: it doesn't tell the keyboard to refresh its state after
the app changes the text contents programmatically.

The workaround only operates in the case where we're clearing the
contents entirely.  When we update it in any other way, such as for
autocomplete, we're still exposed to this RN bug; that's zulip#5072.

And then it turns out that since RN v0.60.0, this workaround has been
superseded by one in RN which also operates only in the case of
completely clearing the contents:
  zulip#4239 (comment)

So we can happily drop it.

I tested this manually on a device which I know is affected by the
underlying RN bug, because it reproduces zulip#5072: a Samsung J5 (model
SM-J510FN) running Android 6.0.1 and Samsung Keyboard 1.5.46.  (From
other reports I believe these issues appear on recent Samsung devices
too; that's just the one I have on hand.)  I sent a message, causing
us to clear the input.  The keyboard state refreshed as it should:
typing a new letter inserted only that letter, not also the last word
typed in the previous message.

Fixes: zulip#4239
Somena1 pushed a commit to Somena1/zulip-mobile that referenced this issue Nov 4, 2021
This library was a workaround for a bug in React Native's TextInput
component: it doesn't tell the keyboard to refresh its state after
the app changes the text contents programmatically.

The workaround only operates in the case where we're clearing the
contents entirely.  When we update it in any other way, such as for
autocomplete, we're still exposed to this RN bug; that's zulip#5072.

And then it turns out that since RN v0.60.0, this workaround has been
superseded by one in RN which also operates only in the case of
completely clearing the contents:
  zulip#4239 (comment)

So we can happily drop it.

I tested this manually on a device which I know is affected by the
underlying RN bug, because it reproduces zulip#5072: a Samsung J5 (model
SM-J510FN) running Android 6.0.1 and Samsung Keyboard 1.5.46.  (From
other reports I believe these issues appear on recent Samsung devices
too; that's just the one I have on hand.)  I sent a message, causing
us to clear the input.  The keyboard state refreshed as it should:
typing a new letter inserted only that letter, not also the last word
typed in the previous message.

Fixes: zulip#4239
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 upstream: RN Issues related to an issue in React Native
Projects
None yet
Development

No branches or pull requests

1 participant