fix(flow): refuse invisible-only text selectors — record coordinates instead#511
Merged
Merged
Conversation
…instead
The recorder's gesture-tap→selector rewrite trusted any label that survived
trim(). Icon-font tab bars expose their glyph as the accessibility label —
a Private Use Area code point like U+E163 — so deriveSelector minted a
'stable' text selector that serializes to YAML which DISPLAYS as
`text:` (empty), is meaningless outside the app's private font, and at
replay silently matched an element no reader of the flow can predict.
Reproduced from the 0.15.0 QA flow files (hexdump: ee 85 a3 = U+E163).
Three layers, one predicate (hasVisibleText: strip Cf/Co/Cc, then trim):
- deriveSelector: text with no visible characters is not stable; fall
through to role, then to the recorder's kept-coordinates path — which
already worked and now actually runs for this case.
- selectorToYaml: the empty-text guard now covers the strict map spelling
too (it only guarded the bare-string form) and rejects invisible-only
text — a hand-built selector fails loudly instead of writing an
unreadable flow.
- selectorSchema: parse-side refinement rejects `text` with no visible
characters, so flow files already recorded broken on 0.15.0 fail with
an actionable message instead of false-passing at replay.
Live-verified on the Pixel_9 emulator against the original QA repro:
the same tab-bar tap now records `tap: {x, y}` with the documented
'kept coordinates (brittle)' warning, replays 2/2, and the old broken
YAML fails parse with the visible-character error.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The bug (found in the 0.15.0 full-surface QA, on both the Android emulator and a physical S24)
Recording a
gesture-tapon an icon-font tab-bar item wrote a flow step that displays as an empty selector, with the coordinates gone:and
flow-executethen silently passed against target"", tapping an element no reader of the flow can predict. Root cause (hexdump of the QA-recorded file:ee 85 a3= U+E163): icon fonts expose their glyph as the element's accessibility label;deriveSelectoraccepted any label that survivedtrim(), so a PUA code point counted as "stable text". This violated the rewrite's own documented contract — "otherwise coordinates are kept with a warning" — whose fallback path existed and worked, but never ran for this case.The fix — one predicate, three layers
hasVisibleText()(strip UnicodeCfformat /Coprivate-use /Cccontrol, then trim; require something left):deriveSelector— invisible-only label/value is not stable text; fall through to role, then to the recorder's existing kept-coordinates path.selectorToYaml— the empty-text guard previously covered only the bare-string spelling; it now also rejects empty/invisible text on the strict{text: …}map form, so a hand-built selector fails loudly instead of writing an unreadable flow file.selectorSchema— parse-side refinement, so flow files already recorded broken on 0.15.0 fail with an actionable message ("text must contain at least one visible character … select by identifier or role instead") instead of false-passing at replay. This also hardens theawait-ui-elementtool surface, which shares the schema.Text that merely contains an icon glyph alongside real text ("⟨U+E163⟩ Explore") stays valid throughout.
Verification
deriveSelectorPUA/ZWSP/mixed cases, serialize + parse rejection round-trips (explicit\uXXXXescapes, no invisible bytes in the test source).tap: {x: 0.75, y: 0.947}with the documented "kept coordinates (brittle)" warning — the role fallback was correctly caught by the existing resolves-to-a-different-element guard — replay passes 2/2 on the coordinate targets, and executing the old broken YAML now fails parse with the visible-character error.🤖 Generated with Claude Code