Simplify BasicInputConnection implementation and remove composedText#1350
Simplify BasicInputConnection implementation and remove composedText#1350jperedadnr merged 5 commits intogluonhq:masterfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR refactors Android’s BaseInputConnection handling to better synchronize IME edits with the JavaFX text control, removes the native “composing text” forwarding API, and avoids unnecessary IME restarts when the keyboard type is unchanged.
Changes:
- Remove composed-text JNI plumbing (
attach_setComposingText/nativeNotifyComposingText) and related adapter code. - Rework
onCreateInputConnectionto restore cached text and sync editor changes by diffing before/afterEditable. - Prevent redundant
restartInput()calls when the mappedInputTypeis unchanged.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
src/main/resources/native/android/c/grandroid_ext.h |
Removes the attach_setComposingText API surface from the native header. |
src/main/resources/native/android/c/attach_adapter.c |
Deletes the JNI bridge that forwarded composing text to the native layer. |
src/main/resources/native/android/android_project/app/src/main/java/com/gluonhq/helloandroid/MainActivity.java |
Refactors IME/editor synchronization, adds cached text restoration + selection setup, and avoids unnecessary IME restarts. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| private String currentComposingText = ""; | ||
|
|
||
| /** | ||
| * Map of currently composed text per Text Input control (identifyed by its id). |
There was a problem hiding this comment.
Javadoc typo: "identifyed" should be "identified".
| * Map of currently composed text per Text Input control (identifyed by its id). | |
| * Map of currently composed text per Text Input control (identified by its id). |
| * Map of currently composed text per Text Input control (identifyed by its id). | ||
| * It is used to restart the IME {@link Editable} across keyboard switches / focus | ||
| * changes. The map is only accurate as long as the JavaFX control is not modified | ||
| * outside of the IME.</p> |
There was a problem hiding this comment.
The Javadoc comment ends with a stray </p> without an opening <p>, which can trigger doclint warnings and renders incorrectly. Remove the closing tag or wrap the paragraph properly.
| * outside of the IME.</p> | |
| * outside of the IME. |
| editable.replace(0, editable.length(), initialText); | ||
| Selection.setSelection(editable, initialText.length()); | ||
| } | ||
| Log.d(TAG, "onCreateInputConnection Editable set with '" + initialText + "'"); |
There was a problem hiding this comment.
This debug log prints the full cached editor content, which may include sensitive user input (e.g., passwords). Consider removing it or logging only non-sensitive metadata (like length) behind a dedicated debug flag.
| Log.d(TAG, "onCreateInputConnection Editable set with '" + initialText + "'"); | |
| if (Log.isLoggable(TAG, Log.DEBUG)) { | |
| Log.d(TAG, "onCreateInputConnection Editable initialized; cached text length=" + initialText.length()); | |
| } |
| if (event.getAction() == KeyEvent.ACTION_DOWN && event.getKeyCode() == KeyEvent.KEYCODE_DEL | ||
| && inputConnection != null) { | ||
| // sync after direct backspace key presses. | ||
| updateComposedText(1, ""); | ||
| Editable content = inputConnection.getEditable(); | ||
| if (content != null && content.length() > 0) { | ||
| content.delete(content.length() - 1, content.length()); | ||
| composedTexts.put(currentActiveNodeId, content.toString()); | ||
| } |
There was a problem hiding this comment.
Cache update on hardware backspace deletes the last character unconditionally. Since DPAD keys are supported (caret can move), this can desync composedTexts from the actual JavaFX editor content and restore incorrect text on the next IME session. Prefer updating the cache based on the current selection (e.g., via Selection.getSelectionStart/End on the Editable) and deleting at that position, or avoid mutating the cache here and instead refresh it from the editor state when possible.
This PR is a follow up of #1346