diff --git a/internal/runner/runner.go b/internal/runner/runner.go index 44d4304..cee2b6e 100644 --- a/internal/runner/runner.go +++ b/internal/runner/runner.go @@ -732,15 +732,18 @@ func inputReplacesText(drv driver.DeviceDriver) bool { // whatever holds focus, so a tap the target never received (a keyboard overlay // window covering it, a target that cannot take focus) would stream the // characters into a different field, corrupting it and every property that -// reads it. Hierarchies that carry no focus at all (iOS) leave nothing to -// compare against, so those platforms are not charged the extra read. +// reads it. Only a field that already holds focus can receive that text, so +// the confirming read is charged only when the pre-tap hierarchy shows focus +// somewhere other than the target: a target that already holds focus, a screen +// with nothing focused, and platforms that never report focus (iOS) all skip +// it and keep the round-trip. func confirmFocus( ctx context.Context, drv driver.DeviceDriver, selector string, tree *hierarchy.Tree, ) error { - if selector == "" || !reportsFocus(tree) { + if selector == "" || !otherElementHoldsFocus(tree, selector) { return nil } dump, err := drv.Hierarchy(ctx) @@ -751,31 +754,24 @@ func confirmFocus( if err != nil { return fmt.Errorf("focus check for %s: %w", selector, err) } - focused := focusedElement(current) - if focused == nil { - return nil - } - if target := current.FindNode(selector); target != nil && holdsFocus(target) { + if !otherElementHoldsFocus(current, selector) { return nil } return fmt.Errorf( "focus tap on %s did not focus it: %s holds focus, so the text would land there", - selector, elementName(focused), + selector, elementName(focusedElement(current)), ) } -// reportsFocus reports whether the platform describes focus at all, which is -// what makes a post-tap focus check meaningful. -func reportsFocus(tree *hierarchy.Tree) bool { - if tree == nil { +// otherElementHoldsFocus reports whether the hierarchy shows focus on +// something outside the selector's subtree, which is the state that sends +// typed text to the wrong field. +func otherElementHoldsFocus(tree *hierarchy.Tree, selector string) bool { + if tree == nil || focusedElement(tree) == nil { return false } - for _, element := range tree.Elements { - if _, ok := element.Attributes["focused"]; ok { - return true - } - } - return false + target := tree.FindNode(selector) + return target == nil || !holdsFocus(target) } func focusedElement(tree *hierarchy.Tree) *hierarchy.Element { diff --git a/internal/runner/runner_test.go b/internal/runner/runner_test.go index 1e0fb07..38db86b 100644 --- a/internal/runner/runner_test.go +++ b/internal/runner/runner_test.go @@ -910,6 +910,70 @@ func TestApplyAction_InputTextSkipsFocusCheckWhenHierarchyOmitsFocus(t *testing. } } +const loginFocusOnNothing = `{"attributes":{"resource-id":"root","bounds":"[0,0,1080,2340]"},"children":[ + {"attributes":{"resource-id":"LoginEmail","bounds":"[94,240,986,372]"},"focused":false,"children":[]}, + {"attributes":{"resource-id":"LoginPassword","bounds":"[94,461,986,593]"},"focused":false,"children":[]} +]}` + +// Typed text can only be corrupted into a field that already holds focus, so +// a pre-tap hierarchy showing focus elsewhere is the one class worth the +// confirming read. +func TestApplyAction_InputTextConfirmsFocusWhenAnotherFieldHeldItBeforeTheTap(t *testing.T) { + fastFocusSettle(t) + tree, err := hierarchy.Parse(loginFocusOnEmail) + if err != nil { + t.Fatalf("Parse: %v", err) + } + driverMock := mockdriver.New() + driverMock.HierarchyJSON = loginFocusOnPassword + action := verifier.Action{ + Kind: verifier.ActionKindInputText, + On: "id:LoginPassword", + Text: "ledger123", + } + + mustDispatch(t, driverMock, action, tree) + if !containsAction(driverMock.Actions(), mockdriver.ActionHierarchy, "") { + t.Errorf("another field held focus before the tap: expected the confirming read, got %v", driverMock.Actions()) + } + if !typedText(driverMock.Actions(), "ledger123") { + t.Errorf("expected InputText once the target took focus, got %v", driverMock.Actions()) + } +} + +// The confirming read is a device round-trip on every InputText step. Where no +// other element holds focus before the tap there is no field for the text to +// be corrupted into, so the read buys nothing and must not be paid for. +func TestApplyAction_InputTextSkipsFocusCheckWhenNoOtherFieldHoldsFocus(t *testing.T) { + fastFocusSettle(t) + for name, beforeTap := range map[string]string{ + "target already holds focus": loginFocusOnPassword, + "nothing holds focus": loginFocusOnNothing, + } { + t.Run(name, func(t *testing.T) { + tree, err := hierarchy.Parse(beforeTap) + if err != nil { + t.Fatalf("Parse: %v", err) + } + driverMock := mockdriver.New() + driverMock.HierarchyJSON = loginFocusOnEmail + action := verifier.Action{ + Kind: verifier.ActionKindInputText, + On: "id:LoginPassword", + Text: "ledger123", + } + + mustDispatch(t, driverMock, action, tree) + if containsAction(driverMock.Actions(), mockdriver.ActionHierarchy, "") { + t.Errorf("no other field held focus: expected no confirming read, got %v", driverMock.Actions()) + } + if !typedText(driverMock.Actions(), "ledger123") { + t.Errorf("expected InputText, got %v", driverMock.Actions()) + } + }) + } +} + func typedText(actions []mockdriver.Action, text string) bool { for _, action := range actions { if action.Kind == mockdriver.ActionInputText && action.Text == text {