From 37f9c5ad85246e7b1bca0b77d39d7972a29270b8 Mon Sep 17 00:00:00 2001 From: Vitor Pamplona Date: Tue, 11 Aug 2026 00:40:26 -0400 Subject: [PATCH] fix(embed): restore the keyboard on tab return, and none for readonly MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Device testing on a tablet (SM-T220, Android 14) walked every text-field focus path in the embedded tab. Two of them were wrong. **The tab-return restore never fired.** `noteKeyboardOnLeave` sampled `WindowInsets.imeAnimationTarget > 0 && isMirroringPageField()` inside `onDispose`, on the assumption that the dispose runs before anything hides the IME. It does not: by the time it runs, the nav transition has already snapped the animation target to 0 *and* taken focus off the view, so both halves read false and every tab was recorded as "left without a keyboard". Instrumented on device, the leave was `keyboardUp=false mirroring=false imeBottomPx=0` for all three nav-rail routes, so `pendingRestore` was false on every return and a tab left mid-typing always came back with the keyboard down. Ask the mirror what it *intends* instead of sampling the window at teardown: `RemoteImeView.keyboardWanted` is set when we raise the keyboard and cleared when the field blurs or the user puts the keyboard away, so it still reads true while the view is being torn down. Telling "user dismissed it" apart from "the tab went away" is what that clearing needs, and there is no key hook for it — Android 13+ routes the IME's back dismissal through OnBackInvokedCallback, so `onKeyPreIme` is never called (tried first; it silently never fired and the tab over-restored). The two cases are distinguishable by what else is true when the insets collapse, measured on device: dismiss: imeBottomPx=0 hasFocus=true mirrors=true tab switch: imeBottomPx=0 hasFocus=false mirrors=false so a collapse while we still mirror the field is the dismissal, and a switch never looks like one — the focus loss lands in the same frame as the insets. **A readonly field raised a keyboard that cannot type.** `isEditable` in the shim never looked at `readOnly`, so the host took the field and showed a keyboard whose keystrokes the page discards. Native, checked side by side in the full-screen WebView on the same page, focuses a readonly field without a keyboard. The field stays "editable" for selection (native offers handles and Copy there); only the raise is suppressed, via one guard in `raiseKeyboard` so the fresh-focus, tap-doorbell and tab-restore paths are all covered. Verified on device, 27/27 checks: fresh focus raises for text/textarea/ contenteditable/email/number/password/search/tel and not for disabled or readonly; BACK-dismiss then re-tap restores; re-tapping a field whose keyboard is up keeps it; leaving mid-typing restores on return (~1s, 5/5 runs) while a dismissed tab stays down; typing after either restore lands in the right field at the right caret; page-background tap blurs; address-bar keyboard never arms an embed restore; and the full-screen round trip leaves the embed IME working. `tools/ime-test/keyboard.html` is the page those checks drive: every field type plus a live focus readout and an event log that marks taps on an already-focused field, which is the case with no DOM event of its own. Co-Authored-By: Claude Opus 5 (1M context) --- .../loggedIn/embed/EmbeddedImeBridge.kt | 7 ++ .../screen/loggedIn/embed/EmbeddedTabLayer.kt | 22 ++-- .../ui/screen/loggedIn/embed/RemoteImeView.kt | 39 ++++++ .../composeResources/files/napplet/shim.js | 5 +- tools/ime-test/keyboard.html | 119 ++++++++++++++++++ 5 files changed, 183 insertions(+), 9 deletions(-) create mode 100644 tools/ime-test/keyboard.html diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/embed/EmbeddedImeBridge.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/embed/EmbeddedImeBridge.kt index c49a87b2ab..bb2673f7bb 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/embed/EmbeddedImeBridge.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/embed/EmbeddedImeBridge.kt @@ -96,6 +96,7 @@ private fun parseFocus(o: JSONObject) = inputType = o.optString("inputType", "text"), enterKeyHint = o.optString("enterKeyHint", ""), multiline = o.optBoolean("multiline", false), + readOnly = o.optBoolean("readOnly", false), text = o.optString("text", ""), selStart = o.optInt("selStart", 0), selEnd = o.optInt("selEnd", 0), @@ -109,6 +110,12 @@ sealed interface ImeEvent { val inputType: String, val enterKeyHint: String, val multiline: Boolean, + /** + * The field is `readonly`: focusable and selectable, but not typeable. Native Chrome focuses such a + * field without raising the keyboard, so the host must not either — otherwise the user gets a keyboard + * whose keystrokes the page discards. + */ + val readOnly: Boolean = false, val text: String, val selStart: Int, val selEnd: Int, diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/embed/EmbeddedTabLayer.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/embed/EmbeddedTabLayer.kt index 50bc57161f..d8441ae2ff 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/embed/EmbeddedTabLayer.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/embed/EmbeddedTabLayer.kt @@ -348,10 +348,14 @@ fun EmbeddedTabLayer(barFavoriteIds: List) { // the selection). All the show/hide state lives in one [SelectionUiState] so the rules — toolbar hides // while dragging or scrolling, handles hide while scrolling — are expressed in one place. val sel = remember { SelectionUiState() } - // Read at dispose time to record whether the keyboard was up as the user left this tab (see - // [EmbeddedTabHost.noteKeyboardOnLeave]). The dispose runs before anything hides the IME — our own - // onPageBlur below is what takes it down — so this still reads the pre-switch state. - val keyboardUp = rememberUpdatedState(imeBottomPx > 0) + // The keyboard collapsing while this view still mirrors the page field means the user put it away on a + // field they are still on (BACK, or the IME's own hide button) — record it so returning to this tab + // doesn't pop the keyboard back over the page. A tab switch also collapses the insets but does NOT + // look like this: measured on device, the switch takes focus off the view in the same frame, so + // isMirroringPageField() is already false there and the mark this tab was owed survives. + LaunchedEffect(activeId, imeBottomPx) { + if (imeBottomPx == 0 && imeView.isMirroringPageField()) imeView.noteKeyboardDismissed() + } DisposableEffect(imeBridge) { val boundId = activeId // A warm tab keeps its page focus while parked, so returning to it fires no focus event: ask the @@ -423,10 +427,14 @@ fun EmbeddedTabLayer(barFavoriteIds: List) { // way: blurring it here would fire the page's own blur handlers — validation, autocomplete // dismissal, submit-on-blur — for a switch the user never made inside the page. // - // Only a keyboard THIS mirror holds counts: the tab's own chrome has host-side fields (the - // browser's address bar), and typing in one of those must not arm a restore for a page field. + // Ask the mirror what it *intends* rather than sampling the window: by the time this dispose + // runs, the nav transition has already snapped `WindowInsets.imeAnimationTarget` to 0 and + // taken focus off the view, so both would report "no keyboard" for every tab the user left + // mid-typing — which is exactly the case this restore exists for. [wantsKeyboardForPageField] + // also answers the other half: only a keyboard THIS mirror holds counts, so typing in the + // browser's own address bar never arms a restore for a page field. if (boundId != null) { - EmbeddedTabHost.noteKeyboardOnLeave(boundId, keyboardUp.value && imeView.isMirroringPageField()) + EmbeddedTabHost.noteKeyboardOnLeave(boundId, imeView.wantsKeyboardForPageField()) } imeView.onPageBlur() imeView.bind(null) diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/embed/RemoteImeView.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/embed/RemoteImeView.kt index bdb55e1aa0..27be249c47 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/embed/RemoteImeView.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/embed/RemoteImeView.kt @@ -72,6 +72,17 @@ class RemoteImeView( // ship the PREVIOUS tab's text to the page on the first keystroke. private var mirroring = false + // Whether the keyboard is *meant* to be up for the field we mirror — our own intent, not the window's + // current state. Deliberately not derived from the IME insets or [hasFocus]: by the time a tab switch + // tears this view down, `WindowInsets.imeAnimationTarget` has already snapped to 0 and the view has + // already lost focus, so anything sampled then reports "no keyboard" for a tab the user left mid-typing. + // Set when we raise the keyboard, cleared when the user dismisses it or the page field blurs. + private var keyboardWanted = false + + // The mirrored field is `readonly`. Kept here rather than checked at each call site so every raise path — + // a fresh focus, the tap doorbell, and a tab restore — is covered by the one guard in [raiseKeyboard]. + private var fieldReadOnly = false + private val imm get() = context.getSystemService(Context.INPUT_METHOD_SERVICE) as InputMethodManager private val flush = Runnable { flushState() } @@ -170,6 +181,7 @@ class RemoteImeView( ) { configureFor(focus) mirroring = true + fieldReadOnly = focus.readOnly // Focus the EditText BEFORE seeding text/selection. An EditText jumps its caret to the end when it // gains focus; if we seed first, that end-position then overrides the seed and gets shipped to the // page — so a tap mid-text lands the caret at the end of the field. Seeding AFTER focus makes the @@ -205,6 +217,13 @@ class RemoteImeView( /** True while the keyboard this view holds belongs to a page field — see [mirroring]. */ fun isMirroringPageField() = mirroring && hasFocus() + /** + * Whether this tab should come back with its keyboard up: we mirror a page field and the keyboard was + * meant to be showing when we were asked. Safe to call while the view is being torn down, which is the + * whole point — see [keyboardWanted]. + */ + fun wantsKeyboardForPageField() = mirroring && keyboardWanted + /** * Put the keyboard back on the field this view already mirrors — the user tapped it after dismissing the * keyboard, which leaves the page's focus (and this mirror) untouched, so there is nothing to re-seed. @@ -213,11 +232,29 @@ class RemoteImeView( */ @Suppress("DEPRECATION") // InputMethodManager.SHOW_IMPLICIT is deprecated; no equivalent flag on the newer API. fun raiseKeyboard() { + // A readonly field takes focus and can be selected/copied, but nothing can be typed into it — native + // Chrome shows no keyboard for one, so neither do we. + if (fieldReadOnly) return + keyboardWanted = true post { if (hasFocus()) imm.showSoftInput(this, InputMethodManager.SHOW_IMPLICIT) } } + /** + * The user put the keyboard away (BACK, or the IME's own hide affordance) while still on this field: a + * deliberate "I'm done typing", so returning to this tab must NOT pop the keyboard back up. + * + * Called by the layer when the IME insets collapse while this view still mirrors the page field. That + * condition is what separates a dismissal from a tab switch — on a switch the view has already lost focus + * by the time the insets collapse, so [isMirroringPageField] is false and this never fires. (Note there is + * no usable key hook for this: Android 13+ routes the IME's back-dismiss through OnBackInvokedCallback, so + * `onKeyPreIme` is never called.) + */ + fun noteKeyboardDismissed() { + keyboardWanted = false + } + // When the current selection first became a range, and how many of its collapse-abandonments we've // re-asserted. Chrome abandons a selection *immediately* (~60ms); a deliberate user tap-to-collapse comes // later — so we only re-assert within a short window of the range forming, bounded for safety. @@ -256,6 +293,8 @@ class RemoteImeView( /** The page field blurred: drop the keyboard. */ fun onPageBlur() { mirroring = false + keyboardWanted = false + fieldReadOnly = false removeCallbacks(reportRangeLost) if (hadRange) { hadRange = false diff --git a/commons/src/commonMain/composeResources/files/napplet/shim.js b/commons/src/commonMain/composeResources/files/napplet/shim.js index 6afbca5e04..4ac67e1cd1 100644 --- a/commons/src/commonMain/composeResources/files/napplet/shim.js +++ b/commons/src/commonMain/composeResources/files/napplet/shim.js @@ -376,7 +376,8 @@ var inputType = isCE(n) ? 'text' : (t === 'TEXTAREA' ? 'textarea' : (n.type || 'text').toLowerCase()); var sel = selOf(n); return { type:'ime.focus', inputType: inputType, enterKeyHint: (n.enterKeyHint || ''), - multiline: multiline, text: valOf(n), selStart: sel[0], selEnd: sel[1], geom: fieldGeom(n) }; + multiline: multiline, readOnly: !!n.readOnly, text: valOf(n), selStart: sel[0], + selEnd: sel[1], geom: fieldGeom(n) }; } // Like `ime.focus`, but for a field that is ALREADY focused: the answer to the host's `ime.resync`, which // it asks for when it needs to (re-)take a field whose focus never moved in the page. @@ -390,7 +391,7 @@ return { type:'ime.refocus', inputType: isCE(n) ? 'text' : (t === 'TEXTAREA' ? 'textarea' : (n.type || 'text').toLowerCase()), enterKeyHint: (n.enterKeyHint || ''), multiline: isCE(n) || t === 'TEXTAREA', - text: valOf(n), selStart: sel[0], selEnd: sel[1] }; + readOnly: !!n.readOnly, text: valOf(n), selStart: sel[0], selEnd: sel[1] }; } // Last selection we either applied (applyState) or already reported, so the asynchronous // selectionchange our own setSel triggers doesn't echo back to the host as a fresh edit. diff --git a/tools/ime-test/keyboard.html b/tools/ime-test/keyboard.html new file mode 100644 index 0000000000..a976e0626d --- /dev/null +++ b/tools/ime-test/keyboard.html @@ -0,0 +1,119 @@ + + +Keyboard focus matrix + + +
FOCUS: (none)
+ +
+
+
+
+
hello world
+
+
+
+
+
+
+
+
+ + + + +
+ +
+ +