mirror of
https://github.com/software-mansion/argent.git
synced 2026-09-14 19:27:14 +08:00
fix/android-clear-verify-selectall
544 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
|
|
c3d31f510e |
docs(keyboard): the blind fallback keeps a tail on a multi-line field, not only a head
The unreadable-length run is `KEYCODE_MOVE_END` plus 158 backspaces, the same end-of-LINE start the sentence two clauses earlier attributes to the repair. So on a multi-line field everything below the caret's line survives it, and "a field only keeps its head" was true of the single-line case alone. |
||
|
|
17489b428d |
docs(keyboard): separate the two un-rolled-back cases, which leave different residue
One clause explained both: "the characters already sent stay in the field". That is the transport failure. The rescue-arm 400 sends no characters at all — what it leaves is whatever the select-all and the delete did — so the sentence described the case it was stretched to cover as the case it was written for. |
||
|
|
6068be415b |
fix(keyboard): drop a dead guard from the placeholder rule, and align the two escape lists
`text !== ""` could never change the answer: an empty `text` measures 0 down the other arm too, so short-circuiting it to 0 was a branch with no outcome. Caught by mutation — removing it left all 207 tests green, which is what a clause that decides nothing looks like. The `cleared` docstring called all four ways out "escapes, skipped silently in each", but the end-of-LINE one is not a skip: that repair runs and simply stops at the caret's line. Three skip it, a fourth limits it. The tool description also listed three of the four; it names the placeholder one now, so the two prose sites enumerate the same set. |
||
|
|
496b38c94d |
fix(keyboard): don't delete a field's own placeholder as clear residue
uiautomator renders an empty field's hint into the same `text` attribute it
uses for a value, and `measureFocusedTextLength` reads that attribute raw.
That was harmless while the reading was only a COUNT — over-measuring costs
a few no-op backspaces — but the select-all's read-back turned it into a
PREDICATE, where an over-measurement is a delete run against a field the
clear already emptied. Reproduced on the exact shape this file documents for
API 36 (`text="Search settings" … hint="Search settings"`, on a level that
HAS `keycombination`, so squarely on this path): a clear that fully worked
emitted `input keyevent 123` plus 23 backspaces into the empty box. Past
MAX_DELETE_COUNT it is worse than wasteful — a 171-character placeholder
turned the same successful clear into a 400 with the `{ clear, text }` call's
text never sent.
`hint` is the discriminator and `parseUiAutomatorXml` already surfaces it, so
the read-back now counts a `text` equal to its own `hint` as empty. Only the
read-back: the sizing read wants the opposite error, since under-measuring
there truncates the field, and the levels it serves emit no `hint` anyway.
That made three rules that all fork the same way, so `maxDumps` and
`unmeasurableIsUnreadable` collapse into one `readBack` flag naming the
caller rather than each rule.
Also, from the same pass:
- The tool description promised "a rejected request never changes the field …
a 400 leaves the field exactly as it was". The rescue arm's over-length
refusal is a 400 raised after a select-all and a DEL have gone out, which
the refusal's own text admits. The paragraph now names that one exception.
- The refusal said the DEL "removes one character where it lands" — true only
where the chord was swallowed. A chord that took makes the same DEL remove
the whole selection; both outcomes are named now.
- The `cleared` docstring listed three escapes from the repair. There are
four, and one is wider than stated: an unmeasurable focused editable
ANYWHERE on screen discards the reading, not only one in the target field.
- `devtoolsHierarchyReader` and `AndroidClearOptions.readHierarchy` still
described a legacy-only consumer and the wrong failure mode for the modern
one, which is now this reader's dominant caller.
- The per-device chain's rationale said a character arriving after the DEL
"replaces the whole selection". The DEL consumes the selection; what a
character landing in the read-back window actually gets is measured as
residue and backspaced away.
Mutation-checked: dropping the placeholder rule, applying it to the sizing
read, matching on `hint`'s presence instead of its value, and each of
`readBack`'s three rules all fail their own tests and nothing else.
|
||
|
|
56d03bf788 |
fix(keyboard): say what the read-back does not repair, and stop asserting what it cannot know
Seven reviewer findings on the verify leg, all of them about a claim the code does not hold up. The read-back leaves two shapes exactly as broken as the fast path does, and neither was named anywhere. `measureFocusedTextLength` counts focused `EditText` nodes only, so a focused WebView input or a custom widget reads as unreadable and gets no rescue at all — reproduced in the unit harness: a focused `android.webkit.WebView` carrying `Monda` sends the chord and the DEL and nothing else, under `cleared: true`. And the rescue is `clearByDeleting`, whose `KEYCODE_MOVE_END` is end-of-LINE, so a multi-line residue is emptied only down to the end of the caret's line — reproduced likewise: `AAAA\nBB\nC` minus the DEL's character emits `input keyevent 123` plus seventeen backspaces and completes clean. The `clear` parameter, the tool description and the `cleared` docstring each said the repair without either qualifier. Two messages still stated device state they cannot establish. The rescue-path over-length refusal opened with "The select-all did not take on this field" and then conceded, three sentences later, that the count can be another window's field or an empty field's placeholder; it now reports what it actually observed — the field read back non-empty. The interrupted-DEL report enumerated two states, but a delete already handed to the device keeps landing after adb is killed, so on a widget that swallowed the chord the field is one character short: a third state, now named. The tool description's 158-backspace fallback sentence had lost its older-levels scoping to a sentence inserted before it, and reads as though a modern level could fire it; it says "on those older levels" now. Also covers the verify leg's `options.readHierarchy` source. Every positive-residue test seeded the read-back through the exec-out dump, which on the ordinary describe -> tap -> keyboard order is the source that cannot answer — dropping `options.readHierarchy` from the verify read left all 202 tests green while disabling the repair whenever the devtools helper holds the UiAutomation connection. Mutation-checked: restoring the flat cause, the two-state enumeration, or the undefined `readHierarchy` each fails exactly its own test. |
||
|
|
b605e1e509 |
fix(keyboard): don't assert a device state the clear cannot know
Two error messages stated as fact what the code has just finished establishing it cannot know. The interrupted-DEL report said the select-all landed and survives — but `input` exits 0 whether or not the selection took, which is this branch's whole premise, so on a swallowed-chord widget that message sent a replacement into an intact, unselected value while promising it would replace a live selection. The rescue-path over-length refusal said the field HAS been modified and is one character down — but the reading that reached it is the longest focused EditText on screen, which can be another window's field or an empty field's placeholder, so what the DEL actually took is unknowable from here. Both now hedge and point at reading the field, which was already their remedy. The tool description also scoped the over-150 refusal to levels without `input keycombination`; through the verify leg the same refusal is reachable on a level that has the subcommand, so the sentence says so now. |
||
|
|
859cc138f1 |
docs(keyboard): split the clear's verification story per backend
Chromium verifies and refuses rather than repairing, so the shared 'both delete what survived' wording was wrong about it; the cleared docstring's lead-in also still said only one backend observes. |
||
|
|
c7e0ccd659 |
docs(keyboard): state the clear contract in the present tense
Drops the transition narration from the cleared docstring and names the unmeasurable-field case among the Android read-back's best-effort degradations, which the tool description omitted. |
||
|
|
b6e903800d |
fix(keyboard): don't read a password field's blind floor as clear residue
The select-all read-back inherited the sizing read's BLIND_DELETE_COUNT floor, so every successful clear of an unmeasurable field — a focused password box — read 150 back and fired a 158-keyevent run into the field just emptied. The read-back now reports an unmeasurable focused editable as unreadable, which leaves the fast path alone. Also refreshes the keyboard tool and result docs that still said Android never reads the field back. |
||
|
|
022e10ce89 | record the length dependence behind the delete-run cap | ||
|
|
f9acc326fc |
fix(keyboard): read the field back, instead of trusting a select-all that exits 0
`input keycombination 113 29` reports exit 0 whether or not the selection took. On a React Native `TextInput` it repeatedly does not: 15 of 85 clears driven through the tool on a Pixel 6 / API 34 came back short by exactly the character the DEL removed, each returning `cleared: true` over a value the `text` was then appended to. The same 85 against a native `EditText` ran clean, so nothing about the call distinguishes the two. No hold duration fixes it — `-t 100` still missed 2/30 and `-t 300` 7/30 — so the clear reads the field back and hands a residue to the selection-free delete run, which needs no selection to be correct and cleared the same field 50/50. Only a positive reading redirects: an unreadable one leaves the fast path as it was rather than spending a blind run on a clear that likely worked, and the read-back does not retry the way the sizing read does, whose stake is a truncated field rather than a skipped rescue. The rescue carries its measurement down so the delete run does not dump again, and the over-length refusal it can reach no longer tells that caller the field is untouched — reached from the select-all path, one character is already gone and the level does have `keycombination`. Also mocks `adbExecOutBinary` in keyboard-android.test.ts, which addressed `emulator-5554` for real: with an emulator attached those tests dumped a live device, so a clear's command stream depended on what was on its screen. Verified: 85/85 clean through the tool-server against the field that broke it, against 15/85 corrupt on the same path unfixed. |
||
|
|
75e4e04c08 |
docs(flow): stop offering two clears a TV cannot do, and an assert that does not exist
`flow-yaml.md` ended its three-TV-family paragraph with "Delete an existing value with repeated backspaces, or use the field's own clear affordance". The `keyboard` tool rejects `key` outright on a TV target, so repeated backspaces are available on Vega alone; on Apple TV and Android TV the field's own affordance is the only way. Both halves were offered to all three families. The clear-only parse test justified itself with "empty the box, then assert the empty state", but there is no such assert: `equals: ""` and `contains: ""` are rejected at parse time and `matches: '^$'` never fires, because absent or empty text is not a haystack — as `references/asserting-field-values.md` already says in words. The comment now names the reasons a clear-only step is legitimate on its own, and points at that reference for what can actually be asserted afterwards. |
||
|
|
2e508c02d9 |
fix(flow): report a type step cancelled mid-Enter as a skip, not a pass
The post-dispatch abort re-check sits before the submit gate, so nothing stood
between the Enter dispatch and `return { ok: true }`. `dispatchOrAbort`
reclassifies only a dispatch that REJECTS under an aborted signal, and the
Android backend takes no signal at all, so a cancel landing mid-Enter left the
call to resolve and handed back `true`. The same cancellation window therefore
flipped a step between `pass` and `skip` on which of its two keyboard calls
happened to be in flight — and the `pass` arm is the one where the submit really
was sent after the caller gave up, which is what the re-check above exists to
prevent.
Driven through the real runner with a signal-less keyboard mock: `{ text }` with
`submit` defaulting on reported `type:pass` with both dispatches issued, and
reports `type:skip` now. An explicit `submit: true` clear-only step, where the
Enter is likewise the last dispatch, is pinned alongside it.
Also corrects two claims the same block makes:
- the backends that take no signal. Chromium now honours one, so Android is the
only one that takes none — and the two that DO only narrow the window rather
than close it, since each checks between keystrokes.
- the `runType` parenthetical's premise. `scrollIncrement` is not "one call per
step": a `scroll-to` calls it up to MAX_SCROLL_ITERATIONS times through
`scrollToVisible`, which guards the signal at the head of every iteration
itself — so the dispatch is safe to leave bare, but not for the stated reason.
|
||
|
|
d91bf6ee51 |
fix(flow): report a cancelled clear-only step as a skip, not a pass
`dispatchOrAbort` reclassifies only a dispatch that REJECTS while the
signal is aborted, and the Android and Chromium keyboard backends take
no signal at all — so a cancel landing mid-call leaves the call to
resolve and returns `true`. The re-check that catches that lived inside
the `if (step.submit ?? step.text !== undefined)` block, which is false
for a clear-only step, so the two shapes diverged: cancel a run during a
`{ into, clear: true }` step's one dispatch and it reported `pass`, while
the identical cancellation on a step with text reported `skip`.
The check moves out of the submit gate. The submitting path is unchanged
— it still runs before the Enter dispatch — and the clear-only path
gains it.
`flow-abort.test.ts` grows the twin of its existing clear-only case: the
existing one mocks the keyboard to throw, which `if (!sent)` already
caught; this one resolves inside the cancelled window, and asserts the
clear DID go out so it cannot pass by the step never reaching the
keyboard. It fails with `type:pass` before this change.
Two review findings on the same file that are documentation, not
behaviour:
- The `FocusOutcome` docblock named the FOCUS_REPORTING_SOURCES guard as
a producer of "unobservable". No flow can reach it: `type` runs only on
ios, android and chromium, and each hard-codes a source already in the
set. It is kept as a forward guard — a new platform would otherwise
poll the full timeout on every type step — and the docblock now says
which of the two producers is live.
- `focusedFromInside`'s containment conjunct was pinned by nothing: every
fixture pairing an out-of-target focused node with a small enough frame
is refused by the namesake counts first, so dropping `frameWithin`
reddened no test. The new fixture puts a same-`resource-id` sibling
outside the container's box, where both counts hold and containment is
the only thing left to refuse on — the ordinary Android shape, since a
resource-id names the layout slot. Verified by mutation: dropping the
conjunct now fails that test and only that test.
|
||
|
|
026b36c3ad |
docs: fit the type directive's polish rules to the two-step recording
Three corrections, all from the same cause: `keyboard` rejects `text` and `key` in one call, so a recorded type-then-submit is ALWAYS two steps and the polish rules have to read them as a pair. `live-authoring.md`'s two carry-across rules contradicted each other on exactly that shape. "Carry a recorded `key: "enter"` across as `submit: true`" covers the Enter step; "a recorded step with `text` but no `key` needs `submit: false`" covers the text step — and always will, since no recorded text step can carry a key. Following both produced `submit: false` for a recording that did submit, so the cleaned login flow never fired the form; following only the first and leaving the raw Enter step in place submitted twice. The rules are now one table over the whole run of `keyboard` steps on a field, which is the unit the conversion actually operates on, with the two failure modes named under it. The conversion table further down defers to it rather than giving a second, shorter answer. `keyboard`'s `clear` parameter said a swallowed select-all leaves the field "one character shorter … and a combined `text` then appends to that". Per the caret rule in the same sentence, that holds on iOS only: on Android the surviving delete acts as a backspace at the caret the tap left mid-value, and the text is spliced there. `flow-yaml.md` already documented the splice per platform, so the two disagreed on Android. `asserting-field-values.md` did not mention that Chromium truncates every accessible name at 200 characters — Chromium alone. An `equals` against a longer value cannot match, and a long pre-filled field is one of the shapes `clear` exists for. |
||
|
|
da61986dec |
test: pin this round's fixes, and the gaps the review's sweep found
New coverage for each fix, every one of them mutation-checked (revert the fix,
the named test goes red):
- an editing host holding identified or password content beside the named
paragraph refuses, while the bare `<div contenteditable><p>…</p></div>`
still clears;
- the tapped element unmounting while a namesake holds focus refuses, and
the same page with it mounted still clears;
- the three ways a focused node borrowed another element's evidence — a
namesake the tap creates, a container that grows onto a field the tap never
reached, a focused field that renames itself out of the namesake set — plus
the control that keeps the name;
- a slow-but-answering tree source is not reported as an outage;
- the chromium walker keeps an invisible element that holds the caret (and
one that contains it), still prunes when the caret is elsewhere, never
resurrects a display:none subtree, and keeps a box-less focused node.
And the gaps the sweep listed:
- no test parsed a non-object `type` body, so the message could name one of
the two valid shapes and stay green;
- no test covered a non-abort `gesture-tap` rejection in `runType`;
- nothing bounded the focus window from ABOVE;
- the skill docs' `clear` YAML examples were outside the parse guard's slice;
- `carriesFocus`'s recursion fixture put the flag at depth 0, where a
non-recursive check reads the same — give the shadow wrapper an id;
- four weak assertions: `toBeDefined()` on a `findById` that returns null,
an `?.identifier` that reads undefined when nothing resolved, a `toBe`
between two strings that pass as `"" === ""`, and a flat-leaf invariant
omitting `focused`, which is now producible.
Also drops the wall-clock half of the early-exit test: `elapsed < 3000` timed
the whole step — temp dir, device resolution, the settle, two keyboard
dispatches — so it measured the host and failed at 3066ms and 3388ms under
load, while the read count beside it pins the exit exactly at any speed.
|
||
|
|
66a67d5f3a |
docs: correct what the clear's own docs claim about it
- `live-authoring.md`'s Replay section said a `type:` whose focus was never confirmed is scored a pass. True of a plain `type`; a `clear` fails. - `flow-yaml.md` said the clear dispatches on three findings and no others. There is a fourth — the window where no read saw focus — which the same paragraph goes on to describe. - The residual behind that fourth is materially bigger than "nothing was focused, so nothing can be lost", in both `flow-yaml.md` and `runType`'s comment. An audit of the prunes in the adapters gives at least four more shapes: an off-screen or scroll-clipped frame, iOS's `hidden`/`alpha` prune, Android's system-UI prune (the notification direct-reply field, the bouncer), and a truncated tree on any source. Each is a field the step never named, and the step reports a pass. - `dispatchOrAbort`'s docblock said android and chromium check the signal between keypresses like iOS. They check once, at their per-device queue, and neither `runAndroidPhoneType` nor `runChromium` takes a signal — cancel a run mid-`type` there and the whole text is still typed. - The same block said `gesture-tap`'s platform handlers discard the context. It has no platform handlers, and `execute` declares no context parameter. - `waitForFrameAndTree` said its stale-tree window opens only when a read fails outright; `settleTree` also returns best-effort when every read succeeded and no two matched. - `describe/contract.ts` said platforms other than Vega leave `focused` unset. All three set it, and the flow focus wait reads nothing else. - `flow-run.ts`'s description never said a `clear` can hard-fail the step, and mentioned `submit` only for the clear-only default. - `flow-finish-recording.ts` described `type.submit` in the one direction that no longer matters. - `reliability-and-recovery.md` prescribed "assert the committed value" with no pointer to the reference that says when it hard-fails. - `argent-qa-flows` never mentioned `clear`, though running the same YAML twice with no manual data reset is exactly what it is for. |
||
|
|
2b9e7b8de8 |
fix: the LOW findings around the gate, the parser and the walker
- `focusedFromInside`'s trailing `underTap.every(...)` is unreachable as false once the `controls` rule holds, so drop it and state why it is implied. - A `type:` step with no `into` rendered its entry as the bare word `undefined`, which in a multi-step flow does not say which step is broken. Catch it in the type arm and echo the step, the way the `claer` typo path already does. - The two clear refusals blamed an overlay or a sibling for what is routinely a field OVERHANGING the container the selector names — default content-box sizing does it for a few pixels, and neither message admitted the possibility. Reproduced and re-read on Chrome 151. - `chromium.ts` claimed the focus flag reaches every describe consumer: `hasContent` does not list `focused`, so a node whose only signal is focus is dropped from the agent-facing render. Verified — an anonymous empty focused contenteditable is absent, the same editor holding a `<p>` is shown. - It also claimed focus inside an iframe leaves the tree with no flag at all. True in the light DOM; inside an OPEN shadow root the HOST is flagged, since the excluded iframe is the root's activeElement. Verified both ways. - The zero-frame sibling drop now consults `focusedSelf` like the two structural collapses beside it — same class of drop, same consequence. |
||
|
|
6014a56d7b |
fix(flow): stop the window's own edge being reported as a dead tree source
Every focus poll is given whatever is left of the window as its read budget, so a read started too late is abandoned mid-flight — and `giveUp` books that abandonment as darkness. The step then failed telling the author "the device's tree source is down" while every read in the window had answered, and because crossing FOCUS_DARK_TAIL_TOLERANCE_MS depends on how the read time divides into the 3000ms window, the verdict was not monotonic in latency. Reproduced on Chrome 151 against the branch tool-server, on a page whose focus the tree cannot report and whose reads were slowed uniformly: 0ms and 150ms passed, 250ms failed as an outage, 300ms and 400ms passed again. No read failed in any run. Android's `getHierarchy` sits in that band — 2.85s / 0.29s / 0.46s / 0.30s across four consecutive idle describes on this host — which is what turns it into a flaky step rather than a consistently wrong one. A read the window cannot afford is no longer started, measured against what the last successful read on this device cost. The same band now passes at every latency, 250ms three times over, while a source whose read never completes inside the window is still reported as an outage. |
||
|
|
8e80634389 |
fix(describe): keep the branch holding the caret through the visibility prune
The walker prunes an invisible subtree before focus is ever considered, so the field that is invisible BY DESIGN — the capture input under rendered OTP/PIN boxes, a barcode-scanner sink, a styled file input — left the tree with no focus flag anywhere. The type directive's focus wait reads that as "nothing is focused", which is the one non-confirmed outcome it dispatches a destructive clear on, and the field the caret is in is very often the one holding the secret. Reproduced on Chrome 151 against the branch tool-server: rendered OTP boxes over an opacity:0 capture input holding `SECRET-OTP-123456`, a step naming the visible box — tree focus flags NONE, step pass, secret rewritten. The identical page at opacity:1 refused. It now refuses at both, with the secret intact. opacity:0 does not stop an element being focused, so the prune now asks whether the element is (or contains) the caret's element first. With nothing focused, opacity:0 and display:none subtrees are pruned exactly as before. |
||
|
|
906a7272fe |
fix(flow): require the focused node's name to be unambiguous in BOTH trees
`focusedFromInside` joins its two halves by name — the focused node comes from
the current tree and the current container box, the evidence about it from the
pre-tap tree and the pre-tap box — and only the pre-tap side was checked for
ambiguity. A focused node could therefore borrow the evidence collected for a
different element. Three ways in, all reproduced on Chrome 151 against the
branch tool-server:
- a namesake the TAP CREATES: a wrapper that appends a second `row_input` on
focus and focuses it passed, and rewrote the field the step never named;
- a container that GROWS: a 44px row growing to 140px on focus swallowed a
field that was never a tap candidate, and cleared it;
- a focused field that RENAMES ITSELF out of the namesake set (a currency
input dropping its separators), leaving the sibling under the tap as the
unique pre-tap match — the amount-row mis-target the docs say is refused.
All three leave two namesakes inside the container in the current tree, so
deciding on that too refuses all three, with nothing emptied. The controls that
remove the growth, the late namesake or the rename still refuse, and the
legitimate wrapper clears still pass — a testID box over one input, both with
the tap centre on the input and with it on a tall label.
|
||
|
|
0cb4fb41b9 |
fix(flow): keep the selector fallback on the point the tap was dispatched at
`trackTarget`'s last resort re-runs the SELECTOR, so what it hands back has no relation to the element the step touched — while `waitForFocus`'s identity arm trusts it absolutely, ahead of the two arms that check a node against `tapCandidates`. When the tapped element stops being findable and a different element answering the same selector holds focus, that element was confirmed and cleared. Reproduced on Chrome 151 against the branch tool-server: `aria-label="Email"` removed on `mousedown`, `aria-label="Email address"` focused beside it — the step passed and `DRAFT-DO-NOT-ERASE` became `replaced-by-clear`. It now refuses and the draft survives. The fallback exists for a re-render that renames a node carrying no identifier, and a re-render happens in place: an anonymous field that swaps its `aria-label` on focus is still followed and still cleared, verified on the same page. |
||
|
|
a71dad6160 |
fix(flow): count an editing host's content, instead of trusting its hoist
`focusedOwnsTargetText` confirmed a clear whenever the focused node's hoisted text equalled the target's. The hoist is not a full account of what the node holds: an identified descendant — or a password field — shields its own text out of every ancestor's `subtreeText`, so a composer holding the named paragraph PLUS any amount of identified content compared equal and took the clear, which is a select-all over the whole editing host. Reproduced on Chrome 151 against the branch tool-server: an editor holding `<p>Draft body paragraph</p>` beside `<div id="signature">` passed, wiped the editor to the replacement and destroyed the signature; `data-testid` and an `<input type=password>` did the same. Making the sibling anonymous refused, which is the shield being the only discriminator. Count the content from the flat tree instead, where the shielded node is still its own leaf. Same three pages now: identified, testid and password siblings all refuse and leave the editor intact, while the bare `<div contenteditable><p>…</p></div>` the guard exists for still passes. |
||
|
|
7076b5f0f8 |
test: pin every fix in this round, and the gaps the review's sweep found
Each of these was verified by mutation: apply the pre-fix code, the named test
goes red; revert, green. Thirteen mutations, thirteen killed.
New coverage for the fixes:
- a focus read that HANGS past the window rather than throwing (H1), asserting
both the outage refusal and that the step answered while the read was still
outstanding - the bound is what keeps "within 3000ms" true;
- a container with a GAP down the middle, once with two controls and once with
one (H3), so what decides is the number of controls and not the gap;
- a focused wrapper around one anonymous FIELD (M3), the editing host's twin;
- `type: { clear: true }` with no `into` as a validation failure, not a crash (L1);
- a replaced getRootNode still reporting focus (L3), and a page describing in
full with ShadowRoot set to a non-constructor (L5).
And the gaps the review named:
- T1 the enclosing-pad sweep now starts at 1px, where the defect lived;
- T4 an UNIDENTIFIED field's draft hoisting into its container's text and
out-ranking a real control for a tap - the three tests beside it all give
their field a placeholder or an id, so they pinned the shielded case while
reading as proof of the general property;
- T5 the new asserting-field-values reference gets a parser guard: `equals: ""`
and `contains: ""` really are rejected, `matches: "^$"` really does parse;
- T6/T7 the ambiguity fixtures now exercise the gates they name. The anonymous
row put focus on the field the tap MISSED, which later gates refuse on their
own, so both arms passed with the ambiguity gate deleted; it now differs in
the colliding name alone, and the same-id re-layout gets the tree order where
"first match wins" would hand back the focused row.
|
||
|
|
dff8420aec |
fix: the LOW findings around the focus read, the walker and the parser
L1 - `type: { clear: true }` with no `into` crashed with "Cannot read
properties of undefined" instead of failing validation: JSON.stringify returns
undefined for undefined, and badEntry read .length off it. The crash predates
the branch, but `clear` in the allowlist is a new door onto it - at the merge
base the same flow failed cleanly as an unknown key. Both `clear` and `text`
forms now return FLOW_ENTRY_UNRECOGNIZED naming `type.into`.
L3 - a page that REPLACES Element.prototype.getRootNode (ShadyDOM,
webcomponents.js) hands back an object that is neither a ShadowRoot nor the
document, and the focus read answered null for it: verified on Chrome 151, the
flag vanished from every node on the page, which is the no-focus tree a
destructive clear goes through on. It falls back to the document's
activeElement - the read this had at the merge base. Focus survives the
replacement now, and an open shadow root still flags the inner element only.
L4 - the iframe descent is dead in a renderer (walk() bails on an inner
documentElement, whose constructor belongs to the inner realm), so it is gone
along with the file header's claim to walk subdocuments. Verified before and
after: an <iframe srcdoc> leaves no trace either way. Its only test passed
because the harness is single-realm, so it went too (T8).
L5 - `typeof ShadowRoot === "undefined"` does not cover a page assigning a
non-constructor. With window.ShadowRoot = null, ShadowRoot.prototype threw
before a single node was walked and describe failed for the whole page
(reproduced). Both that read and the instanceof test now require a function.
L6, L7, L8 - three comments corrected against what the code does: preTapTree is
the last read that SUCCEEDED and can predate the tap by a failed read's whole
timeout; the plain-type early exit deliberately does not fire on a tree that
flags focus nowhere, so that shape still pays the full wait; and no flow report
is written to disk - the reason travels to stdout and the agent's context,
which is reason enough to name no element text.
|
||
|
|
9bc4c036f1 |
fix(flow): an editing host's content is text, never another field
focusedOwnsTargetText asked only that the focused node carry no text of its own and that the target's text be all it shows. A focused non-editable wrapper around ONE unidentified field satisfies that exactly: the field's value is its accessible name, an anonymous field does not shield, so the value hoists into the wrapper and the wrapper has no text of its own either. The docblock's android.webkit.WebView argument only rules out a WebView carrying a whole form; one wrapping a single field - hosted payment fields are one input per iframe - went through. Reproduced on Chrome 151: a <div tabindex=0> focus-trapped around an unidentified <input value="hello-target"> CONFIRMED the gate, and only the keyboard tool's "the focused element DIV#wrap is not a text field" stopped the clear. iOS and Android dispatch a clear at whatever holds focus, so nothing catches it there. The discriminator the trees do carry is that an editing host's content is static: a paragraph or a text node, never a control. The trapped wrapper now refuses in the flow itself, with the message that already exists, while a contenteditable holding <p>hello-target</p> still clears and is rewritten - both verified live. Where a source reports no controls at all (iOS never sets clickable) the residual stands, and the docblock now says so. |
||
|
|
dc5e7937d3 |
docs(flow): a field's contents are shielded by its id, not by being a field
The new asserting-field-values reference told authors that a field's contents
never hoist into a container, so a container text assert reads the page and not
the draft in the box. That holds only for a field carrying an id/testID - the
id is what shields. An unlabelled field with no id puts its contents in the
node's accessible name, and the name hoists.
Measured on Chrome 151 through flow-execute, against unidentified <textarea>s:
`text: { in: <chat>, contains: "unsent draft" }` PASSED with the message list
empty, and `tap: { on: { text: "Save" } }` focused a note holding the word
"Save" instead of the Save button. Adding an id to the composer flips the same
assert to a fail. An author following the old sentence writes a clear-proof
that passes on the value it exists to catch.
The walker's own comment made the same claim for the three harms it lists.
Only the hoist depends on the identifier; a selector match and the resolver's
exact-field ranking read the label directly, so an unidentified field leaks
through all three whatever the walker does.
|
||
|
|
faeee64598 |
fix(flow): confirm a container's clear only when it holds ONE control
The inert-chrome arm asked whether anything under the tap point was a control. underTap holds only what COVERS that point, so any gap between two children - a flex gap, space-between, a margin, a divider the adapter does not emit - leaves the container's centre covering nothing and the every() passes vacuously. The clear then landed on whichever sibling already held focus, chosen by layout arithmetic rather than by the gesture. Reproduced on Chrome 151 through flow-execute: a row of two inputs with a 30px gap, its centre falling between them and the LEFT one focused before the step, passed and rewrote that left input while the report named only the row. Closing the gap sends the same clear to the input under the tap, which is correct. So the arm now requires what its own commit title and references/flow-yaml.md already promised: exactly one control inside the container, and that control is the one reporting focus. The target itself is excluded from the count, which is what the comment claimed and the old sentinel did not do - a clickable container satisfied it on its own. Verified live: the gapped row refuses with both fields intact, the ungapped row still clears the input the tap hit, and a tall label holding a container's centre over its one input still passes. |
||
|
|
3385468cbd |
fix(flow): size the containment test exactly, so no pad hides inside its slack
frameNoLargerThan carried the same per-dimension slack frameWithin applies per edge, so a focused node padded symmetrically around the target by half the threshold satisfied both and took the clear. On a 900px window that is a trap one pixel larger on every edge: reproduced on Chrome 151, where a focusin trap holding DRAFT-do-not-erase over an <input> passed the step, was emptied and rewritten with the new address, and left the named input holding its old value. There is no tolerance that fixes it. A trap enclosing the field by 1px and a child overhanging its wrapper by 1px are the same geometry, and the flow trees carry no ancestry to tell them apart - flattenHoisting returns leaves under one root on every platform. So the size test is exact: equal frames still confirm (the everyday wrapper-around-input shape, verified live), and any overhang refuses, which fails the step loudly instead of emptying a field the step never named. The overhang sweep moves with the contract: pad 0 is now the admitting case and 1-3px refuse, where the 1-2px arm previously pinned the tolerance this defect lived in. |
||
|
|
0347391fff |
fix(flow): bound the focus read, and date the dark tail from when it started
A tree source goes down by hanging, not by throwing fast. The dark-tail guard counted consecutive FAILED polls, and the counter can only reach two when the failures come back cheaply: every transport's own timeout (15s android getHierarchy, 10s per CDP call, 5s per ViewInspector RPC) outlasts the whole 3000ms focus window, so one hang ended the window with the counter at one. The verdict fell through to "unobservable" - the one non-confirmed outcome runType dispatches a destructive clear on. Reproduced on Chrome 151 through flow-execute: a page that blocks its own main thread for 12s from the focusing tap's mousedown took the step from a refusal to a pass that emptied a field the step never named, and the step ran 13.9s while its report claimed a 3000ms window. The same page blocking 200ms refused. So the read is bounded by what is left of the window, the way waitForIdle already bounds its own, and darkness is measured in elapsed milliseconds from the moment the first unanswered read STARTED as well as in polls. The repro now refuses with the outage message and finishes in 3.8s; the 200ms control is unchanged and an ordinary clear still passes. The two comments justifying a poll count with the cost of a `uiautomator dump` went with it: the flow tree never uses uiautomator - fetchFlowTree routes Android to the android-devtools getHierarchy RPC. |
||
|
|
b5bf4134c2 |
docs: correct four claims the code, the platforms or this PR's own fixes disprove
- `flow-yaml.md` described two confirm paths for the clear; there are three, and it named none of the conditions the fixes in this PR added. It now states all three — the element you named, the input the tap landed on (or the container's only control, where the tap landed on its own chrome), and the editing host whose whole content is what you named — plus the naming-is-not-identity refusal, and the narrowed "nothing focused" residual: only a window in which NO read saw focus still clears. - `flow-actions.ts` named only Android and iOS as flattening to leaves, which reads as exhaustive and implies Chromium keeps ancestry. It does not — `flow-chromium-tree` runs the same `flattenHoisting` — and every reproduction cited in that same docblock comes from Chrome, where the agent-facing `describe` DOES print a nested tree, so a reader is invited to conclude the sharper structural test was available there. - Two comments in this PR gave opposite answers about the iframe descent, and `chromium.ts`'s was the wrong one: it justified excluding a host <iframe> on the grounds that the inner element carries the flag instead. The descent is dead — `el instanceof Element` rejects the inner `documentElement`, whose constructor belongs to the OUTER realm — so focus in a subdocument reaches the tree from neither side. Verified on Chrome 151: an `<iframe srcdoc>` with a unique marker leaves no trace in describe, and focusing an input inside it leaves the tree with no focus flag anywhere. The exclusion is still right; its stated reason was not available. - "Android and Chromium splice at the caret, iOS appends" appeared in four places with nothing behind it — no test, constant or recorded measurement, and `grep -rn caret` over `src/` returns only prose. What is measurable is the mechanism, and it is not per-platform: the caret lands where the focusing tap put it, so a value long enough to reach the field's middle splices (measured again here on Chrome 151 — forty `A`s typed with `ZZZ` came back `AAA…ZZZ…AAA`) and a shorter one appends because the tap fell past its end. All four sites now say that instead. `asserting-field-values.md`'s two DOCS findings — the "always exposes its live contents" claim and the undermined "assert the consequence instead" advice — went with the textarea fix, which also removed the `equals`-vs-`contains` trap the Chromium bullet gave no guidance for. |
||
|
|
2b77ccf42a |
test: close the gaps the review's mutation sweep left open
Every pin below was mutation-checked: the pre-fix code was restored in `flow-actions.ts` / `chromium.ts` and each named test went red. - `sameElement`'s LABEL fallback had no coverage, and it is the whole name-is-not-an-identity defect. Every fixture in the runner file gives its nodes a resource-id, so the anonymous path — the everyday React Native and plain-DOM shape — never ran. `anonymousRowXml` runs it both ways: a namesake and a differently named sibling. - `trackTarget`'s proximity tie-break had none either. `twoSameIdRowsXml` is the re-layout that made it adopt the wrong namesake. - `focusedOwnsTargetText` needed the trap whose OWN text equals the target's; the existing editor fixture only ever exercised the arm that should confirm. - `requireEvidence`'s safety half was unpinned AND the test written for it did not reach it: `overlayFocusXml` carries no `email-wrapper`, so every overlay read was consumed by selector resolution before the tap went out and the focus poll's first read already confirmed. `overlayOverWrapperXml` carries the wrapper, so the clear really has to poll past an overlapping read. - The textarea change had no coverage past the walker itself. Four cases now run the real walker THROUGH the real flow adapter and matcher: the container hoist, the page-wide `visible` check, the tap ranking, and the clear-only proof that still has to work on an unlabelled composer — plus an `<input>`-vs-`<textarea>` spelling case. - `describe-chromium-script.test.ts` certified behaviour that cannot happen: it asserted focus inside a same-origin iframe is carried by the inner element. The walker never descends — `el instanceof Element` rejects the inner `documentElement`, whose constructor belongs to the OUTER realm — and the mock passed only because it is single-realm. Verified live on Chrome 151: an `<iframe srcdoc>` with a unique marker leaves no trace in describe, and focusing the inner input leaves the tree with NO focus flag at all. That is the "unobservable" state a destructive clear dispatches on, so the test now pins it as such. - The refusal tests ran at ~75% of vitest's 5000ms default (measured 3757-3760ms) on the one file whose names would make a load-induced timeout look like a real destructive-clear regression. They get their own testTimeout. |
||
|
|
991b228256 |
fix: four LOW findings around the clear's focus wait and the shadow-root read
- A page that removes `Element.prototype.shadowRoot` took describe down entirely. `protoGetter` falls back to a direct property read when the descriptor is absent — this file's own documented threat model — so a `<form>` holding `<input name="shadowRoot">` hands the CONTROL element to the new ShadowRoot `activeElement` accessor, and Blink throws `Illegal invocation` for the whole page. Verified live: `CHROMIUM_DESCRIBE_FAILED` for every element until the descriptor was restored, and clean with the `isShadowRoot` guard the same function already applies to `root` two lines down. The base could not throw here — it only ever read `shadow.children`. The mock ShadowRoot's accessor is brand-checked now, so a test can see this at all. - A focused node whose frame EQUALS the target's was classified as "encloses". `frameWithin`'s slack is per-edge, so identical frames satisfy it, and the refusal then tells the author to point the selector at the input itself when it already does. `frameNoLargerThan` — added by this PR for exactly this distinction — is now consulted here as it is in `focusedOwnsTargetText`. - The focus wait could overrun its stated budget by a whole tree read. The deadline was checked only AFTER `fetchFlowTree`, while the sleep is capped to land exactly on it, so one more read always started past it — 1-2s of a `uiautomator dump` beyond the 3000ms every refusal message claims the window was. - The `FlowStep` union admitted a `type` step with neither `text` nor `clear`: it serializes to YAML the parser rejects, labels itself `⇐ (clear only)` in a recording summary, and dispatches no keyboard call at all — a silently passing no-op. Pairing the two fields in the union keeps the parser the only thing that can build one. The remaining LOW findings need no change: a labelled `<textarea>`'s 200-char truncation and its live contents feeding the settle fingerprint both went with the textarea fix, which stops the contents becoming node text at all. |
||
|
|
645771dd55 |
fix(describe): spell a <textarea>'s contents the way the field holds them
`accessibleName` whitespace-normalized a `<textarea>`'s value where the `<input>` path deliberately did not, so the same typed text was assertable back in one element type and not the other: `equals: " hello "` matched at the base and stopped matching, and a two-line value came back spelled as one line while the identical string in an `<input>` came back raw. Nothing warned an author which spelling applied to which element. The normalization was there because the walker also emitted the value as node text, and two spellings of one value cannot both be quoted by a failing assert. The contents now reach the tree as exactly one string, so that string is the one the field holds. Verified live: an `equals` on `" hello "` passes against a textarea and an input alike. |
||
|
|
e4eacd3847 |
fix(flow): let a tap that lands on inert chrome confirm the container's one control
`tapCandidates` probes exactly one point — the container's geometric centre — and a container is not laid out to put its input there. When the selector names a wrapper (a testID container, the documented and supported shape) and the tap on it legitimately moves focus to the input inside, the clear was refused unless the input happened to sit under that one point. Nothing else failed: the tap worked, the right element ended up focused, and the same step without `clear` succeeded. The refusal then misdirected, blaming an overlay or a sibling row, neither of which existed. Measured on Chrome 151 against this branch's tool-server, sweeping only the label's height on one page (the input is ~34px tall): 16/24/32px pass, 40/48/60px fail. It flips exactly where the label grows taller than the input, i.e. where the container's centre crosses out of it — so a two-line label, a label with vertical padding, or a label plus helper text is enough. The React Native `Pressable`-wrapping-a-`TextInput` shape does it with no label at all. So when nothing under the tap point could have taken the keys itself, the app routing focus to the container's one control is the honest consequence of the gesture. A control under the point is the whole difference from the row this gate refuses — there the tap DID reach something that takes focus. Verified live: the 60px label and the RN wrapper both clear now, while the two-input row still refuses whether its inputs share a name or not. The relaxation needs positive evidence that the source reports controls at all: the iOS full-hierarchy adapter never sets `clickable`, so "nothing under the point is a control" would be a statement about the adapter there and would wave every sibling through. Without that evidence this stays the refusal it is today. |
||
|
|
c46eb09844 |
fix(flow): bound the focus wait's dark tail, instead of failing open on it
`lastRead` is only ever written by a read that SUCCEEDED, so the "unreadable" outcome fired only when EVERY read in the window threw. One successful poll disarmed it permanently — including the first, which is the one most likely to report "no focus yet", because the app's focus round-trip has not finished. That maps to "unobservable", the one non-confirmed outcome `runType` deliberately lets a destructive clear through on. A transient tree-source outage in the tail of the window therefore converted a correct refusal into a blind clear. Reproduced on Chrome 151 against this branch's tool-server: nothing focused at the first poll, the tree read poisoned from t+600ms, focus landing on `#other` at t+900ms where no read could see it. The step passed and wrote `WIPED` into `#other`, a field it never named; the same page with the reads left alone refused, and so did the same page poisoned before the first read. The file already states the rule 700 lines up — `CONDITION_DARK_TAIL_TOLERANCE_MS`, "a verdict narrated from the reads before the darkness would describe a screen nobody saw at the deadline". This applies it to the one place where that stale verdict authorises a destructive action, counted in consecutive failed polls rather than in milliseconds: one poll costs a whole `uiautomator dump` on Android, so an elapsed-time budget would read a slow device as an outage. One failure is still the last-poll blip the tolerance exists to absorb. The refusal now says the tree stopped answering rather than that it could not be read at all, which was only ever true of the every-read-failed case. |
||
|
|
9743fbfcaf |
fix(flow): stop a read that cannot SEE focus erasing one that found it
The focus wait classified an empty `focused[]` as the observation "no-focus", which `giveUp` maps to "unobservable" — the one non-confirmed outcome `runType` lets a destructive clear through on. `lastRead` is latest-read-wins with no floor, so a single read in which the focused element's leaf is simply MISSING erased everything the earlier reads had established. Nothing throws; the tree is complete and still holds the target. Both flow adapters produce exactly that for a frame clipping to zero area, which is what keyboard avoidance does to a field it scrolls away, and on Android what a healthy `uiautomator` dump reports for a focused view whose bounds have collapsed. Reproduced on Chrome 151 against this branch's tool-server: `#other` holds the caret and never loses it; the tap on the target does not move focus; collapsing `#other`'s box 1500ms into the wait took the step from "focus never reached" to a pass that emptied `#other` and wrote `WIPED` into it — with `#other` still `document.activeElement` at the end. The same page without the collapse refuses. So a read that saw nothing no longer overwrites one that saw where focus is. Latest-read-wins still decides between sightings, which is what keeps a blur on the focusing tap from being answered with history, and a window in which NO read ever saw focus is still "unobservable" — verified live that a page with nothing focused still dispatches the clear, which is what keeps the directive working on an iOS build whose injected framework omits `firstResponder`. The residual `runType` documents narrows accordingly: an app that blurs, or a field that scrolled away, now only reads as unobservable when it happened before the first read. |
||
|
|
85fa54a06a |
fix(describe): stop a <textarea>'s draft reading as text the page displays
The walker gave a `<textarea>` its live `el.value` as the node's own text. That
fixed the real problem it was aimed at — child text is the markup DEFAULT and
never tracks the value — but node text is what the page DISPLAYS: it hoists into
an ancestor's `subtreeText`, a `text` selector matches it, and the resolver
ranks an exact field match on it above a substring hit. A draft sitting in a
composer therefore behaved like text the page was showing.
Measured on Chrome 151 against this branch's tool-server, each against the same
page with only this hunk changed:
- `assert: { text: { in: { id: chat }, contains: "hello team" } }` PASSED with
the message list empty and the draft still in the box, so a saved regression
test written that way can never go red;
- `assert: { visible: "Alpha" }` PASSED on the composer holding "Alpha" — the
very "assert the consequence instead" pattern the new reference prescribes;
- `tap: { text: "Save" }` landed in a note whose draft was the word "Save",
not on the Save button, whose label is the only "Save" on the page.
A field's contents are not the page's text and there is no third channel to tell
them apart in, so a `<textarea>` now reads its contents back exactly as an
`<input>` does: through `accessibleName`'s value fallback, which an aria-label
or a placeholder pre-empts. All three repros are refusals again, a `tap` lands
on the button, and the clear-only proof still works on an unlabelled composer
(`hidden` fails before the clear, passes after).
Two MEDIUMs fall out with it: a >200-character value can no longer be reported
twice (the dedup compared an unsliced text to an already-sliced name), and a
labelled textarea's contents no longer reach a failing assert's quoted text, so
a secret typed into one stays out of the flow report.
|
||
|
|
a82655c988 |
fix(flow): admit only a node with no text of its OWN as the target's editing host
`focusedOwnsTargetText` let any enclosing focused node confirm a destructive clear as soon as its text equalled the target's. That re-opened the "encloses" shape the same wait refuses: a `focusin` focus trap holding a `<textarea>` over the field confirms whenever its draft happens to equal the field's value. Reproduced on Chrome 151 against this branch's tool-server — the step passed, the TEXTAREA was emptied and rewrote with `new@example.com`, and the `#email` the step named kept its old value throughout. The same page with any other draft in the textarea refused. The coincidence is structural, not lucky: an anonymous target does not shield, so its text hoists into every enclosing node's `subtreeText`. An editing host is distinguishable without ancestry, which the flattened flow trees do not carry: it has no text of its OWN. Quill/ProseMirror/Lexical render an editable with no id, no accessible name and no own text, so the only text on it is the target's, hoisted. A trapped textarea fails that — an unlabelled one's value IS its accessible name — and so does a focused WebView carrying a whole form's text. Verified both ways on the live build: the composer the arm was written for still clears and replaces, the trap no longer can. |
||
|
|
4d108e75ee |
fix(flow): stop an ambiguous name re-identifying the target as a different element
`trackTarget` follows the element the step tapped rather than re-running the selector — the right instinct — but "the same element" is `sameElement`, a name test. When several nodes answered to the name it fell through to "nearest to where the tap landed", and any re-layout between the tap and the focus poll can put a different namesake nearest. Keyboard avoidance, the everyday movement this function exists to follow, is exactly such a re-layout. The identity arm in `waitForFocus` then trusts that node absolutely, bypassing `focusedFromInside` and the under-the-tap gate entirely. Reproduced on Chrome 151 against this branch's tool-server: two rows sharing a `data-testid`, the selector resolving to the row holding `KEEP-ME`, and a 100px scroll during the focus wait moving the second row to where the first had been. The step passed and emptied the row holding `SECRET-B` — a field the selector never resolved to. The same page with distinct testids refused. An ambiguous name now yields no target node at all, not even the selector's own pick: that ranks by frame and reading order, so it would hand back one of the same namesakes on a tie-break. With no target there is nothing to confirm against, which is a refusal for a `clear` and the best-effort path a plain `type` was already on. |
||
|
|
717b475c21 |
fix(flow): require the focused node's name to identify ONE element under the tap
`focusedFromInside` is the only path by which a node that is not the target can confirm a destructive clear, and its whole discriminator was "is this focused node one of the elements the tap could have hit". That question was answered by `sameElement`, which compares role + identifier and falls back to the label — names, not identities. Two elements sharing a name were therefore interchangeable, so the focused node only had to be a NAMESAKE of whatever sat under the tap. That is the currency/amount row and the OTP row the gate's own docblock says it refuses. Reproduced on Chrome 151 against this branch's tool-server: a row of two anonymous inputs both reading "Amount", the tap landing on the right one and the app forcing focus to the left one, cleared and rewrote the LEFT field and reported a pass on the row; the identical page with the second input named "Currency" refused. On Android it needs no coincidence of labels — `identifier` is the raw resource-id, which names the layout slot. So the name must resolve, in the tree the tap was dispatched from, to exactly one candidate inside the tapped box, and that candidate is the one that must have been under the tap. `nodesUnderTap` becomes `tapCandidates` and returns both lists from one walk, since the ambiguity scope and the hit test are two questions about the same set. |
||
|
|
591c8ea4fd |
docs: correct nine claims this PR's own code or platforms disprove
Comments and skill pages that describe something other than what happens. No
behaviour change.
- `live-authoring.md` told an agent to KEEP a recorded `tool: keyboard` clear
raw, on the false premise that `type:` accepts only `into`/`text`/`submit`
(`flow-utils.ts` allowlists `clear`) and pointing at a table exception this PR
deleted. It is the raw form that is unsafe: it replays with no focus tap and
no focus check. Converting it is the whole mitigation, and that paragraph
said not to.
- The same page claimed an unconfirmed poll falls through to typing. It does
for a plain `type`; a `clear` hard-fails, which is what `flow-yaml.md`
already says.
- `dispatchOrAbort`'s docstring said `keyboard` discards the `ToolContext`. All
three platforms `clear` supports forward the signal and check it between
keypresses, so an abort-driven rejection is the designed common case there —
which is what makes the wrapper load-bearing rather than defensive.
- The `requireEvidence` fast path was justified by uiautomator flagging a
WebView instead of the EditText inside it. Surveyed across eight screens on
API 36 / WebView 151, including a real in-app WebView, the WebView is
`focused="false"` and the leaf EditText carries the flag. The exit stays; its
stated reason does not.
- `FOCUS_REPORTING_SOURCES` said a source outside the set never reports focus,
naming Vega. Vega's source-parser does set it and `flow-vega-tree` preserves
it; it is excluded because `runDirective` refuses `type` there first.
- The Android TV comment had a `clear` step failing at the first keyboard
dispatch, which presupposes the focus gate passed. `android-devtools` is in
the reporting set, so the wait reports "unconfirmed" and the refusal returns
before any dispatch — as `flow-yaml.md` says.
- `flow-run.ts` cited `(down)` as a sibling option echo twenty lines above the
branch that suppresses exactly that value.
- `asserting-field-values.md` described the textarea case as either/or, gave an
over-broad rule for when an `<input>` exposes its value (`accessibleName`
gates on truthy/resolving, so `aria-label=""` still falls through), and
recommended `assert: { hidden: … }` as working on every platform when it
passes vacuously on exactly the platforms where the contents never reach the
tree. It now also states the secret exposure a `<textarea>` carries.
- `flow-keyboard-clear.test.ts`'s header still gave the allowlist as the reason
the round trip is worth pinning. The round trip is; that reason is not.
|
||
|
|
d1e28a412d |
test(flow): pin the shapes the static fixtures could not express
Every focus fixture held the tree constant across polls and split its rows unevenly, which is what let five defects through a suite that was otherwise green. Each test below fails against the code as it stood one commit before its fix, and each ships with the control that keeps the fix from being a blanket refusal. - an EVENLY split row whose focused half is not the one the tap hit refuses, and the same row with focus on the half the tap DID hit still clears; - a focused input overhanging its wrapper by 1-2px still clears, and 3px — six pixels of extra width on a 1080px screen — refuses, which pins both sides of the containment epsilon instead of only the far one; - an overlay appearing over the tap point AFTER the tap refuses, and the same overlay already there at tap time still clears; - a named container that GROWS between the tap and the poll still clears; - a selector that re-resolves to a better-ranked node after the tap still clears; - a rich-text composer whose only text sits in a child node clears, and the same editor holding a second paragraph refuses; - a `<textarea>` holding a newline reports its contents once, not once raw and once normalized. Also pinned, all three of which a mutation survived: a `clear` keeps polling past an intermediate overlap read instead of refusing on it (the safety half of `requireEvidence`, whose perf half was already covered), the focus flag is suppressed on a shadow host when the flag sits NESTED inside its subtree rather than one level down, and no refusal reason quotes the focused element's own text — the invariant that keeps a field's value out of a report on disk, until now asserted only in a comment. |
||
|
|
acfce47362 |
fix(describe): stop a multi-line textarea reporting its contents twice
The walker gives a `<textarea>`'s node text its live value, whitespace- normalized. `accessibleName` already fell through to `el.value` for an unlabelled textarea, but returned it RAW. For any value holding a newline or a run of spaces the two strings differ, so the walker emitted both — the raw value as the node's label and the normalized value as its value — and the flow's text matching concatenated them. An `equals` assert on the field's real contents then matched neither spelling, which is the assert the skill prescribes as the proof a clear landed. Normalize the textarea fall-through the same way, so the two agree and only one is emitted. Reproduced and re-run on Chrome 42: `line one\nline two` in a `<textarea>` rendered as `textarea "line one\nline two" value="line one line two"` and the prescribed assert failed quoting the contents twice; it now renders once and the assert passes. The single-line control passed before and still does. |
||
|
|
8087e6e6a7 |
fix(flow): let a rich-text composer's own content name it for a clear
Quill, ProseMirror and Lexical render `<div contenteditable><p>…</p></div>`. The editable node carries no id, no accessible name and no own text, so the only text in the tree belongs to the block child and every selector that can name the editor's content resolves to something INSIDE the focused node. That classified as "encloses" and was refused with advice — point the selector at the input itself — that nothing on the page can satisfy. Confirm when EVERY focus flag sits on an element whose entire content is what the selector named. Equality is what keeps the shapes "encloses" exists for out of it: a focused `android.webkit.WebView` wrapping a form, and a focus trap holding a `<textarea>` over the field, both show text of their own beyond the target's. Tree ancestry would ask this more directly, but the flow adapters flatten to leaves under one root, so the hoisted `subtreeText` is what survives. Reproduced and re-run on Chrome 42: the Quill shape and the Lexical shape (a `<span>` inside the `<p>`) both went fail + editor untouched, and now pass with the editor replaced. Both negative controls still refuse with the field intact — a focus trap that keeps focus on a `<textarea>` covering the target, and an editor holding a second paragraph the selector did not name. |
||
|
|
e4d001e650 |
fix(flow): follow the element the step tapped, don't re-rank the selector
`waitForFocus` re-resolved the selector from scratch on every post-tap read.
The `tappedFrame` fallback covered a round where the selector failed to resolve
at all, but not one where it resolved to a DIFFERENT, better-ranked element
than the one that was tapped. Both confirming arms then tested focus against an
element the step never touched, so a clear was refused with a message saying
focus never reached the target while focus was exactly where the flow put it.
The trigger is ordinary: a field that opens a typeahead list on focus where a
suggestion repeats the field's current value. The chip is smaller than the
input and `selectorToNode` ranks smallest-frame-wins, so the chip takes the
match.
`trackTarget` follows the tapped element by name instead, keeping the selector
as the fallback for a round it cannot be found in, so a field that keyboard
avoidance scrolls away is still recognised where it now is.
Reproduced and re-run on Chrome 42: `into: { text: "Paris" }` with a suggestion
reading "Paris" went fail + field untouched, and now passes with the field
replaced. The control that changes only the suggestion's text still passes, and
so does the same step without `clear`.
|
||
|
|
729e7845ec |
fix(flow): settle where the tap landed once, instead of re-deriving it per poll
`focusedFromInside` recomputed the tap point from the target's CURRENT frame every round, on the argument that keyboard avoidance cannot then stale it. That is true of a container that MOVES and false of one that GROWS. An autocomplete wrapper rendering its listbox inside itself on focus gets taller after the tap, so the recomputed centre leaves the input and lands in the option list, and the clear is refused with a reason naming an overlay or a sibling row — neither of which is on the page. Downshift, HeadlessUI and most React Native autocompletes render the list inside the wrapper, and the wrapper is where a testID sits. The pre-tap record added for the overlay fix already answers the question the recompute was asking, and answers it against the tree the gesture was actually dispatched from, so drop the per-round geometry and keep the identity match. A field that keyboard avoidance scrolls away still matches itself, because `sameElement` follows the element rather than its box. Reproduced and re-run on Chrome 42: a combobox wrapper whose listbox opens on focus went fail + `#city-input` untouched, and now passes with the field replaced. The control with the listbox handler removed still passes, and the whole seam/overlay battery from the two previous commits is unchanged. Not fixed by this, and distinct: a wrapper that is ALREADY more than twice the field's height before the tap (a two-line validation message under a 40px input) makes the focusing tap miss the input itself, so nothing takes focus and the keyboard tool refuses the clear. That is wrapper targeting, not focus confirmation, and it predates `clear`. |
||
|
|
21def28210 |
fix(flow): confirm a clear only from an element the tap could have hit
`focusedFromInside` accepted any focus-flagged node covering the tap point, and the comment justifying it argued that such a node IS what the tap hit. That holds only for an overlay already on screen when the gesture was dispatched. An overlay rendered in RESPONSE to the tap — an @-mention list, an inline picker, a formatting bar with its own input — was hit by nothing, and taking focus was enough to make it swallow the clear while the composer kept its draft and the step reported a pass on the composer. Keep the settled tree the frame was resolved from, and match a focus-flagged node against the elements that were under the tap point in it (`nodesUnderTap`, `sameElement`). Matching by role + identifier rather than by frame is what lets the field move — keyboard avoidance scrolling it away from where it was tapped — without losing its identity across reads. Reproduced and re-run on Chrome 42 through the branch tool-server: a popover opened by the composer's own `focus` handler went pass + `#mention` destroyed, and now fails with both fields intact. The two controls still pass — the same page with no popover clears the composer, and the same popover ALREADY open and holding focus at tap time still takes the clear, which is the case the old comment was right about. |
||
|
|
e727a13feb |
fix(flow): read the seam between two children the way the OS hit test does
`focusedFromInside` confirmed a clear when the focus-flagged node covered the point the tap landed on, and asked `frameContains`, which is inclusive on all four edges. A container split evenly between two children puts its centre ON the seam, so BOTH halves contain it and the test discriminated nothing: the clear emptied and overwrote whichever half held focus, and the step reported a pass on the container. The OS routes the tap itself to exactly one child — left/top inclusive, right/bottom exclusive in `Rect.contains`, `CGRectContainsPoint` and Chrome's `elementFromPoint` alike — so ask the same question with `frameCoversTap`. Reproduced and re-run on Chrome 42 through the branch tool-server, one variable at a time: a 50/50 row whose LEFT input takes focus via a `focusin` redirect went pass + `#first` destroyed, and now fails with both fields intact; the same row with no redirect still passes and still types into the tapped half; the same page at 30/70 still refuses; a wrapper over a single input still passes. |
||
|
|
a0d1ebe0e4 |
docs(flow): the empty-state assert a clear-only step was justified by does not exist
"empty the box, then assert the empty state" is the reason `text` became
optional, and it is the one shape the flow language cannot express. Both
spellings fail, on opposite sides:
- `equals: ""` / `contains: ""` are rejected at parse time - "assert
text needs a non-empty `equals`".
- `matches: '^$'` parses and then cannot match: `regexMatchesNonEmpty`
treats absent or empty text as a non-haystack, so the pattern is
false for exactly the state it describes.
Measured on Chrome 151 through the branch tool-server, `<input
id="plain-input" value="OLD PLAIN">` emptied by the directive: the assert
fails reporting `its text was ""`, with the element's value genuinely "".
What does work on the same page is asserting the OLD value's absence -
`assert: { hidden: "OLD PLAIN" }` passes - so a clear-only step IS
verifiable, just not the way the comment or the skill said. Both now say
the spelling that works.
`references/asserting-field-values.md` also gets the Chromium bullet
corrected for the textarea fix earlier in this branch: a `<textarea>` now
always exposes its live contents, so the label-only limitation is an
`<input>` one.
|