mozilla-mobile / android-components

⚠️ This project moved to a new repository. It is now developed and maintained at: https://github.com/mozilla-mobile/firefox-android
https://github.com/mozilla-mobile/firefox-android
Mozilla Public License 2.0
2.02k stars 473 forks source link

ui-autocomplete: correct InputConnection behavior #13002

Closed rocka closed 2 years ago

rocka commented 2 years ago

This PR fix 2 issues in InlineAutocompleteEditText:

  1. Backspace erases extra character (related issue: https://github.com/mozilla-mobile/fenix/issues/25249)

Modern input methods tend to use InputConnection APIs rather than sending keycode to simulate keypress. Override method InputConnection#deleteSurroundingTextInCodePoints(int,int) available on API level 24+ to unify the behavior.

  1. Composing text state desync between Editor and IME

https://user-images.githubusercontent.com/13914967/197770365-8b3e81a1-9353-4482-9257-7a0ff67e3d8d.mp4

As the video above indicates, input state desyncs between Editor and IME when press Backspace while there is unfinished composing text.

To explain the behavior step by step:

  1. Type redd in address bar, IME presents composing text redd, and Inline AutocompleteEditText completes to redd|it.com
  2. Press Backspace. IME receives backspace and requests to set composing text to red. But AutocompleteEditText ignores the request, finishing composing text redd instead
  3. Press Backspace again. IME receives backspace, and requests to set composing text to re. But composing range was lost because AutocompleteEditText has finished composing already, causing IME set composing text after finished text redd

Looking through setComposingText implementation, it always call removeAutocompleteOnComposing first.

https://github.com/mozilla-mobile/android-components/blob/bd1adfd687aa496f2bd4805816553241126c32c9/components/ui/autocomplete/src/main/java/mozilla/components/ui/autocomplete/InlineAutocompleteEditText.kt#L602-L611

In removeAutocompleteOnComposing, when incoming text shorter than existing composing text, it calls finishingComposingText() and drops incoming text. Althrough the comment says "having finishComposingText() send change notifications to the IME", but there is NO API for IME to know that composing text has been changed, which is the cause of state desync.

https://github.com/mozilla-mobile/android-components/blob/bd1adfd687aa496f2bd4805816553241126c32c9/components/ui/autocomplete/src/main/java/mozilla/components/ui/autocomplete/InlineAutocompleteEditText.kt#L575-L592

This PR changes finishingComposingText() to restartInput(), so IME can know previous InputConnection has been interrupted, allowing IME to reset it's internal state.

After the change, if IME is capable of providing text correction based on surrounding text, it can show candidate words as usual. Otherwise (as the video above), composing range in InlineAutocompleteEditText and IME are both cleared, no desync would occur.

possible related issues:

PS: it would be nice if there are some documentation mentions that you need AppCompat Theme to make InlineAutocompleteEditText work correctly. I spent hours on setting up development environment and test project for this.

Pull Request checklist

After merge

GitHub Automation

Used by GitHub Actions.

csadilek commented 2 years ago

Thank you @rocka for your contribution! This repository was migrated to https://github.com/mozilla-mobile/firefox-android and you'd have to clone the new repo and manually re-apply your changes to continue. Sorry for the inconvenience.