From 49191567d4df1db0975c3bbf83fedf20c596c8ec Mon Sep 17 00:00:00 2001 From: GenericJam Date: Thu, 27 Aug 2026 03:24:04 -0600 Subject: [PATCH 1/3] =?UTF-8?q?Bump=20to=200.7.31=20=E2=80=94=20frame-trac?= =?UTF-8?q?ker=20ownership=20fix=20+=20honest=20platform=20docs?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Release-gate review of 0.7.29..master (the policy that landed in 0.7.30) caught three things in the MOB-103 work that 0.7.30 already shipped. 1. iOS: a frame tracker whose write was REFUSED lost its ownership token. `record` assigned box.seq unconditionally, so a generation-refused write (returns 0) overwrote a valid seq with 0 — and mob_unregister_frame ignores seq 0, permanently disabling that tracker's own cleanup. It bites when an outgoing screen's .move writes are refused by the generation gate AND the incoming screen's element with the same :id isn't laid out (a lazy row below the fold): the id stays in the tree so the purge keeps it, nothing deletes it, and Mob.Test.tap_id taps the old screen's coordinates. Keeping the last SUCCESSFUL seq is strictly safe — the compare-and-delete still refuses to delete whenever an incoming tracker has since claimed the id. The .opacity (reset) transition doesn't hit this, which is the tell: it doesn't move the outgoing frame, so nothing gets refused. 2. Mob.Test.element_frames/1 documented the drop-when-not-laid-out behaviour as if it were cross-platform. It is iOS-only: Android clears frames wholesale on a navigation transition and never per element (MobBridge.setRootJson), so a scrolled-away lazy_list row keeps its frame and tap_id happily taps it. The docs now say so plainly rather than implying parity. Android parity is worth its own issue. 3. CHANGELOG's top section was still [Unreleased] when 0.7.30 was tagged, so release.yml's `## []` extraction found nothing and published 0.7.30 with empty release notes. Retitled to [0.7.30] so the notes exist for anyone reading back, with this release's entries above it. Also wires `make -C test/native run` into the macOS CI job. That harness is the only automated guard on the registry's ownership/generation/purge rules and it reproduces the algorithm rather than linking it, so running it is what keeps the copy honest — it was committed in 0.7.30 but nothing ran it. Preflight: format, credo --strict, compile --warnings-as-errors, 1111 tests, clang-format, and the native harness (20/20) all clean. Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/test.yml | 8 ++++++++ CHANGELOG.md | 20 +++++++++++++++++++- ios/MobRootView.swift | 17 ++++++++++++++++- lib/mob/test.ex | 23 ++++++++++++++++------- mix.exs | 2 +- 5 files changed, 60 insertions(+), 10 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 15e80db..326dfb3 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -76,6 +76,14 @@ jobs: ios/mob_nif.m \ android/jni/mob_beam.h + - name: Native harness (frame registry — MOB-102/MOB-103) + # The only automated guard on the element-frame registry's ownership, + # generation-gating and purge rules. It reproduces the algorithm rather + # than linking ios/mob_nif.m (which needs UIKit + the swiftc-generated + # header), so it can drift — running it in CI is what keeps the copy + # honest. Same toolchain the clang-format step above already uses. + run: make -C test/native run + - name: Install swiftlint run: brew install swiftlint diff --git a/CHANGELOG.md b/CHANGELOG.md index de046e1..122dbf3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,7 +8,25 @@ Full module documentation: [hexdocs.pm/mob](https://hexdocs.pm/mob). --- -## [Unreleased] +## [0.7.31] - 2026-08-27 + +### Fixed +- iOS: a frame tracker whose write was refused (an outgoing screen mid-nav) + no longer loses its ownership token. It kept the last returned value even + when that was the "refused" sentinel, which silently disabled its own + cleanup — so if the incoming screen's element with the same `:id` wasn't + laid out (a lazy row below the fold), the old screen's entry survived and + `Mob.Test.tap_id/2` tapped its coordinates. + +### Changed +- `Mob.Test.element_frames/1` docs now say plainly that the drop-when-not-laid-out + behaviour is **iOS only**. Android clears frames wholesale on a navigation + transition and never per element, so a scrolled-away row still reports a + position there — the previous wording read as cross-platform. + +--- + +## [0.7.30] - 2026-08-27 ### Fixed - **`Mob.Test.element_frames/1` no longer reports elements that are in the diff --git a/ios/MobRootView.swift b/ios/MobRootView.swift index 2d34854..31239ee 100644 --- a/ios/MobRootView.swift +++ b/ios/MobRootView.swift @@ -600,9 +600,24 @@ private struct MobFrameTracker: ViewModifier { if box.generation == 0 { box.generation = mob_frame_generation() } - box.seq = mob_register_frame( + let written = mob_register_frame( id, box.generation, Double(frame.minX), Double(frame.minY), Double(frame.width), Double(frame.height)) + + // Keep the last SUCCESSFUL seq. A refused write returns 0, and + // overwriting the token with 0 would make this tracker's .onDisappear + // a permanent no-op (mob_unregister_frame ignores seq 0) — it could + // then never clean up the entry it still owns. That bites when an + // outgoing screen's `.move` writes are refused by the generation gate + // and the incoming screen's element with the same :id isn't laid out + // (a lazy row below the fold): the id stays in the tree so the purge + // keeps it, nothing deletes it, and tap_id taps the old screen's + // coordinates. Retaining the token is strictly safe — the + // compare-and-delete still refuses to delete whenever an incoming + // tracker has claimed the id since. + if written != 0 { + box.seq = written + } } } diff --git a/lib/mob/test.ex b/lib/mob/test.ex index 2371c64..6e21a33 100644 --- a/lib/mob/test.ex +++ b/lib/mob/test.ex @@ -1113,13 +1113,22 @@ defmodule Mob.Test do Mob.Test.element_frames(node) #=> %{"save" => {24.0, 720.0, 327.0, 48.0}, "row_3" => {0.0, 300.0, 393.0, 56.0}} - ## What counts as "rendered" - - An element appears here once it has laid out, and is dropped when it leaves - the tree *or* stops being laid out — a lazy-list row scrolled out of range, - a tab that isn't the active one, a dismissed sheet's content. Those all stay - in the render tree while off screen, so tree membership alone would report - them; they're dropped on the platform's own disappear signal instead. + ## What counts as "rendered" — and it differs by platform + + **On iOS**, an element appears here once it has laid out, and is dropped when + it leaves the tree *or* stops being laid out — a lazy-list row scrolled out of + range, a tab that isn't the active one, a dismissed sheet's content. Those all + stay in the render tree while off screen, so tree membership alone would + report them; they're dropped on the platform's own disappear signal instead. + + **On Android**, only the first half holds. Frames are cleared wholesale on a + navigation transition and never per-element, so an element that stays in the + tree but stops being laid out keeps its last frame until you navigate. A + scrolled-away `lazy_list` row still reports a position there, and `tap_id/2` + will happily tap it — so on Android, treat a frame for content that may have + scrolled or switched tabs as unverified, and confirm against `render_tree/1` + or a screenshot before acting on it. Bringing Android to parity is tracked + separately. A frame is a *last known* position, not a synchronous read: it's recorded as the element lays out. After a render that moves an element, there's a brief diff --git a/mix.exs b/mix.exs index 38b477c..c3461ce 100644 --- a/mix.exs +++ b/mix.exs @@ -4,7 +4,7 @@ defmodule Mob.MixProject do def project do [ app: :mob, - version: "0.7.30", + version: "0.7.31", elixir: "~> 1.19", start_permanent: Mix.env() == :prod, elixirc_paths: elixirc_paths(Mix.env()), From 39029af94d38fd82eba9e427b945e05a0ac84c72 Mon Sep 17 00:00:00 2001 From: GenericJam Date: Thu, 27 Aug 2026 09:12:14 -0600 Subject: [PATCH 2/3] MOB-104: add mob_send_dismiss for Android MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Android had no dismiss sender at all, so mob_new's generated sheet renderer fell back to nativeSendTap and delivered {:tap, tag}. Mob.UI.sheet/2 documents :on_dismiss as {:dismiss, tag} (lib/mob/ui.ex:287) and iOS has always sent that (mob_send_dismiss, ios/mob_nif.m:233), so a screen written to the contract never matched: Mob.Screen forwards the unmatched message to handle_info, which raises FunctionClauseError and takes the screen down — or silently drops it with a catch-all, in which case the BEAM never learns the sheet closed and can't re-present it. sendEvent already parameterises the atom, so this is the tap sender with "dismiss" instead of "tap", plus the mob_beam.h declaration the generated beam_jni.c thunk compiles against. Paired with mob_new's MOB-104 change, which adds the nativeSendDismiss extern and JNI thunk and switches sendDismissOnce over. The coupling is compile- enforced: mob_new's "beam_jni.c passes clang -fsyntax-only" test builds the rendered thunk against MOB_DIR's mob_beam.h, so mob_new CI fails until this lands. mob releases first. Co-Authored-By: Claude Opus 5 (1M context) --- android/jni/mob_beam.h | 5 +++++ android/jni/mob_nif.zig | 14 ++++++++++++++ 2 files changed, 19 insertions(+) diff --git a/android/jni/mob_beam.h b/android/jni/mob_beam.h index 13a8294..fe2d396 100644 --- a/android/jni/mob_beam.h +++ b/android/jni/mob_beam.h @@ -16,6 +16,11 @@ void mob_ui_cache_class(JNIEnv *env, const char *bridge_class); // Called from the app's Java_..._MobBridge_nativeSendTap JNI stub. void mob_send_tap(int handle); +// Send a {:dismiss, tag} event — the shape Mob.UI.sheet/2 documents for +// :on_dismiss, matching iOS. Called from the app's +// Java_..._MobBridge_nativeSendDismiss JNI stub. +void mob_send_dismiss(int handle); + // Send a {:change, tag, value} event. Called from the app's // Java_..._MobBridge_nativeSendChange* JNI stubs. void mob_send_change_str(int handle, const char *utf8); diff --git a/android/jni/mob_nif.zig b/android/jni/mob_nif.zig index 8cf856d..167be72 100644 --- a/android/jni/mob_nif.zig +++ b/android/jni/mob_nif.zig @@ -1074,6 +1074,20 @@ pub export fn mob_send_tap(handle: c_int) callconv(.c) void { sendEvent(handle, "tap"); } +/// Called from beam_jni.c's `nativeSendDismiss` JNI stub. Sends +/// `{:dismiss, tag}` — the shape `Mob.UI.sheet/2` documents for `:on_dismiss` +/// and the one iOS has always delivered (`mob_send_dismiss` in ios/mob_nif.m). +/// +/// Android had no dismiss sender at all, so the generated sheet renderer fell +/// back to `nativeSendTap` and delivered `{:tap, tag}`. A screen written to the +/// documented contract never matched it: `Mob.Screen` forwards the unmatched +/// message to `handle_info`, which raises `FunctionClauseError` and takes the +/// screen down — or silently drops it if the screen has a catch-all, in which +/// case the BEAM never learns the sheet closed and can't re-present it (MOB-104). +pub export fn mob_send_dismiss(handle: c_int) callconv(.c) void { + sendEvent(handle, "dismiss"); +} + pub export fn mob_send_change_str(handle: c_int, utf8: [*:0]const u8) callconv(.c) void { const tmp = erts.enif_alloc_env() orelse return; defer erts.enif_free_env(tmp); From c0a3a52e84b80d669315de637be45be36d0c15dd Mon Sep 17 00:00:00 2001 From: GenericJam Date: Thu, 27 Aug 2026 09:31:23 -0600 Subject: [PATCH 3/3] =?UTF-8?q?MOB-104:=20address=20adversarial=20review?= =?UTF-8?q?=20=E2=80=94=20changelog,=20harness=20coverage,=20cross-refs?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - CHANGELOG: 0.7.31 never mentioned mob_send_dismiss, the new exported symbol mob_new's generated beam_jni.c links against. Someone hitting the "call to undeclared function 'mob_send_dismiss'" build error would have searched the changelog and found nothing. Added, with the Android behaviour change spelled out: anyone who worked around the old bug by matching {:tap, tag} for a sheet dismissal has a dead clause now. Also reinstated the [Unreleased] heading that the previous commit deleted outright, so the next change has somewhere to land. - test/native: the harness narrated the PRE-fix caller ("refused write leaves seq 0, so disappear can't delete") while asserting behaviour the fixed caller no longer produces — it passed by a different mechanism than the comment claimed, and the scenario the fix actually exists for had no coverage at all. Reverting MobRootView.swift left CI green. Case 4 now describes the retained- token model, and 4b covers the untested case: outgoing writes refused AND no incoming tracker ever claims the id, so the retained token is the only thing that can clean up. Both caller behaviours are modelled side by side, since the difference lives in Swift and this harness only has the C half. - lib/mob/test.ex: the Android caveat said parity was "tracked separately" without naming an issue, unlike every other cross-reference here. Filed and cited MOB-105. - CHANGELOG: fixed the dangling "note the Unreleased entry above" in 0.7.29, left pointing at a heading the previous commit renamed. Preflight: format, 1111 tests, clang-format, native harness all clean. Co-Authored-By: Claude Opus 5 (1M context) --- CHANGELOG.md | 23 +++++++++++++-- lib/mob/test.ex | 3 +- test/native/frame_registry_test.m | 47 +++++++++++++++++++++++++++---- 3 files changed, 64 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 122dbf3..35c8a68 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,8 +8,27 @@ Full module documentation: [hexdocs.pm/mob](https://hexdocs.pm/mob). --- +## [Unreleased] + ## [0.7.31] - 2026-08-27 +### Added +- **`mob_send_dismiss` (Android)** — sends `{:dismiss, tag}` to the process + registered for a handle, the shape `Mob.UI.sheet/2` documents for + `:on_dismiss` and the one iOS has always delivered. Android had no dismiss + sender at all, so generated sheet renderers fell back to the tap sender and + delivered `{:tap, tag}`; a screen written to the documented contract never + matched it and died with `FunctionClauseError` (or silently dropped the + dismissal and could never re-present the sheet). Pairs with mob_new 0.4.24, + which adds the `nativeSendDismiss` extern and JNI thunk — **generated Android + apps need both halves**, and `mix mob.doctor` (mob_dev) now warns about a + project still carrying the old wiring (MOB-104). + + **Behaviour change for Android:** if you worked around the old bug by + matching `handle_info({:tap, tag}, ...)` for a sheet dismissal, that clause + is now dead — switch it to `{:dismiss, tag}`. iOS callers are unaffected; + they always received `{:dismiss, tag}`. + ### Fixed - iOS: a frame tracker whose write was refused (an outgoing screen mid-nav) no longer loses its ownership token. It kept the last returned value even @@ -104,8 +123,8 @@ Full module documentation: [hexdocs.pm/mob](https://hexdocs.pm/mob). fires when an element's frame *changes*, so anything that stayed put went missing until something moved it. Only ids absent from the incoming tree are dropped now (MOB-102). See - `decisions/2026-08-27-frame-registry-purge-by-id.md` — and note the - Unreleased entry above, which corrects the converse case this introduced. + `decisions/2026-08-27-frame-registry-purge-by-id.md` — and note the 0.7.31 + entry above, which corrects the converse case this introduced. *(Documented after the fact: this shipped in 0.7.29 but was omitted from its notes, so it is not in the published 0.7.29 changelog.)* diff --git a/lib/mob/test.ex b/lib/mob/test.ex index 6e21a33..241e46d 100644 --- a/lib/mob/test.ex +++ b/lib/mob/test.ex @@ -1127,8 +1127,7 @@ defmodule Mob.Test do scrolled-away `lazy_list` row still reports a position there, and `tap_id/2` will happily tap it — so on Android, treat a frame for content that may have scrolled or switched tabs as unverified, and confirm against `render_tree/1` - or a screenshot before acting on it. Bringing Android to parity is tracked - separately. + or a screenshot before acting on it. Bringing Android to parity is MOB-105. A frame is a *last known* position, not a synchronous read: it's recorded as the element lays out. After a render that moves an element, there's a brief diff --git a/test/native/frame_registry_test.m b/test/native/frame_registry_test.m index 2f009e6..96ebc67 100644 --- a/test/native/frame_registry_test.m +++ b/test/native/frame_registry_test.m @@ -161,12 +161,17 @@ int main(void) { // 4. The shared-:id nav case — the one tree membership alone can't // reject, because the id IS in the new tree. The outgoing screen's - // generation is superseded, so its slide-out writes are refused and - // its .onDisappear (seq 0) is a no-op. + // generation is superseded, so its slide-out writes are refused. + // + // Note what protects the incoming entry here: the tracker RETAINS + // its last successful seq rather than storing the refusal (see + // MobFrameTracker.record — `if written != 0`), so its teardown + // calls unregister with that old token, which loses the + // compare-and-delete against the incoming screen's newer seq. reset(); mob_adopt_frame_ids([NSSet setWithArray:@[ @"save" ]]); uint64_t oldGen = mob_frame_generation(); - mob_register_frame("save", oldGen, 24, 720, 327, 48); + uint64_t outgoingSeq = mob_register_frame("save", oldGen, 24, 720, 327, 48); mob_bump_frame_generation(); // nav push: non-"none" transition uint64_t newGen = mob_frame_generation(); mob_adopt_frame_ids([NSSet setWithArray:@[ @"save" ]]); // still present @@ -175,8 +180,40 @@ int main(void) { uint64_t lateSeq = mob_register_frame("save", oldGen, -393, 720, 327, 48); check(lateSeq == 0, "MOB-103: outgoing screen's slide-out write is refused (shared id)"); check(yOf("save") == 640.0, "the incoming screen's frame survives the animation"); - mob_unregister_frame("save", lateSeq); // lateSeq == 0 → no-op - check(present("save"), "MOB-103: refused write leaves seq 0, so disappear can't delete"); + mob_unregister_frame("save", outgoingSeq); // retained token, not the refusal + check(present("save"), "MOB-103: stale token loses the compare-and-delete"); + check(yOf("save") == 640.0, "...and the incoming frame is untouched"); + + // 4b. The case the retained-token fix actually exists for: the + // outgoing screen's writes are refused AND the incoming screen's + // element with the same :id never lays out (a lazy row below the + // fold), so nothing ever claims the id with a newer seq. Storing + // the refusal (seq 0) here would make the tracker's own teardown a + // no-op — mob_unregister_frame ignores 0 — and the id stays in the + // tree so the purge keeps it too. The old screen's frame would + // survive forever and tap_id would tap its coordinates. + // + // This is the check that fails if MobFrameTracker.record goes back + // to assigning box.seq unconditionally. + reset(); + mob_adopt_frame_ids([NSSet setWithArray:@[ @"save" ]]); + uint64_t g4b = mob_frame_generation(); + uint64_t heldSeq = mob_register_frame("save", g4b, 24, 720, 327, 48); + mob_bump_frame_generation(); + mob_adopt_frame_ids([NSSet setWithArray:@[ @"save" ]]); // id still in tree + check(mob_register_frame("save", g4b, -393, 720, 327, 48) == 0, + "outgoing slide-out write refused (nothing else claimed the id)"); + // Both caller behaviours are modelled side by side, because the + // difference lives in Swift (MobFrameTracker.record) and this + // harness only has the C half: the pre-fix caller stored the + // refusal and passed 0, the fixed one passes its retained token. + mob_unregister_frame("save", 0); // pre-fix caller: stored the refusal + check(present("save"), + "pre-fix caller: refusal token disables cleanup — entry survives (the bug)"); + + mob_unregister_frame("save", heldSeq); // fixed caller: retained token + check(!present("save"), + "MOB-103: retained token still deletes when no one else claimed the id"); // 5. MOB-102 must not regress: a static, unmoved element survives an // unrelated re-render without re-registering. An ordinary re-render