From b678bf3ccdfba1db5c7ae089e351c5fac4a21b7e Mon Sep 17 00:00:00 2001 From: iris <2+iris@noreply.localhost> Date: Thu, 3 Sep 2026 23:52:43 -0400 Subject: [PATCH] Press things by name in ui-trace, not by coordinate `--do "tap 'Save'"` finds whatever currently carries that label -- the text, or the padded control around it, preferring the clickable one -- resolves its box from the accessibility tree at the moment of the gesture, and presses its centre. It is the name the control already has for assistive technology, so there is nothing extra to keep in step with it. `tap X Y` still works and is now the exception. A coordinate is a position measured once by hand: anything that moves the control makes the tap land on whatever now sits there, and the script then reports a number that was never measured, which reads exactly like a result rather than like a failure. ai-app's two benchmark scripts pressed a header button at `tap 723 205` and that button has now moved; Iris asked on 2026-09-03 that the fix be in the tool rather than a habit each script remembers. Two things make the failure loud, which is the whole point. A label that is not on screen ends the recording with `# error` and a non-zero exit, and `record` now prints the error lines rather than the head of a trace that can be thousands of frames long. And the sampling loop waits for the action thread before exiting -- a script whose last step outlasted the recording used to have its outcome discarded, including that error. The label is looked for over three seconds rather than once: the accessibility connection has no window at all for the first frames after it is made, and a control revealed by the previous step arrives a frame or two later. The wait is bounded and the failure is still loud. Exercised against a running emulator: a tap by label, a tap that finds nothing, and a failure that lands after the recording's own duration. Co-Authored-By: Claude Opus 5 --- README.md | 26 +++++++- bin/ui-trace | 16 ++++- share/ui-trace/UiTrace.java | 122 +++++++++++++++++++++++++++++++++++- 3 files changed, 159 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index 370dcb0..7b57d71 100644 --- a/README.md +++ b/README.md @@ -148,7 +148,7 @@ are comfortable, and three were not. accessibility tree at 60Hz, so an animation or a settling layout is visible rather than happening between two samples. - ui-trace record -d 3000 --do 'tap 540 800' -o /tmp/t.txt + ui-trace record -d 3000 --do "tap 'Session settings'" -o /tmp/t.txt ui-trace elements /tmp/t.txt # what is on screen, to pick from ui-trace show /tmp/t.txt # what moved ui-trace show /tmp/t.txt -m 'Send|Stop' --field box @@ -159,6 +159,30 @@ that measured identically at 171x105 looked different in a screenshot, because the filled one read as larger than the disabled grey one. Screenshots are for appearance — colour, weight, whether it looks right. +### Press things by name, never by coordinate + +`tap 'Save'` finds whatever currently carries that label — the text, or the +padded control around it, preferring the one that is clickable — resolves its +box from the tree **at the moment of the gesture**, and presses its centre. It +is the name the control already has for assistive technology, so nothing has +to be kept in step with it. + +`tap X Y` still exists and is the exception. A coordinate is a position +measured once by hand, and anything that moves the control — a button added to +the row, a font size, a density, another device — makes the tap land on +whatever now sits there. The script then reports a number that was never +measured, which reads exactly like a result rather than like a failure. Asked +for by Iris on 2026-09-03, after ai-app's two benchmark scripts pressed a +header button at `tap 723 205` and that button moved. + +A label that is not on screen ends the recording with `# error action failed` +and a non-zero exit, so a run that could not press what it meant to press +produces no numbers at all. + +The check that none is left in a project's scripts is one grep: + + grep -n "tap [0-9]" path/to/*.sh + `adb exec-out screencap -p` still works and comes back scaled to 800px on its long edge, which is ~380 tokens to read instead of ~1460 and still legible for layout, contrast and small labels. `ADB_SCREENCAP_MAX_EDGE` raises it for diff --git a/bin/ui-trace b/bin/ui-trace index bebdb03..b9e6ac7 100755 --- a/bin/ui-trace +++ b/bin/ui-trace @@ -16,11 +16,18 @@ re-interrogated with a different selector without touching the device again, which matters because the interesting question is usually the one you think of after seeing the first answer. - ui-trace record -d 3000 --do 'tap 540 800' -o /tmp/t.txt + ui-trace record -d 3000 --do "tap 'Session settings'" -o /tmp/t.txt ui-trace elements /tmp/t.txt # what is on screen, to pick from ui-trace show /tmp/t.txt # what moved (the default question) ui-trace show /tmp/t.txt -m 'Called|ask' # a timeline for those +Press things by **name**, not by coordinate. `tap 'Save'` resolves the label +against the tree at the moment of the gesture, so it survives anything that +moves the control and fails loudly when the control is genuinely not there. +`tap X Y` still works and is the exception: a coordinate is a position measured +once by hand, and the first thing that moves the control makes the tap land on +whatever now sits there -- which reads exactly like a result. + Nothing here is specific to any app. """ @@ -102,7 +109,12 @@ def record(args): stderr=subprocess.STDOUT) text = out.read_text() if result.returncode != 0 or "# error" in text: - print(text[:2000], file=sys.stderr) + # The error lines rather than the head of the file. An action that failed -- a + # `tap` whose label is not on screen -- reports itself part way through a trace + # that may be thousands of frames long, so printing the beginning showed a + # perfectly ordinary first frame and said nothing about what went wrong. + problems = [line for line in text.splitlines() if line.startswith(("# error", "# at"))] + print("\n".join(problems[:40]) if problems else text[:2000], file=sys.stderr) sys.exit("ui-trace: the device recorder failed") frames, _ = parse(out) print(f"ui-trace: {len(frames)} frames over {frames[-1][0] if frames else 0}ms " diff --git a/share/ui-trace/UiTrace.java b/share/ui-trace/UiTrace.java index 9e9bc39..151d4b4 100644 --- a/share/ui-trace/UiTrace.java +++ b/share/ui-trace/UiTrace.java @@ -31,6 +31,8 @@ import java.util.List; public final class UiTrace { private static final int MAX_DEPTH = 60; + /** How long `tap LABEL` keeps looking before it decides the label is not there. */ + private static final long FIND_TIMEOUT_MS = 3000; private final UiAutomation automation; private final PrintWriter out; @@ -117,7 +119,11 @@ public final class UiTrace { try { perform(step); } catch (Exception problem) { - synchronized (out) { out.println("# action failed: " + problem); } + // "# error", not a gentler word: the reader of this trace is usually a + // benchmark script, and an action that did not happen must fail the run + // rather than leave numbers that look measured. ui-trace's own `record` + // exits on this line. + synchronized (out) { out.println("# error action failed: " + problem); } } } }, "ui-trace-actions"); @@ -141,6 +147,23 @@ public final class UiTrace { if (slack > 0) SystemClock.sleep(slack); } } + + // Waited for rather than left behind. The actions run beside the sampling, so a script + // whose last step outlasts the recording used to have its outcome thrown away by the + // exit -- including the "# error" from a `tap` that never found its label, which is the + // one line the caller most needs. A run that could not press what it meant to press must + // not end quietly. + try { + actor.join(FIND_TIMEOUT_MS + 1000); + } catch (InterruptedException interrupted) { + Thread.currentThread().interrupt(); + } + if (actor.isAlive()) { + synchronized (out) { + out.println("# error the recording ended before its actions did -- raise -d"); + out.flush(); + } + } } private void emit(AccessibilityNodeInfo node, int depth) { @@ -188,7 +211,13 @@ public final class UiTrace { synchronized (out) { out.println("A " + at + "\t" + String.join(" ", step)); out.flush(); } switch (step[0]) { case "wait" -> SystemClock.sleep(Long.parseLong(step[1])); - case "tap" -> tap(Integer.parseInt(step[1]), Integer.parseInt(step[2])); + case "tap" -> { + if (step.length == 3 && isNumber(step[1]) && isNumber(step[2])) { + tap(Integer.parseInt(step[1]), Integer.parseInt(step[2])); + } else { + tapLabel(join(step)); + } + } case "swipe" -> swipe(Integer.parseInt(step[1]), Integer.parseInt(step[2]), Integer.parseInt(step[3]), Integer.parseInt(step[4]), step.length > 5 ? Long.parseLong(step[5]) : 300); @@ -196,6 +225,95 @@ public final class UiTrace { } } + private static boolean isNumber(String value) { + if (value.isEmpty()) return false; + for (int i = 0; i < value.length(); i++) { + if (!Character.isDigit(value.charAt(i))) return false; + } + return true; + } + + /** Everything after the verb, as one label, with any quotes the shell left on it removed. */ + private static String join(String[] step) { + StringBuilder joined = new StringBuilder(); + for (int i = 1; i < step.length; i++) { + if (i > 1) joined.append(' '); + joined.append(step[i]); + } + String label = joined.toString(); + if (label.length() >= 2) { + char first = label.charAt(0), last = label.charAt(label.length() - 1); + if ((first == '\'' || first == '"') && first == last) { + label = label.substring(1, label.length() - 1); + } + } + return label; + } + + /** + * Presses whatever is currently labelled `label`, wherever it is. + * + * This is the action to reach for, and `tap X Y` is the exception. A coordinate is a position + * measured once by hand: anything that moves the thing being pressed -- a control added to the + * row, a font size, a density, a different device -- makes the tap land on whatever now sits + * there, and the script then reports a number that was never measured, which reads exactly + * like a result. A name is what the control already carries for assistive technology, so it + * survives all of that and fails loudly when it genuinely is not there. + * + * The box is read from the tree at the moment of the gesture rather than from an earlier + * frame, because the screen the gesture lands on is the only one that decides where the mark + * is. + */ + private void tapLabel(String label) { + // Looked for repeatedly for a moment rather than once. Two things make a single look + // wrong: the accessibility connection has no window at all for the first frames after it + // is made, so an action at the very start of a trace found nothing to search; and a + // control revealed by the step before this one -- a dialog's button, a row that has just + // loaded -- arrives a frame or two later. The wait is bounded and the failure is still + // loud, which is the whole point: what must not happen is pressing the wrong thing. + List found = new ArrayList<>(); + long giveUp = SystemClock.uptimeMillis() + FIND_TIMEOUT_MS; + while (true) { + AccessibilityNodeInfo root = automation.getRootInActiveWindow(); + if (root != null) collect(root, label, 0, found); + if (!found.isEmpty() || SystemClock.uptimeMillis() >= giveUp) break; + SystemClock.sleep(50); + } + if (found.isEmpty()) { + throw new IllegalArgumentException("nothing on screen is labelled \"" + label + + "\" (looked for " + FIND_TIMEOUT_MS + "ms)"); + } + // The clickable one where there is a choice: a label and the padded control wrapping it + // both carry the text, and the control is the thing a finger is meant to find. Its centre + // is also where a ripple is drawn, so this presses what the reader would have pressed. + AccessibilityNodeInfo target = found.get(0); + for (AccessibilityNodeInfo candidate : found) { + if (candidate.isClickable()) { target = candidate; break; } + } + Rect box = new Rect(); + target.getBoundsInScreen(box); + if (box.width() <= 0 || box.height() <= 0) { + throw new IllegalStateException("\"" + label + "\" has no size on screen"); + } + synchronized (out) { + out.println("# tap \"" + label + "\" at " + box.centerX() + "," + box.centerY() + + " in " + box.left + "," + box.top + "," + box.right + "," + box.bottom); + out.flush(); + } + tap(box.centerX(), box.centerY()); + } + + /** Every visible node whose text or description is exactly `label`, in tree order. */ + private static void collect(AccessibilityNodeInfo node, String label, int depth, + List found) { + if (node == null || depth > MAX_DEPTH) return; + if (node.isVisibleToUser() + && (label.equals(clean(node.getText())) || label.equals(clean(node.getContentDescription())))) { + found.add(node); + } + for (int i = 0; i < node.getChildCount(); i++) collect(node.getChild(i), label, depth + 1, found); + } + private void send(int action, long down, int x, int y) { MotionEvent event = MotionEvent.obtain(down, SystemClock.uptimeMillis(), action, x, y, 0); event.setSource(InputDevice.SOURCE_TOUCHSCREEN);