From 8359fa39bd796be809fd53a2d2e5ccdfa40d3267 Mon Sep 17 00:00:00 2001 From: Anton Lykhoyda Date: Thu, 1 Oct 2026 21:41:32 +0200 Subject: [PATCH] fix(uiautomator2): hideKeyboard confirms the keyboard hidden with two reads A dumpsys read taken while the keyboard is still coming up can report it not shown, and Appium's hide_keyboard can answer 404 at the same moment, so the step returned without pressing BACK and the keyboard stayed over the next target. Treat the keyboard as hidden only when two reads 300 ms apart agree, both before and after Appium's call. BACK is still sent only while the keyboard is shown. --- CHANGELOG.md | 1 + pkg/driver/uiautomator2/commands.go | 5 +- pkg/driver/uiautomator2/keyboard.go | 26 +++++-- pkg/driver/uiautomator2/keyboard_hide_test.go | 71 +++++++++++++++++++ 4 files changed, 95 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ddfda590..022ae9a1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] ### Fixed +- **UIAutomator2 `hideKeyboard` no longer trusts a single "keyboard hidden" read.** While the keyboard is still coming up, `dumpsys window InputMethod` can report it not shown and Appium's hide_keyboard can answer 404, so the step returned without pressing BACK and the keyboard stayed over the next target. The driver now treats the keyboard as hidden only when two reads 300 ms apart agree, both before and after Appium's call, and still presses BACK only while the keyboard is shown. - **WDA keeps enough idle connections for its own parallel reads.** The driver reads an element's name, rect, text and displayed at once, and a tap looks an element up four ways at once, but Go's default transport keeps two idle connections per host, so every burst closed two connections and opened two new ones. Through a forwarded port to a physical iPhone, new connections opened together fail with EOF and are sent again, which costs time on every step. The WDA client now keeps up to eight. - **`retry` counts retries, not attempts, as Maestro does.** `maxRetries: 1` now runs the commands twice (once, then one retry), an unset `maxRetries` means one retry, and the value is capped at 3. The runner ran exactly `maxRetries` attempts, three when unset and with no cap, so `maxRetries: 1` never retried. A value that is not an integer is logged and read as 1 instead of failing the step. - **WDA `launchApp` restarts a running app unless `stopApp: false`, as Maestro does.** It only activated the running app, so a relaunch left the app on the screen it was already on, and a flow checking what survives a restart restarted nothing. diff --git a/pkg/driver/uiautomator2/commands.go b/pkg/driver/uiautomator2/commands.go index b2de8855..4a1f22af 100644 --- a/pkg/driver/uiautomator2/commands.go +++ b/pkg/driver/uiautomator2/commands.go @@ -538,8 +538,9 @@ func (d *Driver) hideKeyboard(_ *flow.HideKeyboardStep) *core.CommandResult { // confirming the keyboard is still up, which is what keeps it from navigating // away (the side effect reported on the devicelab driver). - // If we can confirm the keyboard isn't shown, there's nothing to do. - if d.device != nil && !d.isKeyboardVisible() { + // If we can confirm the keyboard isn't shown, there's nothing to do. One read + // isn't enough: mid-transition it can say hidden while the IME is coming up. + if d.device != nil && d.keyboardConfirmedHidden() { return successResult("Keyboard not visible", nil) } diff --git a/pkg/driver/uiautomator2/keyboard.go b/pkg/driver/uiautomator2/keyboard.go index 545de6bc..61b0dfb9 100644 --- a/pkg/driver/uiautomator2/keyboard.go +++ b/pkg/driver/uiautomator2/keyboard.go @@ -89,21 +89,35 @@ func (d *Driver) isKeyboardVisible() bool { return d.getKeyboardBounds() != nil } -// waitKeyboardHidden polls (up to ~600ms) until the soft keyboard is no longer -// shown, allowing for the dismissal animation. Returns true once hidden. When -// there's no shell to inspect (d.device == nil) it reports hidden immediately — -// the caller can't verify, so it best-efforts the result. +// keyboardHiddenConfirmGap separates the two reads keyboardConfirmedHidden needs: +// while the IME is still coming up, dumpsys can report it not shown for a moment. +const keyboardHiddenConfirmGap = 300 * time.Millisecond + +// keyboardConfirmedHidden reports hidden only when two reads keyboardHiddenConfirmGap +// apart both say the keyboard is not shown. +func (d *Driver) keyboardConfirmedHidden() bool { + if d.isKeyboardVisible() { + return false + } + time.Sleep(keyboardHiddenConfirmGap) + return !d.isKeyboardVisible() +} + +// waitKeyboardHidden polls (about 600ms, plus the confirm gap) until the soft +// keyboard is confirmed hidden, allowing for the dismissal animation. When there's +// no shell to inspect (d.device == nil) it reports hidden immediately — the caller +// can't verify, so it best-efforts the result. func (d *Driver) waitKeyboardHidden() bool { if d.device == nil { return true } for i := 0; i < 6; i++ { - if !d.isKeyboardVisible() { + if d.keyboardConfirmedHidden() { return true } time.Sleep(100 * time.Millisecond) } - return !d.isKeyboardVisible() + return d.keyboardConfirmedHidden() } // tapWouldHitKeyboard returns true if a tap on the element's center would land diff --git a/pkg/driver/uiautomator2/keyboard_hide_test.go b/pkg/driver/uiautomator2/keyboard_hide_test.go index ef0e12b7..890f0e76 100644 --- a/pkg/driver/uiautomator2/keyboard_hide_test.go +++ b/pkg/driver/uiautomator2/keyboard_hide_test.go @@ -1,6 +1,7 @@ package uiautomator2 import ( + "errors" "strings" "testing" @@ -107,6 +108,76 @@ func TestHideKeyboard_NotVisible_NoOp(t *testing.T) { if len(client.pressKeyCalls) != 0 { t.Errorf("expected no key events when keyboard already hidden, got %v", client.pressKeyCalls) } + if len(shell.commands) < 2 { + t.Errorf("expected hidden confirmed by a second read, got %d reads", len(shell.commands)) + } +} + +// errHideKeyboard404 is what Appium answers when it thinks no keyboard is shown. +var errHideKeyboard404 = errors.New("HTTP 404: Soft keyboard not present, cannot hide keyboard") + +// keyboardSequenceShell replays reads in order (repeating the last) until BACK is +// pressed, after which the keyboard reads hidden. +func keyboardSequenceShell(client *MockUIA2Client, reads ...string) *closureShell { + n := 0 + return &closureShell{fn: func(string) (string, error) { + for _, kc := range client.pressKeyCalls { + if kc == uiautomator2.KeyCodeBack { + return kbHiddenDumpsys, nil + } + } + out := reads[min(n, len(reads)-1)] + n++ + return out, nil + }} +} + +func backPresses(client *MockUIA2Client) int { + backs := 0 + for _, kc := range client.pressKeyCalls { + if kc == uiautomator2.KeyCodeBack { + backs++ + } + } + return backs +} + +// A read taken while the keyboard is still coming up says hidden, and so does +// Appium's 404. Neither may stand alone: the next read sees the keyboard, so the +// driver must fall back to BACK instead of leaving it over the next target. +func TestHideKeyboard_HiddenThenVisible_SendsBack(t *testing.T) { + client := &MockUIA2Client{hideKeyboardErr: errHideKeyboard404} + shell := keyboardSequenceShell(client, kbHiddenDumpsys, kbShownDumpsys, kbHiddenDumpsys, kbShownDumpsys) + d := New(client, &core.PlatformInfo{ScreenWidth: 1080, ScreenHeight: 2400}, shell) + + result := d.hideKeyboard(&flow.HideKeyboardStep{}) + if !result.Success { + t.Fatalf("expected success, got %v", result.Error) + } + if client.hideKeyboardCalls != 1 { + t.Errorf("expected Appium HideKeyboard tried once, got %d", client.hideKeyboardCalls) + } + if backs := backPresses(client); backs != 1 { + t.Errorf("expected exactly one BACK, got %d (keyCalls=%v)", backs, client.pressKeyCalls) + } + if result.Message != "Keyboard hidden (via back key)" { + t.Errorf("unexpected message %q", result.Message) + } +} + +// Appium answers 404 while the keyboard is plainly up: BACK closes it. +func TestHideKeyboard_Appium404KeyboardVisible_SendsBack(t *testing.T) { + client := &MockUIA2Client{hideKeyboardErr: errHideKeyboard404} + shell := keyboardSequenceShell(client, kbShownDumpsys) + d := New(client, &core.PlatformInfo{ScreenWidth: 1080, ScreenHeight: 2400}, shell) + + result := d.hideKeyboard(&flow.HideKeyboardStep{}) + if !result.Success { + t.Fatalf("expected success, got %v", result.Error) + } + if backs := backPresses(client); backs != 1 { + t.Errorf("expected exactly one BACK, got %d (keyCalls=%v)", backs, client.pressKeyCalls) + } } // Extended orientations write user_rotation and then wait for `dumpsys