Repository navigation
fix(ios): mark zero-area nodes unmeasured and point their press refusal at scroll --until - #4495
Conversation
…al at scroll --until A scroll view can keep its scrolled-out content in the tree with a zero-area frame (Flutter does on iOS). The Simulator AX bridge fold kept such a node as if it were on screen, and pressing one was refused with no recovery. The fold now marks such a node [unmeasured]. A target with no usable frame is still refused; when a scrollable sibling is listed before it, which is how Flutter publishes a scroll view and its content, the refusal names scroll --until with no direction, since the frame gives none. Fixes callstack#4451
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 6 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/capture-kit/src/ios-snapshot-engine/geometry.ts">
<violation number="1" location="packages/capture-kit/src/ios-snapshot-engine/geometry.ts:154">
P2: This fold runs for every acquired iOS producer, so Appium and Revyl trees also label child nodes with unusable frames as `[unmeasured]`, although this behavior is documented as Simulator AX-specific. Pass producer provenance into the fold and apply the hint only for `simulator-ax-bridge`.</violation>
</file>
Reply to a comment to ask cubic a question or push back. It learns from your replies.
View guided diff | Re-trigger cubic
…n the unmeasured recovery The hint serializes the selector as a double-quoted argument, so one holding an apostrophe survives. The ref form names snapshot -i, as the off-screen ref hint does. A scroll bar listed before the target is no longer read as its scroll container. The help and docs say where the mark is added: every iOS snapshot presented from an acquired tree, not only the Simulator AX bridge.
There was a problem hiding this comment.
All reported issues were addressed across 4 files (changes from recent commits).
Reply with feedback, questions, or to request a fix.
View guided diff | Re-trigger cubic
…he unmeasured mark to regular snapshots The recovery hint wrapped the selector with JSON.stringify, and inside double quotes a shell still expands $, backticks and !. It now uses a POSIX single-quote wrap. The help and snapshots.md say the [unmeasured] mark belongs to regular snapshots: --raw returns before the fold.
|
Reviewed 2cc05f3. I found no blocking code problems, but the evidence is incomplete: the Appium and Revyl provider routes were not run, and nobody measured how many zero-size nodes native iOS apps publish. I ran no tests and no device. The "base 2 failed, head 23 passed" count and the Flutter before/after output are the author's and I did not reproduce them. The ownership question comes first. The mark is applied only in the TypeScript fold (https://github.com/callstack/agent-device/blob/2cc05f3/packages/capture-kit/src/ios-snapshot-engine/geometry.ts#L171), so it reaches the bridge and provider routes. Simulators and physical devices use XCTest through the Swift presenter, which has no equivalent, and the differential harness omits presentationHints, so this gap passes silently. Would it be simpler to add 'unmeasured' to presentationHints in the producer-neutral pass in buildSnapshotState, next to annotateCoveredSnapshotNodes? That would cover XCTest, the bridge and both providers with no Swift change, and the provider-provenance question in the open thread goes away. Android and macOS zero-rect nodes would get the mark too, which looks correct. Either the mark is part of the shared Swift/TS presentation contract (change both engines and the conformance fixture) or it is a producer-neutral annotation (move it); the PR currently picks neither. Not blocking, take or leave: the press recovery builds its own shell quoting, so please use shellQuote from kernel instead. The scroll hint is built twice with different wording and detail keys (throwIfOffscreenInteractionTarget and scrollContainerRecovery), and scrollContainerRef keeps the '@' while details.ref is bare by convention. The mark fires for every kept non-root node without a positive finite rect, so a before/after full snapshot of Settings on the bridge route with the count of newly marked nodes would show the noise level; if non-interactive nodes are marked, limit it to nodes with semantic content or interactive nodes. unmeasuredHint re-derives "no frame" with isPositiveFiniteRect, while traversalDecision already computes hasFrame, so carrying unmeasured on GeometryDecision would remove the duplicate if the mark stays in the fold. On the open review threads: #4495 (comment) (provider provenance) is moot because the docs now match the code, and it ends if the mark moves. #4495 (comment) (hint overwrite) does not apply because nothing upstream sets hints on iOS nodes, and #4495 (comment) (scroll indicator) is fixed at this head. Please resolve all three. CI is green with 1 check and none failing, and I know of no conflicts. The local check:affected unit failure in daemon-runtime-composition.test.ts is not reproduced in CI, and this diff touches no daemon file. Before merge, answer the ownership question and switch to shellQuote. |
…and quote scroll hints with shellQuote The unmeasured mark moves from the iOS acquired-tree fold to buildSnapshotState, next to the covered annotation, so every producer gets it: the XCTest runner, the Simulator AX bridge, the provider trees, Android and desktop. Regular snapshots only, as before. The zero-area recovery takes its selector already quoted from the command-runtime caller, which uses the kernel's shellQuote; the selectors entry cannot import it without growing its eager closure. The off-screen hint uses the same helper, so a selector with an apostrophe no longer breaks its command. scrollContainerRef is the bare ref body, like details.ref.
|
Pushed 2addea4. Ownership: moved, as you proposed. Noise, measured at this head (full
The 3 on the repro are the scrolled-out content. The 12 on SpringBoard are all One limit of reading the published rect: both iOS presenters publish a zero-size rect for an application or window they keep while it is fully clipped, so such a node would get the mark too. None appeared in these captures. shellQuote: the recovery now uses the kernel's Also taken:
Not run: the Appium and Revyl routes (no backend here), physical devices, macOS and Linux. Local |
There was a problem hiding this comment.
All reported issues were addressed across 11 files (changes from recent commits).
Reply with feedback, questions, or to request a fix.
View guided diff | Re-trigger cubic
…it from ref targets The zero-area recovery quotes its selector with shellQuoteIfNeeded, as the moving-target gate does, so a bare selector stays bare. The ref arm of the failure type forbids shellQuotedSelector, so a ref target cannot be handed the selector form of the hint.
|
I reviewed a91a366 against the code and found no remaining problems. Both earlier review threads are fixed at this head: The evidence gap from the earlier review is closed. The simulator before/after output and the noise table are yours, and I checked them against the code only. I ran no tests, gate or device. I did not run the eager-closure budget. I only confirmed that CI is not done. The one reported check passes, but the repo CI jobs (unit, typecheck, layering and closure gates) show action_required for this fork PR and have not run on this head. This diff touches the closure-gated A maintainer must approve the fork CI run, and the checks must pass before merge. One small note: the off-screen hint still uses always-quoting |
thymikee
added
the
ready-for-human
…baseline * origin/main: ci: use smaller runners for control and publishing jobs (#4492) docs(help): point verification at read-only commands, not bare find (#4497) fix(android): let open --relaunch accept a package Android restarts at once (#4498) fix(snapshot): name the captured Android app in the snapshot header (#4499) feat(proxy): configure trusted public upload origins (#4501) fix(android): restore screenshot demo permission state (#4477) feat(harmonyos): support app uninstall beside deploy (#4292) fix(ios): confirm foldable rotation from the lit panel in either simulator set (#4343) (#4440) chore: mark 0.21.37 as released fix(ios): make back the verified left-edge swipe (#4460) fix(ios): mark zero-area nodes unmeasured and point their press refusal at scroll --until (#4495) fix(ios): never repeat a synthesized gesture that may have run (#4474) fix(capture): publish cropped screenshots atomically after scaling (#4478) ci(android): reuse cached system images across emulator workflows (#4490)
Summary
Fixes #4451.
buildSnapshotStatemarks a node published with a zero-area frame as[unmeasured], next to the covered annotation, so every producer gets it. Regular snapshots only;--rawis unchanged.target_bounds_invalid, not dispatched). When a scrollable sibling is listed before such a target, the refusal addsscrollContainerRefand a hint namingscroll <direction> --until <selector>, with no direction.9 files. Scope expansion: the off-screen hint now quotes its selector with
shellQuotetoo; an apostrophe broke its command.Not covered: under a framed scroll ancestor the off-screen stage still refuses first, with
scrollDirection: "up"read from the 0×0 frame.Validation
Base
eec35e9, heada91a366.vitest run snapshot-state.test.ts resolution-touch-point.test.ts target-visibility-stages.test.ts: base 6 failed, 15 passed; head 21 passed.pnpm check:affected --run --head a91a366(exactCommit: true): failed atunit, 1 of 19,494:daemon-runtime-composition.test.ts:86, identical on an isolated base checkout; no daemon file touched. Host cause unproven; CI ownsunit. The runner stops there; by hand,check:doc-proseandcheck:doc-links(Node 24.21.0) passed.press 'label="View all"':@e6 [button] "View all";Next: Read the diagnostics log for the failing step.@e6 [button] "View all" [unmeasured];Next: … run scroll <direction> --until 'label="View all"' …. Following it, the press taps.