Skip to content

Commit 346d2eb

Browse files
committed
fix(ios): address review — focus-change gate + secure-field clear
Review P1 (focus race): the isKeyboardVisible early-exit in stabilizeTextInputBeforeTyping and waitForTextEntryReadiness fired the instant the keyboard was visible — but when it was ALREADY up from a previous field (back-to-back fills), that is before first-responder moves to the newly-tapped field, so app.typeText could target the old field. Gate the fast-path on a keyboard hidden->visible TRANSITION via a shared keyboardBecameVisible(wasVisibleAtEntry:) helper; when the keyboard was already up, fall back to the settle/timeout (the prior, correct behavior) instead of the ~2.4s dead wait the fresh case avoids. Review P1 (F2): clearTextInput used editableTextValue(...) ?? "" and skipped clearing on empty — but editableTextValue returns nil for secure (and unknown) fields, so secure fields were NEVER cleared and replace concatenated stale+new. Distinguish nil (clear) from "" (skip). Device-validated: fresh fill fast-path preserved + exact; a second fill with the keyboard already up still types into the correct field and replaces (not concatenates).
1 parent 5af67e9 commit 346d2eb

1 file changed

Lines changed: 30 additions & 16 deletions

File tree

ios-runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+Interaction.swift

Lines changed: 30 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -316,13 +316,16 @@ extension RunnerTests {
316316
}
317317

318318
func clearTextInput(_ element: XCUIElement) {
319-
// Nothing to clear: skip both the delete burst and the moveCaretToEnd edge-tap. The
320-
// edge-tap computes a point from the element frame, which can be stale after the field
321-
// repositions on focus (e.g. the Settings search bar jumps bottom->top and reveals a
322-
// "Suggestions" list) — tapping there navigates away instead of clearing. Replacing into
323-
// an already-empty field is a no-op, so returning early is also semantically correct.
324-
let existing = editableTextValue(for: element, treatingPlaceholderAsEmpty: true) ?? ""
325-
if existing.isEmpty {
319+
// Skip the clear (delete burst + moveCaretToEnd edge-tap) ONLY when we can confirm the
320+
// field is empty. Why skip: the edge-tap computes a point from the element frame, which can
321+
// be stale after the field repositions on focus (e.g. the Settings search bar jumps
322+
// bottom->top and reveals a "Suggestions" list) — tapping there navigates away instead of
323+
// clearing; and replacing into an already-empty field is a no-op anyway.
324+
// editableTextValue returns nil for secure (and unknown) fields, where we CANNOT confirm
325+
// emptiness — those must still be cleared, or replace would concatenate stale + new text.
326+
// So distinguish nil (clear) from "" (skip).
327+
if let existing = editableTextValue(for: element, treatingPlaceholderAsEmpty: true),
328+
existing.isEmpty {
326329
return
327330
}
328331
#if !os(tvOS)
@@ -425,15 +428,15 @@ extension RunnerTests {
425428
return target
426429
#else
427430
let latest = target
431+
let keyboardVisibleAtEntry = isKeyboardVisible(app: app)
428432
let deadline = Date().addingTimeInterval(TextEntryTiming.focusTimeout)
429433
while Date() < deadline {
430434
if let focused = focusedTextInput(app: app) {
431435
return focused
432436
}
433-
// focusedTextInput is intentionally nil on iOS, so the software keyboard becoming
434-
// visible is the reliable readiness signal. Exit as soon as it is up instead of
435-
// always burning the full focusTimeout.
436-
if isKeyboardVisible(app: app) {
437+
// focusedTextInput is intentionally nil on iOS; treat the keyboard transitioning to
438+
// visible after our tap as the focus-moved signal. Don't fast-path when it was already up.
439+
if keyboardBecameVisible(app: app, wasVisibleAtEntry: keyboardVisibleAtEntry) {
437440
return latest
438441
}
439442
sleepFor(TextEntryTiming.pollInterval)
@@ -886,6 +889,7 @@ extension RunnerTests {
886889
) -> XCUIElement? {
887890
#if os(iOS)
888891
var latest = resolveTextEntryElement(app: app, target: target)
892+
let keyboardVisibleAtEntry = isKeyboardVisible(app: app)
889893
let deadline = Date().addingTimeInterval(timeout)
890894
let hardwareKeyboardFallback = Date().addingTimeInterval(
891895
min(TextEntryTiming.hardwareKeyboardFallbackTimeout, timeout)
@@ -898,11 +902,12 @@ extension RunnerTests {
898902
return focused
899903
}
900904
}
901-
// The software keyboard being visible is the reliable readiness signal on iOS
902-
// (focusedTextInput is intentionally nil there). Once it is up the field is accepting
903-
// input, so return immediately rather than burning the full readinessTimeout — the
904-
// warmup-first-char echo check and post-type verify/repair remain as drop safety nets.
905-
if isKeyboardVisible(app: app) {
905+
// Fast-path on a keyboard hidden->visible transition: our tapped field gained focus, so
906+
// return immediately instead of burning the full readinessTimeout (warmup-first-char echo
907+
// + post-type verify/repair remain as drop safety nets). When the keyboard was ALREADY up
908+
// (back-to-back fills), this isn't a focus signal — fall through to the settle/timeout so
909+
// text isn't sent to the previously-focused field.
910+
if keyboardBecameVisible(app: app, wasVisibleAtEntry: keyboardVisibleAtEntry) {
906911
return latest
907912
}
908913
sawSoftwareKeyboard = sawSoftwareKeyboard || keyboardElementExists(app: app)
@@ -961,6 +966,15 @@ extension RunnerTests {
961966
return visibleKeyboardFrame(app: app) != nil
962967
}
963968

969+
/// A focus-moved signal for iOS text entry, where `focusedTextInput` is intentionally nil.
970+
/// The software keyboard TRANSITIONING from hidden (at entry) to visible means the field we
971+
/// just tapped gained first-responder. If the keyboard was ALREADY up (e.g. back-to-back
972+
/// fills into different fields), its visibility is not evidence focus moved to the new field,
973+
/// so callers must keep waiting rather than typing into the previously-focused field.
974+
private func keyboardBecameVisible(app: XCUIApplication, wasVisibleAtEntry: Bool) -> Bool {
975+
return !wasVisibleAtEntry && isKeyboardVisible(app: app)
976+
}
977+
964978
private func keyboardElementExists(app: XCUIApplication) -> Bool {
965979
#if os(iOS)
966980
var exists = false

0 commit comments

Comments
 (0)