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

fix(web): better predictive-text handling of text wordbreaking transitions #12864

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jahorton
Copy link
Contributor

@jahorton jahorton commented Jan 9, 2025

Fixes: #12494.
Replaces #12860.

The underlying issue was occurring due to a previously unforeseen consequence of default wordbreaker use, combined with some of our custom handling.

Good case: suppose the text just typed reads isn'. A natural suggestion for English is the word isn't. While end-of-token ' is cause for wordbreaking, since the text insertion point is adjacent to the ', we disable wordbreaking for that ' to facilitate the isn't suggestion.

Bad case: suppose we just typed text' and then add another character that should trigger wordbreaking - like a " - gets typed. At this point, due to our handling for the "good case", we transition from a tokenized context of [text'] to [text, ', "]. The "old context" no longer matches - [text'] vs [text, '] - but the method responsible for detecting this previously did not do so, which is what led to the original error.

Additionally, I added a unit test for this case to the testing suite. It checks the circumstance that leads to the error, rather than the original error itself, so user testing is still wise here.

User Testing

TEST_NOCRASH: follow the steps outlined in #12494 and verify that the crash no longer happens.

@jahorton jahorton requested a review from mcdurdin as a code owner January 9, 2025 02:57
@keymanapp-test-bot keymanapp-test-bot bot added has-user-test user-test-required User tests have not been completed labels Jan 9, 2025
@keymanapp-test-bot keymanapp-test-bot bot added this to the A18S19 milestone Jan 9, 2025
@dinakaranr
Copy link

Test Results

I tested this issue with the attached "18.0.164-alpha-test-12864" build on the Andoird 14. Here is my observation.

  • TEST_NOCRASH (Passed):
  1. Open the keyman app in the Android mobile app.
  2. Added EuroLatin keyboard.
  3. Added a sentence.
  4. Added letters to these orders.
  5. Longpress g --> g̃,
  6. Longpress h --> ḥ,
  7. Longpress j --> ĵ,
  8. Flick down on c --> ',
  9. Flick down on v --> ",
  10. Verified that no error appears on the toast message.
    It works well in good and bad cases. It works well. Thank you.

@keymanapp-test-bot keymanapp-test-bot bot removed the user-test-required User tests have not been completed label Jan 9, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

bug(web): undefined reading 'deleteLeft' causing crash in Android app
2 participants