search

keymanapp / keyman

Keyman cross platform input methods system running on Android, iOS, Linux, macOS, Windows and mobile and desktop web
https://keyman.com/
Other
372 stars 102 forks source link

bug(android): Predictive text not working after editing a selected text #11337

Closed bharanidharanj closed 1 week ago

bharanidharanj commented 2 weeks ago

Describe the bug

I noticed that after editing a selected text the predictive text is not at all working.

Reproduce the bug

  1. Install Keyman 18.0.25-alpha.
  2. Open Keyman In-app.
  3. Type a sentence (eg., All things are beautiful)
  4. Select the last word 'beautiful'.
  5. Press Backspace key to delete that word.
  6. Type the text 'ha..' (eg.,happen)

Here, I noticed the predictive text stuck and did not show the correct suggestion. Seems to be an issue.

I have attached the video file for reference.

https://github.com/keymanapp/keyman/assets/19683143/43f338b3-2211-4e00-8aec-de02bcd04d89

Expected behavior

The predictive text should function correctly after editing the text.

Related issues

No response

Keyman apps

Keyman version

18.0.25-alpha

Operating system

Android 12, 13, 14

Device

Samsung Galaxy A23 Mobile device

Target application

No response

Browser

No response

Keyboard name

sil_eurolatin

Keyboard version

3.0.2

Language name

English

Additional context

No response

mcdurdin commented 2 weeks ago

@jahorton can you take a look at this?

sentry-io[bot] commented 2 weeks ago

Sentry Issue: KEYMAN-ANDROID-59E

jahorton commented 2 weeks ago

The Sentry event above:

I can't guarantee that it's a match, but there's a pretty good chance that it is a match.

The event's details are a very close mirror to #11344, though the user-witnessed behaviors aren't the same.

Whenever a JS error occurs within KMW, it has a tendency to prevent anything else that may have needed handling in the same overarching task. That doesn't seem like something that should break predictive-text, yet... evidence points to the underlying cause being a match.

That said, it's worth note that #11344 was posted after this one (#11337), so it's possible that the event isn't actually correlated, even if discovered within approximately the same testing session.

jahorton commented 2 weeks ago

I'm currently failing to get a repro for the error seen above... at least when using the current beta. (Both with and without the fix for #11344.)

Edit: nothing from 18.0.26-alpha, either.

mcdurdin commented 1 week ago

I was able to repro this pretty easily. Symptoms are similar but not identical. Looks like a context desync of some kind.

Keyman for Android 17.0.319-beta.

https://github.com/keymanapp/keyman/assets/4498365/c478ba59-2d1c-4214-8db5-186a47b28d2d

mcdurdin commented 1 week ago

Debugging session yields this:

image

jahorton commented 1 week ago

About the image from the prior screenshot:

With this repro, the call-stack at the point KMW detects a context-desync is highlighted. It's during handling for the first part of a batched JS command from the Android side.

Cross-reference with:

updateKMText(): https://github.com/keymanapp/keyman/blob/0a28678c5324fbe5ea8aec5e129df48f9c8b080c/android/KMEA/app/src/main/assets/android-host.js#L238-L245

As Web has already deleted the text before updateKMText call 1, that call signals "context desync!" and restores the recently-deleted text.

keyman.context.setText() https://github.com/keymanapp/keyman/blob/0a28678c5324fbe5ea8aec5e129df48f9c8b080c/web/src/app/webview/src/contextManager.ts#L98-L108

When Web is handling context desyncs, it auto-jumps to the end of the context, awaiting instruction on where to place the selection range (hence updateKMSelectionRange following call 1 immediately).

From my perspective, this itself is the error. There should be no context-desync triggered during the press of the backspace key. If the first updateKMText weren't being called here, we wouldn't have this issue. But it is, so... why?

jahorton commented 1 week ago

I figured I'd investigate where those calls could be coming from. Searching the whole repository for the string updateSelectionRange, used within Keyman Engine for Android, led me to this:

KMTextView.java, updateTextContext() https://github.com/keymanapp/keyman/blob/0a28678c5324fbe5ea8aec5e129df48f9c8b080c/android/KMEA/app/src/main/java/com/keyman/engine/KMTextView.java#L60-L70

Well, isn't that something? The first three out of four calls in the batched JS from the repro above can all be directly explained by this method's implementation! To be clear, updateSelectionRange returns true unless there's an error accessing context information.

After a bit of extra testing, we found that the "easy repro" that worked in-app... did not with the system keyboard. (KMTextView provides special in-app handling, after all.) We're almost certainly seeing this method be triggered.

jahorton commented 1 week ago

Note that this method has a very limited set of ways to be called:

  1. During keyboard host-page loading, we need to retrieve context when the keyboard is ready.
    • This route should include more calls in the JS batch, though - we would be seeing, at a minimum, a preceding setSpacebarText command.
  2. Whenever KMTextView regains focus.

onWindowFocusChanged() https://github.com/keymanapp/keyman/blob/0a28678c5324fbe5ea8aec5e129df48f9c8b080c/android/KMEA/app/src/main/java/com/keyman/engine/KMTextView.java#L175-L193

setOnFocusChangedListener() https://github.com/keymanapp/keyman/blob/0a28678c5324fbe5ea8aec5e129df48f9c8b080c/android/KMEA/app/src/main/java/com/keyman/engine/KMTextView.java#L117-L131

The in-app keyboard isn't a perfect match for the system keyboard quite yet - it doesn't actually inherit from InputMethodService or anything. (https://developer.android.com/reference/android/widget/TextView#attr_android:inputMethod seems the best lead on how to link TextViews in the future should this change.) We have to maintain the keyboard state in association with a TextView ourselves when in-app.

We're probably getting some sort of 'focus change' event during input... but when and why?

jahorton commented 1 week ago

And... nope, neither of those. It's called by something else. Fortunately, this does reproduce with a reasonably-recent emulation target through Android Studio (even if not on the Android 6.0.1 physical device):

image

Explanation: despite being in the middle of a batch edit, the selection-manipulating method instantly triggers corresponding update operations for the text view. The text view then immediately, in the middle of our text-update operations, signals that the selection has changed - and does so immediately before we actually delete the (originally, but no longer) selected text.