mirror of
https://github.com/vitorpamplona/amethyst.git
synced 2026-10-05 11:18:24 +00:00
fix(embed): restore the keyboard on tab return, and none for readonly
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) <noreply@anthropic.com>
This commit is contained in:
Vitor Pamplona
co-authored by
Claude Opus 5
Claude Opus 5
parent
b7f55d8697
commit
37f9c5ad85
+7
@@ -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,
|
||||
|
||||
+15
-7
@@ -348,10 +348,14 @@ fun EmbeddedTabLayer(barFavoriteIds: List<String>) {
|
||||
// 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<String>) {
|
||||
// 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)
|
||||
|
||||
+39
@@ -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
|
||||
|
||||
@@ -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.
|
||||
|
||||
@@ -0,0 +1,119 @@
|
||||
<!doctype html>
|
||||
<meta name="viewport" content="width=device-width,initial-scale=1">
|
||||
<title>Keyboard focus matrix</title>
|
||||
<style>
|
||||
body { font: 13px/1.35 system-ui, sans-serif; margin: 6px; background: #111; color: #eee; }
|
||||
#focus { font: 700 15px/1.4 ui-monospace, monospace; background: #04203a; color: #7fd4ff;
|
||||
padding: 6px 8px; border-radius: 5px; position: sticky; top: 0; z-index: 5; }
|
||||
.row { display: flex; align-items: center; gap: 6px; margin: 5px 0; }
|
||||
.row label { width: 74px; font: 700 11px ui-monospace, monospace; color: #9ab; flex: none; }
|
||||
input, textarea, [contenteditable] { flex: 1; font-size: 15px; padding: 7px;
|
||||
border-radius: 5px; border: 1px solid #456; background: #1b1f24; color: #eee; }
|
||||
input:focus, textarea:focus, [contenteditable]:focus { border-color: #7fd4ff; outline: none; }
|
||||
[contenteditable] { min-height: 20px; }
|
||||
#log { font: 11px/1.3 ui-monospace, monospace; color: #7dff9b; background: #000;
|
||||
padding: 5px; height: 190px; overflow: auto; border-radius: 5px; margin-top: 6px;
|
||||
white-space: pre-wrap; }
|
||||
button { font-size: 12px; padding: 5px 9px; border-radius: 5px; border: 1px solid #567;
|
||||
background: #223; color: #eee; }
|
||||
</style>
|
||||
|
||||
<div id="focus">FOCUS: (none)</div>
|
||||
|
||||
<div class="row"><label>empty</label><input id="empty" type="text" placeholder="empty text"></div>
|
||||
<div class="row"><label>filled</label><input id="filled" type="text" value="hello world"></div>
|
||||
<div class="row"><label>ta-empty</label><textarea id="taEmpty" rows="1" placeholder="empty textarea"></textarea></div>
|
||||
<div class="row"><label>ta-filled</label><textarea id="taFilled" rows="1">hello world</textarea></div>
|
||||
<div class="row"><label>editable</label><div id="ce" contenteditable>hello world</div></div>
|
||||
<div class="row"><label>email</label><input id="email" type="email" placeholder="a@b.c"></div>
|
||||
<div class="row"><label>number</label><input id="number" type="number" placeholder="123"></div>
|
||||
<div class="row"><label>password</label><input id="password" type="password" placeholder="secret"></div>
|
||||
<div class="row"><label>search</label><input id="search" type="search" placeholder="search"></div>
|
||||
<div class="row"><label>tel</label><input id="tel" type="tel" placeholder="+1"></div>
|
||||
<div class="row"><label>readonly</label><input id="readonly" type="text" value="readonly" readonly></div>
|
||||
<div class="row"><label>disabled</label><input id="disabled" type="text" value="disabled" disabled></div>
|
||||
<div class="row">
|
||||
<label>actions</label>
|
||||
<button id="btnBlur">blur()</button>
|
||||
<button id="btnFocusFilled">focus(filled)</button>
|
||||
<button id="btnClear">clear log</button>
|
||||
</div>
|
||||
|
||||
<div id="log"></div>
|
||||
|
||||
<script>
|
||||
const logEl = document.getElementById('log');
|
||||
const focusEl = document.getElementById('focus');
|
||||
const t0 = performance.now();
|
||||
const stamp = () => (' ' + (performance.now() - t0).toFixed(0)).slice(-6);
|
||||
|
||||
function log(msg) {
|
||||
const line = stamp() + ' ' + msg;
|
||||
logEl.textContent += line + '\n';
|
||||
logEl.scrollTop = logEl.scrollHeight;
|
||||
console.log('[KbDiag] ' + line);
|
||||
}
|
||||
|
||||
const idOf = (el) => !el || el === document.body ? '(none)' : (el.id || el.tagName.toLowerCase());
|
||||
|
||||
function paintFocus() {
|
||||
const el = document.activeElement;
|
||||
const sel = (el && 'selectionStart' in el && el.selectionStart !== null)
|
||||
? ' sel=' + el.selectionStart + '..' + el.selectionEnd : '';
|
||||
focusEl.textContent = 'FOCUS: ' + idOf(el) + sel;
|
||||
}
|
||||
|
||||
// Focus/blur are the signals the host turns into ime.focus; a tap on an
|
||||
// already-focused field fires NEITHER, which is the case this branch fixes.
|
||||
document.addEventListener('focusin', (e) => { log('FOCUSIN ' + idOf(e.target)); paintFocus(); });
|
||||
document.addEventListener('focusout', (e) => { log('FOCUSOUT ' + idOf(e.target)); paintFocus(); });
|
||||
|
||||
document.addEventListener('click', (e) => {
|
||||
const already = e.target === document.activeElement;
|
||||
log('CLICK ' + idOf(e.target) + (already ? ' [ALREADY-FOCUSED -> needs wantkb]' : ''));
|
||||
paintFocus();
|
||||
}, true);
|
||||
|
||||
document.addEventListener('selectionchange', paintFocus);
|
||||
|
||||
document.addEventListener('input', (e) => {
|
||||
const el = e.target;
|
||||
const v = ('value' in el ? el.value : el.textContent) || '';
|
||||
log('INPUT ' + idOf(el) + ' val="' + v.slice(0, 40) + '" len=' + v.length +
|
||||
('selectionStart' in el && el.selectionStart !== null
|
||||
? ' sel=' + el.selectionStart + '..' + el.selectionEnd : ''));
|
||||
paintFocus();
|
||||
});
|
||||
|
||||
document.addEventListener('keydown', (e) => log('KEYDOWN ' + idOf(e.target) + ' key=' + e.key));
|
||||
|
||||
// Page-level visibility: a tab moved off-screen keeps DOM focus, so this is
|
||||
// how we tell "the host hid us" apart from "the page lost focus".
|
||||
document.addEventListener('visibilitychange',
|
||||
() => log('VISIBILITY ' + document.visibilityState + ' activeElement=' + idOf(document.activeElement)));
|
||||
|
||||
document.getElementById('btnBlur').addEventListener('click', (e) => {
|
||||
e.preventDefault();
|
||||
if (document.activeElement && document.activeElement.blur) document.activeElement.blur();
|
||||
log('ACTION programmatic blur()');
|
||||
});
|
||||
document.getElementById('btnFocusFilled').addEventListener('click', (e) => {
|
||||
e.preventDefault();
|
||||
document.getElementById('filled').focus();
|
||||
log('ACTION programmatic focus(filled)');
|
||||
});
|
||||
document.getElementById('btnClear').addEventListener('click', (e) => {
|
||||
e.preventDefault();
|
||||
logEl.textContent = '';
|
||||
});
|
||||
|
||||
// Heartbeat: catches focus drifting on its own while the tab sits idle.
|
||||
let lastBeat = '';
|
||||
setInterval(() => {
|
||||
const now = idOf(document.activeElement);
|
||||
if (now !== lastBeat) { log('DRIFT activeElement -> ' + now); lastBeat = now; }
|
||||
}, 500);
|
||||
|
||||
log('ready — ' + location.href);
|
||||
paintFocus();
|
||||
</script>
|
||||
Reference in New Issue
Block a user