Withdraw textPropertyForNode API, add removeKeyboardTypeForNode API by jperedadnr · Pull Request #442 · gluonhq/attach

jperedadnr · 2026-04-17T22:26:49Z

This PR is a follow-up of #439

It withdraws the textPropertyForNode API: there is no point in having a textProperty outside the TextField (which can be easily out of sync if the textField is updated programatically or via JavaFX paste. Even if synced, it can't be used on the same textField, that is used for writing (so it would only make sense if this textField was wrapped with another control that showed only the composed text. To prevent changes from predictive text, the better option for now is some kind of batching mechanism that takes the text only after some time (preventing the internal changes that happen due to preventive text processing on Android).
It adds removeKeyboardTypeForNode API, so nodes that were registered can be properly removed, if needed.
It reworks the call to set the keyboard type, to prevent race conditions when the keyboard was hidden.

Copilot

Pull request overview

This PR removes the previously added “composed text tracking” API (textPropertyForNode) and its native plumbing, and reworks keyboard-type handling to be focus-driven while adding APIs to unregister keyboard-type and visibility tracking.

Changes:

Withdraw textPropertyForNode and remove Android composing-text callback/config/native stubs.
Add removeKeyboardTypeForNode(Node) and releaseVisibilityForNode(Node) APIs.
Switch keyboard type application from per-node mouse event filtering to scene focus-owner tracking to reduce race conditions.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
modules/keyboard/src/main/resources/META-INF/substrate/config/jniconfig-aarch64-android.json	Removes JNI config entry for composing-text callback.
modules/keyboard/src/main/native/android/c/keyboard.c	Removes composing-text JNI method lookup and native-to-Java forwarding function.
modules/keyboard/src/main/java/com/gluonhq/attach/keyboard/impl/IOSKeyboardService.java	Drops overrides tied to removed APIs; relies on Base implementation.
modules/keyboard/src/main/java/com/gluonhq/attach/keyboard/impl/BaseKeyboardService.java	Adds unregister APIs, moves visibility tracking into Base, and adds scene focus tracking to drive keyboard type.
modules/keyboard/src/main/java/com/gluonhq/attach/keyboard/impl/AndroidKeyboardService.java	Removes composing-text callback and redundant visibility methods.
modules/keyboard/src/main/java/com/gluonhq/attach/keyboard/KeyboardService.java	Removes `textPropertyForNode`, adds `releaseVisibilityForNode` and `removeKeyboardTypeForNode`.
gradle/include/android/grandroid_ext.h	Removes composing-text hook declaration/stub.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-17T22:31:40Z

    BaseKeyboardService() {
-        VISIBLE_HEIGHT.addListener((obs, ov, nv) -> {
-            if (nv != null && nv.doubleValue() <= 0) {
-                if (debug) {
-                    LOG.info("Keyboard hidden, reset default type");
-                }
-                applyActiveNodeId(""); // reset active node
-                applyKeyboardType(KeyboardType.ASCII.getValue());
-            }
-        });
+    }


The previous implementation reset the active node id and keyboard type back to ASCII when VISIBLE_HEIGHT dropped to 0 (keyboard hidden). With the constructor now empty, that reset no longer happens, which changes behavior and may conflict with the setKeyboardTypeForNode contract/Javadoc. If the reset is still intended, reintroduce it (possibly with the updated focus-based approach); otherwise consider updating the public Javadoc to match the new behavior.

Copilot · 2026-04-17T22:31:41Z

+     * Removes the keyboard type assignment and event filter previously installed via
+     * {@link #setKeyboardTypeForNode(Node, KeyboardType)}. After this call the node
+     * will simply use the default keyboard type.


The Javadoc says this removes an “event filter”, but the implementation now uses a scene-level focusOwnerProperty() listener rather than an event filter on the node. Please update the documentation to describe what is actually installed/removed (and what the lifecycle expectations are).

Suggested change

* Removes the keyboard type assignment and event filter previously installed via

* {@link #setKeyboardTypeForNode(Node, KeyboardType)}. After this call the node

* will simply use the default keyboard type.

* Removes the keyboard type assignment previously registered via

* {@link #setKeyboardTypeForNode(Node, KeyboardType)} and stops tracking scene

* focus-owner changes for that node. The registration remains active until this

* method is called. After this call the node will simply use the default keyboard

* type.

Copilot · 2026-04-17T22:31:41Z

    ATTACH_LOG_FINE("Initializing native Keyboard from OnLoad");
    jAttachKeyboardClass = (*env)->NewGlobalRef(env, (*env)->FindClass(env, "com/gluonhq/attach/keyboard/impl/AndroidKeyboardService"));
    jAttach_notifyHeightMethod = (*env)->GetStaticMethodID(env, jAttachKeyboardClass, "notifyVisibleHeight", "(F)V");
-    jAttach_notifyComposingTextMethod = (*env)->GetStaticMethodID(env, jAttachKeyboardClass, "notifyComposingText", "(Ljava/lang/String;Ljava/lang/String;)V");
    initKeyboard();


notifyComposingText support was removed, but jAttach_notifyComposingTextMethod is still declared at the top of the file. With no initialization or use, this can trigger unused-variable warnings in stricter builds. Please remove the leftover jAttach_notifyComposingTextMethod declaration (and any related includes/defs if present).

Copilot · 2026-04-17T22:31:41Z

+    private void attachFocusTracker(Node node) {
+        Scene scene = node.getScene();
+        if (scene != null) {
+            trackScene(scene);
+            // If this node is already the focus owner, apply its type now
+            if (scene.getFocusOwner() == node) {
+                applyTypeFor(node);
+            }
+            return;


When tracking a Scene for the first time, the current focus owner’s keyboard type/id isn’t applied unless it happens to be the node being registered. Since focusOwnerProperty() listeners don’t fire on registration, the native layer can remain out of sync until the next focus change. Consider applying applyTypeFor(scene.getFocusOwner()) right after installing the listener (and similarly after trackScene(nv) in the deferred branch).

Copilot · 2026-04-17T22:31:42Z

-            idToNode.put(syntheticId(n), n);
-            return new ReadOnlyStringWrapper("");
-        }).getReadOnlyProperty();
+        nodeKeyboardTypes.remove(node);


removeKeyboardTypeForNode only removes the stored mapping. If the node is currently the focus owner, the native keyboard type won’t revert to the default until focus changes again. Consider detecting this case (e.g., node.getScene()!=null && node.getScene().getFocusOwner()==node) and calling the same apply logic so the default ASCII type/id is pushed immediately.

Suggested change

nodeKeyboardTypes.remove(node);

nodeKeyboardTypes.remove(node);

Scene scene = node.getScene();

if (scene != null && scene.getFocusOwner() == node) {

applyTypeFor(node);

}

Copilot · 2026-04-17T22:31:42Z

+    /** Map of nodes to their visibility listeners. */
+    private final Map<Node, ChangeListener<Number>> visibilityListeners = new WeakHashMap<>();



visibilityListeners is a WeakHashMap, but each stored listener lambda closes over the node key, which defeats the weak-key behavior and can keep nodes alive until releaseVisibilityForNode is called. Consider using a regular HashMap (and documenting the need to release), or refactor the listener to hold only weak references to the node/parent if you want automatic cleanup.

jperedadnr added 5 commits April 17, 2026 17:55

Add API to unregister nodes from keyboard visibility changes

6ec1475

withdraw textPropertyForNode API

ce8078a

Add removeKeyboardTypeForNode API

fb6d637

Rework keyboardtype deactivation

c5440bb

Merge branch 'master' into 165-2-keyboardapi

5f9e007

Copilot AI review requested due to automatic review settings April 17, 2026 22:26

Copilot started reviewing on behalf of jperedadnr April 17, 2026 22:27 View session

Copilot AI reviewed Apr 17, 2026

View reviewed changes

cleanup

e7e3f2a

jperedadnr merged commit e98b0c0 into gluonhq:master Apr 17, 2026
1 check passed

jperedadnr deleted the 165-2-keyboardapi branch April 17, 2026 22:47

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Withdraw textPropertyForNode API, add removeKeyboardTypeForNode API#442

Withdraw textPropertyForNode API, add removeKeyboardTypeForNode API#442
jperedadnr merged 6 commits intogluonhq:masterfrom
jperedadnr:165-2-keyboardapi

jperedadnr commented Apr 17, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Apr 17, 2026

Uh oh!

Copilot AI Apr 17, 2026

Uh oh!

Copilot AI Apr 17, 2026

Uh oh!

Copilot AI Apr 17, 2026

Uh oh!

Copilot AI Apr 17, 2026

Uh oh!

Copilot AI Apr 17, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

-     * Removes the keyboard type assignment and event filter previously installed via
-     * {@link #setKeyboardTypeForNode(Node, KeyboardType)}. After this call the node
-     * will simply use the default keyboard type.
+     * Removes the keyboard type assignment previously registered via
+     * {@link #setKeyboardTypeForNode(Node, KeyboardType)} and stops tracking scene
+     * focus-owner changes for that node. The registration remains active until this
+     * method is called. After this call the node will simply use the default keyboard
+     * type.

		/** Map of nodes to their visibility listeners. */
		private final Map<Node, ChangeListener<Number>> visibilityListeners = new WeakHashMap<>();

Conversation

jperedadnr commented Apr 17, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 17, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 17, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 17, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 17, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 17, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 17, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants