#579 - set-f-trashwarn is never slotted into an admin page — is it reachable at all? - Cordy/Cairn

Cordy commented

2026-09-17 20:19:20 +00:00

Owner

Found by the new web/test/settings-slots.test.js guard added in #571, which asserts that every set-f-* field in the markup appears in SET_SLOTS.

What

Every setting lives once, in the hidden #settingsdlg. ensureSettingsInShell() then moves each field listed in SET_SLOTS into its admin slot:

for (const [id, slot] of SET_SLOTS) { const e = $(id), s = $(slot); if (e && s) s.appendChild(e); }

A field that is not in SET_SLOTS is never moved, and #settingsdlg is not itself reachable from the admin shell — so the field renders nowhere. The markup looks entirely correct while this is true, which is why it went unnoticed.

set-f-trashwarn is in that state today.

Why this is not obviously a bug

Two readings, and I do not know which is right:

It should be visible and was missed when the settings dialog was broken up into admin pages — in which case it needs a SET_SLOTS entry (probably adm-trash-ret, beside set-f-autopurge and set-f-purgeafter) and a SET_DESC entry.
It is deliberately not exposed — a setting that exists in the object and is saved, but which the admin UI intentionally does not offer.

If (2), the allow-list in the test should stay and gain a note saying why. If (1), the fix is two lines and the allow-list goes.

Current state

The test allow-lists it so the guard can land without silently changing behaviour I do not understand:

// Known orphan, tracked separately: see this issue.
const KNOWN_UNSLOTTED = new Set(["set-f-trashwarn"]);

Worth noting

The same guard caught a brand-new instance of this in #571 — a switch that had been merged, looked perfect in the markup, and would have rendered on no page at all. The invariant is worth keeping either way.

Found by the new `web/test/settings-slots.test.js` guard added in #571, which asserts that every `set-f-*` field in the markup appears in `SET_SLOTS`. ## What Every setting lives once, in the hidden `#settingsdlg`. `ensureSettingsInShell()` then **moves** each field listed in `SET_SLOTS` into its admin slot: ```js for (const [id, slot] of SET_SLOTS) { const e = $(id), s = $(slot); if (e && s) s.appendChild(e); } ``` A field that is not in `SET_SLOTS` is never moved, and `#settingsdlg` is not itself reachable from the admin shell — so the field renders **nowhere**. The markup looks entirely correct while this is true, which is why it went unnoticed. `set-f-trashwarn` is in that state today. ## Why this is not obviously a bug Two readings, and I do not know which is right: 1. **It should be visible** and was missed when the settings dialog was broken up into admin pages — in which case it needs a `SET_SLOTS` entry (probably `adm-trash-ret`, beside `set-f-autopurge` and `set-f-purgeafter`) and a `SET_DESC` entry. 2. **It is deliberately not exposed** — a setting that exists in the object and is saved, but which the admin UI intentionally does not offer. If (2), the allow-list in the test should stay and gain a note saying why. If (1), the fix is two lines and the allow-list goes. ## Current state The test allow-lists it so the guard can land without silently changing behaviour I do not understand: ```js // Known orphan, tracked separately: see this issue. const KNOWN_UNSLOTTED = new Set(["set-f-trashwarn"]); ``` ## Worth noting The same guard caught a brand-new instance of this in #571 — a switch that had been merged, looked perfect in the markup, and would have rendered on no page at all. The invariant is worth keeping either way.

Cordy referenced this issue

2026-09-17 20:26:57 +00:00

fix(#571): the internal-sharing switch never reached the admin page #580

Cordy added this to the v0.7 — Closed beta (dogfood + hardening) milestone

2026-09-18 00:29:35 +00:00

Cordy referenced this issue from a commit

2026-09-18 02:26:05 +00:00

scout: Phase-0 recon — trashwarn (#579) and entitled-write capping (#595)

Cordy referenced this issue from a commit

2026-09-18 02:37:55 +00:00

scout: round 2 — fix harnesses for #579 and #595

Cordy referenced this issue from a commit

2026-09-18 02:45:01 +00:00

chore: one-shot for the #579 fix (red then green on fix-579)

forgejo-actions referenced this issue from a commit

2026-09-18 02:47:54 +00:00

fix(#579): the sweep-off warning is slotted, toggled, and its dead twin removed

Cordy referenced this issue

2026-09-18 02:50:05 +00:00

fix(#579): the sweep-off warning is slotted, toggled, and its dead twin removed #599

Cordy referenced this issue from a commit

2026-09-18 02:58:19 +00:00

fix(#579): the sweep-off warning is slotted, toggled, and its dead twin removed (#599)

Cordy referenced this issue

2026-09-18 03:06:50 +00:00

docs: v0.6.215 changelog entry #601

Cordy commented

2026-09-18 03:17:14 +00:00

Author

Owner

Answered and fixed — #599, shipped in v0.6.215, live on both dogfoods.

The answer to the issue's either/or turned out to be "reading 1, but worse": set-f-trashwarn is the "deleted files accumulate without limit" notice meant to appear while auto-purge is off, and it was doubly orphaned — not in SET_SLOTS (rendered on no page) and not toggled by syncTrashAuto (would have stayed hidden even if slotted). It also had a stowaway: sdTrashWarn, a fully ×4-translated description for a "warn people in the file browser before the sweep" feature that was never built and referenced nowhere.

Decision (Nikola): slot it. Shipped: the notice is slotted beside the auto-purge switch on the Deleted-files page, syncTrashAuto shows it exactly while the sweep is off, the KNOWN_UNSLOTTED allow-list is empty again (mechanism kept), and the dead key is gone. Red witnessed twice — the slots guard on the un-allow-listed orphan, and the new trash-warn.test.js on the missing toggle.

The guard that found this has now caught three of this class (#571's switch, the #571 allow-list landing, this). Worth its weight.

**Answered and fixed — #599, shipped in v0.6.215, live on both dogfoods.** The answer to the issue's either/or turned out to be "reading 1, but worse": `set-f-trashwarn` is the *"deleted files accumulate without limit"* notice meant to appear while auto-purge is off, and it was **doubly** orphaned — not in `SET_SLOTS` (rendered on no page) *and* not toggled by `syncTrashAuto` (would have stayed hidden even if slotted). It also had a stowaway: `sdTrashWarn`, a fully ×4-translated description for a "warn people in the file browser before the sweep" feature that was never built and referenced nowhere. Decision (Nikola): slot it. Shipped: the notice is slotted beside the auto-purge switch on the Deleted-files page, `syncTrashAuto` shows it exactly while the sweep is off, the `KNOWN_UNSLOTTED` allow-list is empty again (mechanism kept), and the dead key is gone. Red witnessed twice — the slots guard on the un-allow-listed orphan, and the new `trash-warn.test.js` on the missing toggle. The guard that found this has now caught three of this class (#571's switch, the #571 allow-list landing, this). Worth its weight.

Cordy closed this issue

2026-09-18 03:17:25 +00:00

Rows
Columns

set-f-trashwarn is never slotted into an admin page — is it reachable at all? #579

What

Why this is not obviously a bug

Current state

Worth noting