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..35c8a68 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,43 @@ 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 + 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 render tree but not on screen.** 0.7.29 shipped a fix (MOB-102) that stopped @@ -86,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/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); 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..241e46d 100644 --- a/lib/mob/test.ex +++ b/lib/mob/test.ex @@ -1113,13 +1113,21 @@ 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 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/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()), 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