diff --git a/AGENTS.md b/AGENTS.md index b972e4a..98a8525 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -127,6 +127,11 @@ them. Revisit only if the stated reason stops holding. so a localized list page doesn't hand a plugin a phantom `locale` filter; the i18n view locals (`t`, `locale`, `locales`, `localeHref`, `localeParam`, `localeSwitch`, `dir`) are likewise reserved names, merged after a handler's `data` so a collision loses the key instead of breaking the shell. +- **The language picker never appears on a POST-rendered page.** That URL frequently answers no GET + (`POST /admin/users/:id/recovery` renders a page and has no GET sibling), so a link there dead-ends + on a 405; on a re-rendered form it would also discard the visitor's input. `i18nLocals` returns an + empty `localeSwitch` for any non-GET/HEAD method, and the picker renders nothing below two choices. + Decided 2026-08-03 after a review reproduced the 405. - **A plugin-owned render always runs on that plugin's context.** The landing slots (`home`, `dashboard`) and an `onRequest` short-circuit dispatch a plugin's handler, so they build the context with `contextFor(pluginId)` exactly as a plugin route does — otherwise `ctx.t` is the core diff --git a/README.md b/README.md index d996dec..bfd06d3 100644 --- a/README.md +++ b/README.md @@ -940,7 +940,10 @@ is stored, so a link is shareable and a page is what its address says it is. Whe for a language, the host carries `?locale=` onto every link *it* renders (menu, sign-in, its own redirects) and `ctx.localeHref(href)` does the same for a plugin's links. The picker in the sidebar footer (and on the auth pages) lists every installed locale, each a plain link to the -same page in that language — it renders only when more than one is installed. +same page in that language. It renders only when more than one is installed, and **not at all on a +page rendered in response to a POST** — that URL often answers no GET (following the link would +dead-end), and on a re-rendered form it would throw away what the visitor typed. A plugin building +its own picker from `ctx.locales` should do the same (`ctx.req.method`). **Writing a catalog.** `en-US.ts` exports the object and its type; every other locale is written against that type, so a missing or misspelled key is a type error before the app ever boots: diff --git a/e2e-tests/full-flow.spec.ts b/e2e-tests/full-flow.spec.ts index 45efc09..9e4745d 100644 --- a/e2e-tests/full-flow.spec.ts +++ b/e2e-tests/full-flow.spec.ts @@ -88,6 +88,7 @@ test.describe.serial("authenticated admin journey", () => { const row = page.locator("tr", { hasText: `lang-${suffix}@plainpages.local` }); const editHref = await row.locator('a[href^="/admin/users/"]').first().getAttribute("href"); await page.goto(`${editHref}`); + await expect(page.locator('summary[aria-label="Språk"]')).toHaveCount(1); // control: it IS there on the GET await page.getByRole("button", { name: "Skapa återställningskod" }).click(); // POST-only route await expect(page.getByText("Återställningskod skapad")).toBeVisible(); await expect(page.locator('summary[aria-label="Språk"]')).toHaveCount(0); // no dead-end link offered diff --git a/examples/plugins/admin/i18n/en-US.ts b/examples/plugins/admin/i18n/en-US.ts index ede0f3c..ab65993 100644 --- a/examples/plugins/admin/i18n/en-US.ts +++ b/examples/plugins/admin/i18n/en-US.ts @@ -138,8 +138,8 @@ const messages = { "admin.users.new": "New user", "admin.users.pagination": "Users pagination", "admin.users.reactivate": "Reactivate", - "admin.users.recovery.body": - "Give it to the user — they enter it on the password-reset screen to set a new password (generate a fresh one if it has expired).", + "admin.users.recovery.body": "Give it to the user — they enter it to set a new password (generate a fresh one if it has expired):", + "admin.users.recovery.link": "the password-reset screen", "admin.users.recovery.generate": "Generate recovery code", "admin.users.recovery.title": "Recovery code generated", "admin.users.save": "Save changes", diff --git a/examples/plugins/admin/i18n/sv-SE.ts b/examples/plugins/admin/i18n/sv-SE.ts index 349e819..350cf75 100644 --- a/examples/plugins/admin/i18n/sv-SE.ts +++ b/examples/plugins/admin/i18n/sv-SE.ts @@ -138,8 +138,8 @@ const messages: AdminMessages = { "admin.users.new": "Ny användare", "admin.users.pagination": "Sidnavigering för användare", "admin.users.reactivate": "Aktivera igen", - "admin.users.recovery.body": - "Ge den till användaren — koden anges på sidan för lösenordsåterställning för att sätta ett nytt lösenord (skapa en ny om den hunnit gå ut).", + "admin.users.recovery.body": "Ge den till användaren — koden anges för att sätta ett nytt lösenord (skapa en ny om den hunnit gå ut):", + "admin.users.recovery.link": "sidan för lösenordsåterställning", "admin.users.recovery.generate": "Skapa återställningskod", "admin.users.recovery.title": "Återställningskod skapad", "admin.users.save": "Spara ändringar", diff --git a/examples/plugins/admin/views/partials/user-form-body.ejs b/examples/plugins/admin/views/partials/user-form-body.ejs index 1de94a7..e9f2971 100644 --- a/examples/plugins/admin/views/partials/user-form-body.ejs +++ b/examples/plugins/admin/views/partials/user-form-body.ejs @@ -14,7 +14,7 @@ <%- include("partials/alert", { text: locals.error, tone: "neg" }) %> <% } -%> <% if (recovery) { -%> -
<%= t("admin.users.recovery.title") %><%- t("admin.users.recovery.body") %><% if (recovery.code) { %><%= recovery.code %><% } %>
+
<%= t("admin.users.recovery.title") %><%= t("admin.users.recovery.body") %> "><%= t("admin.users.recovery.link") %><% if (recovery.code) { %><%= recovery.code %><% } %>
<% } -%>
diff --git a/src/http/app.test.ts b/src/http/app.test.ts index 768c1bd..a9b6334 100644 --- a/src/http/app.test.ts +++ b/src/http/app.test.ts @@ -23,6 +23,7 @@ import type { Plugin } from "../plugin-host/plugin.ts"; import { contentTypeFor, resolveStaticPath, routePublic } from "./static.ts"; import adminManifest from "../../examples/plugins/admin/plugin.ts"; import { createI18n } from "../i18n/runtime.ts"; +import type { MenuConfig } from "../ui/menu-config.ts"; import { loadI18n } from "../i18n/load.ts"; const viewsDir = join(dirname(fileURLToPath(import.meta.url)), "..", "..", "views"); @@ -1486,3 +1487,19 @@ test("a plugin that owns a landing page, or short-circuits a hook, translates fr assert.equal(await (await fetch(`${url}/hooked`)).text(), "Hello from the plugin"); assert.equal(await (await fetch(`${url}/dashboard`, { headers: { cookie } })).text(), "Hello from the plugin"); }); + +test("an error page renders without composing the menu — it exists for when the shell's data is what failed", async (t) => { + // The chrome getter is lazy on purpose; a render that reads no chrome must not trigger it, or a + // broken menu takes the error pages down with it. + let built = 0; + const menu: MenuConfig = { branding: { get name() { built++; return "Plainpages"; } }, override: {} }; + const app = createApp({ jwks: staticJwks([ecJwk]), menu }); + await new Promise((r) => app.listen(0, r)); + t.after(() => app.close()); + const url = `http://localhost:${(app.address() as AddressInfo).port}`; + + const res = await fetch(`${url}/no-such-page`); + assert.equal(res.status, 404); + assert.match(await res.text(), /Page not found/); + assert.equal(built, 0); +}); diff --git a/src/http/app.ts b/src/http/app.ts index 5a7d741..e2cea3b 100644 --- a/src/http/app.ts +++ b/src/http/app.ts @@ -120,7 +120,17 @@ export function createApp(options: AppOptions = {}): Server { // it. A plugin's context carries that plugin's translator, so its own catalog wins in its own views. // They are merged LAST: these names are reserved (README → Building plugins), and a handler that // happens to use one loses that key rather than breaking the shell that renders around it. - const localsOf = (ctx: RequestContext): I18nRequest => ({ ...ctx, method: ctx.req.method ?? "GET" }); + // Named field by field on purpose: spreading the context would trigger its lazy `chrome` getter, + // composing the menu for every render — including the standalone error pages, which exist to + // render when the shell's own data is what failed. + const localsOf = (ctx: RequestContext): I18nRequest => ({ + locale: ctx.locale, + localeHref: ctx.localeHref, + locales: ctx.locales, + method: ctx.req.method ?? "GET", + t: ctx.t, + url: ctx.url, + }); const viewsFor = (ctx: RequestContext): ViewRenderer => (view, data) => render(view, { ...data, ...i18nLocals(localsOf(ctx)) }); const pluginViewsFor = (ctx: RequestContext, id: string): ViewRenderer => (view, data) => renderView(id, view, { ...data, ...i18nLocals(localsOf(ctx)) }); @@ -307,7 +317,9 @@ export function createApp(options: AppOptions = {}): Server { } csrfMint.setCookie(); const result = (await match.route.handler(routeCtx)) ?? null; - if (anyResponseHooks) await runResponseHooks(plugins, contextFor, result); // observers; a throw → 500 + // The responding plugin observes its own route, params and all; the others get a plain + // context for their own id (never another plugin's params). + if (anyResponseHooks) await runResponseHooks(plugins, (id) => (id === match.plugin.id ? routeCtx : contextFor(id)), result); await sendResult(res, result, pluginViewsFor(routeCtx, match.plugin.id), carryLocale); return; } @@ -334,7 +346,14 @@ export function createApp(options: AppOptions = {}): Server { if (err instanceof GuardError) { if (res.headersSent) return void res.end(); if (err.location) return void res.writeHead(303, { location: err.location }).end(); - return void sendHtml(res, err.status, await renderPage("403", {})); + try { + return void sendHtml(res, err.status, await renderPage("403", {})); + } catch (renderErr) { + // Same last resort as the 500 branch below: a throw here would leave the socket open + // (this catch is the one that would have handled it), so end the response ourselves. + reqLog.error("error page render failed", { error: renderErr instanceof Error ? (renderErr.stack ?? renderErr.message) : String(renderErr) }); + return void res.writeHead(err.status, { "content-type": "text/plain; charset=utf-8" }).end("Forbidden"); + } } reqLog.error("unhandled request error", { error: err instanceof Error ? (err.stack ?? err.message) : String(err) }); if (res.headersSent) return void res.end(); // a partial body is already on the wire diff --git a/src/i18n/locales/en-US.ts b/src/i18n/locales/en-US.ts index 0129513..3353690 100644 --- a/src/i18n/locales/en-US.ts +++ b/src/i18n/locales/en-US.ts @@ -141,7 +141,7 @@ const messages = { "pagination.go": "Go", "pagination.label": "Pagination", "pagination.next": "Next page", - "pagination.of": "of", + "pagination.summary": "{{from}}–{{to}} of {{total}}", "pagination.previous": "Previous page", "pagination.rows": "Rows", diff --git a/src/i18n/locales/sv-SE.ts b/src/i18n/locales/sv-SE.ts index e09a926..d6ea294 100644 --- a/src/i18n/locales/sv-SE.ts +++ b/src/i18n/locales/sv-SE.ts @@ -127,7 +127,7 @@ const messages: CoreMessages = { "pagination.go": "Visa", "pagination.label": "Sidnavigering", "pagination.next": "Nästa sida", - "pagination.of": "av", + "pagination.summary": "{{from}}–{{to}} av {{total}}", "pagination.previous": "Föregående sida", "pagination.rows": "Rader", diff --git a/views/partials/pagination.ejs b/views/partials/pagination.ejs index f05e2f8..3378e1c 100644 --- a/views/partials/pagination.ejs +++ b/views/partials/pagination.ejs @@ -3,7 +3,7 @@ Page items are , inert ones (current/ellipsis/disabled) aren't. Config (all optional; never throws): label? nav aria-label (default "Pagination") - summary? { from, to, total } → "from–to of total" + summary? { from, to, total } → the pagination.summary message rows? { name, value?, options, label?, submitLabel?, action?, hidden? } rows-per-page form options: (number | { value, label })[]; hidden: { name, value }[] carries list state prev?, next? { href? } page step; omit href ⇒ disabled @@ -21,7 +21,7 @@ -%>