Skip to content

Commit edf8d6a

Browse files
committed
feat(geometry): persist detached-tab window size and position
A popped-out tab window was excluded from geometry persistence structurally, alongside the home surface, so every pop-out opened at the consuming app's hardcoded default regardless of how the last one was resized. The exclusion was over-broad. What makes a window unpersistable is having no stable identity to key on — true of the home surface (one window, many states) but not of a detached tab: every consumer already derives its detach token from a durable tab identity, so `shell-detach:<token>` names the same tab across sessions. Dropping the clause hands detached windows the whole existing restore path — work-area clamp, most-overlap monitor pick, fullscreen guard, display-gone fallback — with no new code. The move/resize cache handler becomes load-bearing rather than a nicety: closing a detached window IS how a tab redocks, so it is the normal path, long before the exit-time flush. Records the two invariants this creates for consumers: the detach token must stay deterministic, and DetachSpec's width/height are now the first-pop-out default only, so docked content must size off the built window's real inner_size().
1 parent 773bbc9 commit edf8d6a

3 files changed

Lines changed: 68 additions & 29 deletions

File tree

CLAUDE.md

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -51,8 +51,10 @@ regardless of what it hosts. It is NOT a place to abstract things that merely *l
5151
scope, not a gap to close.
5252
- **`geometry`** (`runtime` feature) — per-window size/position persistence. It owns all of it:
5353
point-based storage, the fullscreen/minimized recording guard, the target-monitor clamp on
54-
restore, and the structural exclusion of the home and every detached-tab window (for save as well
55-
as restore — see `detach`'s label scheme above). Nothing stays per-app beyond handing it a
54+
restore, and the structural exclusion of the home surface (for save as well as restore).
55+
**A detached-tab window is persisted, not excluded** — its label is deterministic per tab, so a
56+
popped-out tab reopens at the size and position it was last left at; see `detach`'s label scheme
57+
below. Nothing stays per-app beyond handing it a
5658
resolved config path: the canonicalize→hash→format filename step
5759
(`.window-geometry-{fnv1a_64(canonicalize(path)):016x}.json`, `geometry_filename`) lives here
5860
once, since it was byte-identical across all three apps' old per-app copies. Uses its own
@@ -208,10 +210,19 @@ regardless of what it hosts. It is NOT a place to abstract things that merely *l
208210
things:
209211
- **A reserved label scheme.** `DETACH_LABEL_PREFIX = "shell-detach:"` + `detached_label(token)`
210212
build a window label for a popped-out tab; `is_detached_label`/`detach_token` are the inverse.
211-
A consumer's hot-reload reconcile calls `is_detached_label` itself to skip these windows, and
212-
`geometry` uses the same check internally to exclude them from persistence — the same
213-
structural exclusion `home::HOME_LABEL` gets, generalized from a single fixed label to an
214-
unbounded set (one per detached tab), whenever a detached window is created.
213+
A consumer's hot-reload reconcile calls `is_detached_label` itself to skip these windows.
214+
- **The `token` a consumer passes is load-bearing beyond uniqueness — it must derive from a
215+
durable tab identity.** `geometry` persists a detached window's size and position keyed on
216+
the resulting label; that is *why* a popped-out tab reopens at the shape you left it. A token
217+
built from a per-session counter or allocation order would silently forget the geometry every
218+
launch and orphan an entry each time. Every consumer already derives it from a stable
219+
identity (warden hashes `origin_label:tab_key`; curator and lector hash the tab's own label),
220+
and each pins that determinism with its own test.
221+
- **`DetachSpec`'s `width`/`height` are the first-pop-out default only.** Restore overwrites
222+
them inside `open_detached`'s `build()` for any tab popped out before, so a consumer sizing
223+
its docked content must read the built window's real `inner_size()` inside `birth_content`,
224+
never the constants it passed in. warden hit exactly this: its reparented native surface was
225+
born at the default rect and only snapped right once `detach.html` reported the true hole.
215226
- **A banner-only shell page** (`detach.html` — title + origin accent, no sidebar), served over
216227
its own custom protocol `DETACH_SCHEME`, registered on the `Builder` by
217228
`register_detach_protocol` (chained into `register_plugins` alongside `home`'s). Same reasoning

src/detach.rs

Lines changed: 18 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,11 @@
44
//! behind each piece here.
55
//!
66
//! - The **label scheme** (`DETACH_LABEL_PREFIX`/`detached_label`/`is_detached_label`/
7-
//! `detach_token`) marks a window as an ephemeral "popped out" tab so a consuming app's
8-
//! hot-reload reconcile can skip it, and so [`crate::geometry`] can exclude it from persistence
9-
//! internally (both save and restore) — the same exclusion [`crate::home::HOME_LABEL`] gets,
10-
//! generalized to an unbounded set of ephemeral windows (one per detached tab).
7+
//! `detach_token`) marks a window as a "popped out" tab so a consuming app's hot-reload
8+
//! reconcile can skip it. The label is *stable per tab* — every consumer derives `token` from a
9+
//! durable tab identity — which is what lets [`crate::geometry`] persist a detached window's
10+
//! size and position like any other window's, rather than excluding it the way
11+
//! [`crate::home::HOME_LABEL`] is excluded.
1112
//! - The **banner-shell page** (`DETACH_SCHEME`/`DetachSpec`/`register_detach_protocol`) is the
1213
//! slim identity banner (title + accent stripe) a detached window shows above its transparent
1314
//! content hole, reporting that hole's rect to the app via `set_hole_rect` — every app already
@@ -17,20 +18,23 @@
1718
1819
/// Prefix marking a window label as a detached-tab window. A label under this prefix is never a
1920
/// real (config-defined) window label, so a consuming app's reconcile can use [`is_detached_label`]
20-
/// to skip it, and [`crate::geometry`] uses the same check internally to exclude it from
21-
/// persistence — the same exclusion `home::HOME_LABEL` gets, generalized to an unbounded set of
22-
/// ephemeral windows (one per detached tab) rather than a single fixed label.
21+
/// to skip it.
2322
pub const DETACH_LABEL_PREFIX: &str = "shell-detach:";
2423

2524
/// Build the Tauri window label for a detached tab identified by `token` (an opaque,
2625
/// caller-chosen identifier — e.g. the tab's own key).
26+
///
27+
/// **`token` must be derived from a durable tab identity, not a per-session counter or an
28+
/// allocation order.** [`crate::geometry`] keys a detached window's remembered size and position
29+
/// on the resulting label, so a token that changes between sessions silently forgets the tab's
30+
/// geometry and leaves an orphan entry behind each time. Every consumer already does this
31+
/// (warden hashes `origin_label:tab_key`; curator and lector hash the tab's own label).
2732
pub fn detached_label(token: &str) -> String {
2833
format!("{DETACH_LABEL_PREFIX}{token}")
2934
}
3035

3136
/// Whether `label` names a detached-tab window (as opposed to a real config-defined window or the
32-
/// home surface). A consuming app's reconcile, and [`crate::geometry`] internally, both use this
33-
/// to skip these windows.
37+
/// home surface). A consuming app's reconcile uses this to skip these windows.
3438
pub fn is_detached_label(label: &str) -> bool {
3539
label.starts_with(DETACH_LABEL_PREFIX)
3640
}
@@ -58,6 +62,11 @@ const DETACH_HTML: &str = include_str!("detach.html");
5862
/// What a popped-out tab's banner shows, plus the size the detached window should open at.
5963
/// `colour` is the tab's/window's accent colour (the same hex the sidebar swatch uses); `None`
6064
/// falls back to the page's own default stripe colour.
65+
///
66+
/// `width`/`height` are the **first-pop-out default only**. Once this tab has been popped out and
67+
/// resized, [`crate::geometry`] restores its remembered size and position over them during
68+
/// [`open_detached`]'s `build()` — so a caller sizing its own content off these constants rather
69+
/// than off the built window's real `inner_size()` will get it wrong for every subsequent pop-out.
6170
pub struct DetachSpec {
6271
pub title: String,
6372
pub colour: Option<String>,

src/geometry.rs

Lines changed: 33 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -166,14 +166,24 @@ struct GeometryState {
166166
cache: Mutex<HashMap<String, Rect>>,
167167
}
168168

169-
/// Windows shell-core owns that are transient by construction and must never persist bounds: the
170-
/// home surface, and any popped-out tab window. Excluded structurally — for **save** as well as
171-
/// restore — rather than by a caller-supplied list, because a detached window is created long
172-
/// after startup and so could never have appeared in one.
169+
/// The one window shell-core owns that must never persist bounds: the home surface. Excluded
170+
/// structurally — for **save** as well as restore — rather than by a caller-supplied list, since
171+
/// every consumer has it and none of them should have to remember to list it. `skip` is the
172+
/// caller's own transient windows on top.
173+
///
174+
/// **A detached-tab window is deliberately NOT excluded**, though it is just as ephemeral. What
175+
/// makes it persistable is that its label is *deterministic per tab*: every consumer derives the
176+
/// detach token from a stable tab identity (warden from `origin_label:tab_key`, curator and lector
177+
/// from the tab's own label), so `shell-detach:<token>` names the same tab across sessions and its
178+
/// stored rect is meaningful on the next pop-out. The home surface has no such identity to key on
179+
/// — it is a single window whose bounds every state it shows would have to share.
180+
///
181+
/// The consequence, accepted: the store gains one entry per tab ever popped out, never pruned.
182+
/// Ordinary windows already accumulate the same way (a title change orphans an entry), an entry is
183+
/// a handful of bytes, and pruning would need tab-identity knowledge this module deliberately
184+
/// does not have.
173185
fn is_excluded(state: &GeometryState, label: &str) -> bool {
174-
label == crate::home::HOME_LABEL
175-
|| crate::detach::is_detached_label(label)
176-
|| state.skip.contains(label)
186+
label == crate::home::HOME_LABEL || state.skip.contains(label)
177187
}
178188

179189
/// A monitor's work area (screen minus menu bar and Dock) in points, converted with **that
@@ -348,7 +358,8 @@ fn flush<R: Runtime>(app: &AppHandle<R>) {
348358
}
349359

350360
/// Build the geometry plugin. `filename` comes from [`geometry_filename`]; `skip_labels` are an
351-
/// app's own transient windows (the home and detached surfaces are excluded structurally).
361+
/// app's own transient windows (the home surface is excluded structurally — see [`is_excluded`],
362+
/// which also records why a detached-tab window is not).
352363
pub fn plugin<R: Runtime>(filename: String, skip_labels: &[&str]) -> TauriPlugin<R> {
353364
let skip: HashSet<String> = skip_labels.iter().map(|s| s.to_string()).collect();
354365

@@ -388,7 +399,10 @@ pub fn plugin<R: Runtime>(filename: String, skip_labels: &[&str]) -> TauriPlugin
388399
// Snapshot on move/resize so a window's bounds survive even when the window itself is
389400
// closed mid-session: by the time `flush` runs on `RunEvent::Exit`, a closed window is
390401
// gone from `app.windows()`, so without this event-driven cache its bounds would be
391-
// lost entirely rather than merely stale. (The cache is memory-only and the exit-time
402+
// lost entirely rather than merely stale. That is not a nicety for a detached-tab
403+
// window — closing it IS how a popped-out tab redocks, so it is the *normal* path, and
404+
// this handler is the only reason its remembered size survives at all.
405+
// (The cache is memory-only and the exit-time
392406
// flush is the only write to disk, so this does *not* protect against an abnormal exit
393407
// or crash — only against a window closing before a normal one.) No suppression around
394408
// `restore` is needed: an echoed event records the geometry the window genuinely has,
@@ -593,19 +607,24 @@ mod tests {
593607
assert_eq!(fnv1a_64(b"foobar"), 0x8594_4171_f739_67e8);
594608
}
595609

596-
/// The one behavioural rule `is_excluded` enforces — home, detached, and caller-skip windows
597-
/// never persist bounds — is exactly the kind of thing a future refactor could silently
598-
/// invert with no error, just wrong windows restored next launch.
610+
/// The one behavioural rule `is_excluded` enforces — home and caller-skip windows never
611+
/// persist bounds, every other window does — is exactly the kind of thing a future refactor
612+
/// could silently invert with no error, just wrong windows restored next launch.
613+
///
614+
/// The detached case is asserted in the *negative* on purpose: a popped-out tab window
615+
/// persists its size and position like any other window, keyed by its deterministic
616+
/// `shell-detach:<token>` label. Re-adding it to the exclusion is the regression this
617+
/// assertion exists to fail on.
599618
#[test]
600-
fn is_excluded_covers_home_detached_and_the_skip_list_but_not_an_ordinary_window() {
619+
fn is_excluded_covers_home_and_the_skip_list_but_not_an_ordinary_or_detached_window() {
601620
let state = GeometryState {
602621
filename: "test.json".to_string(),
603622
skip: ["sidebar".to_string()].into_iter().collect(),
604623
cache: Mutex::new(HashMap::new()),
605624
};
606625
assert!(is_excluded(&state, crate::home::HOME_LABEL));
607-
assert!(is_excluded(&state, "shell-detach:abc123"));
608626
assert!(is_excluded(&state, "sidebar"));
609627
assert!(!is_excluded(&state, "main"));
628+
assert!(!is_excluded(&state, "shell-detach:abc123"));
610629
}
611630
}

0 commit comments

Comments
 (0)