fix(android): let a covering sibling hide only what its content covers - #1808
Conversation
Size Report
Startup median (7 runs, lower is better):
Top changed chunks:
|
|
Reviewed exact head 2d3ccdf. P1 remains in the new footprint model: |
Review on #1808: a bounding box of two corner controls spans the viewport, so a transparent overlay with a control in each corner still acquired a full-screen footprint and could prune the app beneath it. Footprints now keep their presented rects apart, and coverage is the overlapped area of the two unions (coordinate-compressed cell sweep). Scrollables count as presenting their box: they consume touches over it, which is what lets a real pushed screen (header, scrollable body, footer) still cover a drawer surface. Adds the disconnected-corner regression.
|
Addressed in e1e706d — thanks, the enclosing rectangle was the wrong primitive.
Re-ran the helper-XML A/B over the Pixel 9 captures against pre-fix: overlay screens still recover everything (45→116/125/111/83), nothing lost anywhere. One new one-node delta on the launcher: the labelled "Google search" search-pill background is no longer condemned by the icons at its ends — a real visible element, so keeping it is correct. Product-detail stack push is unchanged (the pruner is idle there; react-native-screens handles it). Local gates green: format, lint, typecheck, layering, fallow, android unit suites. |
|
Clean re-review at e1e706d: the prior bounding-box blocker is resolved with disconnected footprint rectangles and actual union-overlap coverage; the opposite-corner regression fails the prior model and the updated body now describes the implementation and four fixtures accurately. Live Pixel/helper-XML evidence remains relevant, exact-head CI including Android Smoke is green, and merge state is CLEAN. Ready-for-human. |
|
One more revision (5be8ba0), prompted by fuzzing the pruner rather than by a report. I generated 20 k random 2–3-sibling trees over the attribute space and diffed pre-fix vs fixed, classifying disagreements by direction. The only direction that can create a new sparse-snapshot bug is "new prunes something old kept". That surfaced one real gap: a container whose only painted content is small (a corner icon) but which also carries testID-only markers or labelled containers was condemned as soon as a touch surface covered the icon — markers and container labels aren't paint, so they never entered the footprint. Fix: footprints now carry two sets. After this, new-prunes-more is 0.14 % of random trees with children clipped to parents, and every inspected case is the intended rule (everything the target shows lies under a higher touch/scroll surface). Live Pixel 9 captures unchanged from the previous revision. New unit fixture pins the marker case (fails on the previous commit). PR body updated to describe paints/shows and the fuzz. |
|
New delta at 5be8ba0 is not ready. |
pruneAndroidCoveredSubtrees credited a higher drawing-order sibling with painting its whole box as soon as it had any content anywhere inside it, or a label of its own. A full-screen DoraemonKit drag surface holding one 189px floating icon therefore condemned the entire app subtree, and an empty labelled match_parent placeholder did the same. Occlusion is now spatial. A subtree's footprint is the bounding box of what it presents (agent targets and labelled leaves); a sibling is covered when its footprint lies under a candidate's footprint. A node's own label is no longer paint evidence: a container's content-desc describes its children and an empty labelled View draws nothing. Only a touch target still hides its full box (scrims). Comparing footprint to footprint keeps stacked screens with matching margins registering as covered. Live on a Pixel 9 Pro XL API 37 emulator with a DoKit-shaped overlay added to the test app: snapshot -i went from 2 nodes + the sparse hint to the full app; helper-XML A/B across home/catalog/form/product-detail recovered every label with none lost, and non-overlay screens are byte-identical.
Review on #1808: a bounding box of two corner controls spans the viewport, so a transparent overlay with a control in each corner still acquired a full-screen footprint and could prune the app beneath it. Footprints now keep their presented rects apart, and coverage is the overlapped area of the two unions (coordinate-compressed cell sweep). Scrollables count as presenting their box: they consume touches over it, which is what lets a real pushed screen (header, scrollable body, footer) still cover a drawer surface. Adds the disconnected-corner regression.
…ints Fuzzing random sibling trees old-vs-new surfaced the one direction the footprint model could still regress: a container whose only painted content is small (one corner icon) but which also carries labelled containers or testID-only markers was condemned as soon as a touch surface covered that icon, since markers and container labels are not paint and never entered the footprint. Footprints now carry two rect sets. `paints` (touch targets, scrollables, labelled leaves) is what a candidate can cover with; it still excludes identifiers and container labels, or the DoKit fix would unwind. `shows` adds every labelled or identified node and is what a covered sibling must lose in full. Focusable-only nodes no longer paint their box either, matching #1733 for descendants as well as siblings. Adds the marker regression. Re-fuzzed 20k trees: new-prunes-more is down to 0.14 %, all of the class where everything the target shows lies under a higher touch/scroll surface. Live captures unchanged.
…e's box A full-screen focusable wrapper holding one clickable icon is a covering candidate; the lower app content must survive. Fails when paintsOwnBox counts focus targets again.
5be8ba0 to
189990a
Compare
|
Both points addressed at 189990a.
PR body updated to list the sixth fixture. |
|
Re-reviewed exact head 189990a: the added focusable-wrapper regression closes the prior gap and is non-vacuous—restoring the old full-box paint behavior prunes the lower content. The delta is test-only, the PR body is current, and Coverage plus all completed gates are green. Code review is clean and ready-for-human; iOS Smoke is still in progress. |
Summary
On Android,
snapshotreturned a 2–4 node tree for a fully rendered screen whenever a higher drawing-order sibling with sparse content sat over the app UI — a DoraemonKit full-screen drag surface holding one floating icon, or an empty labelledmatch_parentplaceholder container.pruneAndroidCoveredSubtreescredited a covering sibling with painting its whole box as soon as it had any agent target anywhere inside it, or a label of its own. Both #1806 patterns qualified and took the entire app subtree with them.Occlusion is now spatial:
No flag, no hint: the defect can't be expressed rather than being detected.
Closes #1806.
Validation
Live, Pixel 9 Pro XL API 37 emulator. Added a DoKit-shaped overlay to the test app (full-screen non-clickable
ViewwithaccessibilityLabel, one smallPressableinside; not committed) and drove the built CLI:Helper-XML A/B (same XML through pre-fix and fixed parser, raw + interactive): with the overlay, home / catalog / form / product-detail all went from the same 45-node skeleton (21 interactive) to 116 / 125 / 111 / 83 nodes with every app label recovered and none lost. Product detail (a native-stack push over the Catalog tab) shows only the pushed screen — no Catalog leak. Settings, Chrome, launcher and the pre-
collapsable={false}build (where RN flattened the overlay away) are byte-identical old vs new.Unit. Six new fixtures fail on the pre-fix parser: the DoKit overlay, the empty labelled placeholder, a sparse-overlay-vs-rich-sibling footprint case, an overlay whose only controls sit in opposite corners (also fails on the first revision's bounding-box model, which is what it pins), a container of testID markers with one covered corner icon (fails on the second revision's single-footprint model), and a focusable full-screen wrapper holding one clickable icon over app content (fails if focusability is allowed to paint a candidate's box). Three existing fixtures were the exact synthetic shape the fix stops calling covered (a full-screen box with one small element over a full-screen box with an element elsewhere) and were rewritten to realistic covering content — a pushed screen with header + scrollable body + footer, a full-width tile, and a clickable scrim; the boundary they pin still holds.
Fuzz. 20 k random 2–3-sibling trees over the attribute space (bounds incl. child overflow, drawing-order, clickable/focusable/scrollable, text, resource-id), pre-fix vs fixed parser, disagreements classified by direction. "New prunes more" — the only direction that can produce a new sparse-snapshot bug — is 0.14 % with children clipped to parents (0.8 % with overflow), and every inspected case is the intended rule: everything the target shows lies fully under a higher touch/scroll surface or an overflowing labelled leaf. The rest is the pruner being more conservative.
check:affectedset green locally: format, lint, typecheck, layering, fallow, build, vitest-related.Notes
openimmediately aftercloseon the same emulator reportedDEVICE_IN_USEby the just-closed session for a couple of seconds.