oficina / home Public

Review nits from #80 and #81: error page links and copy, Appearance return, browser tests open

On branch chore/review-nits

Branch chore/review-nits, cut from origin/main 83c5d16. Two commits: c5ed5ea, d595a02.

What changed, and how each one is tested

  1. The 500 page for a GET kept "Go to Oficina". Before, "Try again" (which points at the URL that just failed) replaced it. Now "Try again" sits beside it, in estilo-error's second link slot (ErrorPage.Back). Test: TestServerErrorKeepsTheWayHome. Before the fix it failed with no Go to Oficina link to /: <p rst-error-cta><a rst-btn="primary" href="/help/open-source?x=1">Try again</a> (the id.* case failed the same way).
  2. Appearance return_to is now the page's path only for a GET that drew a 200, and / otherwise. pageFor and account now take the status. Test: TestAppearanceReturnsOnlyToARealPage. Before the fix it failed with return_to "/no/such/page", want "/" and return_to "/appearance", want "/", and the POST-drawn 200, the GET 503 and the GET 500 all failed with return_to "/help/open-source".
  3. Hostile return_to cases added to TestAppearanceSetsTheTheme: //evil, /%09/evil, /%5c/evil, /%5C/evil, https://evil, https:%2F%2Fevil and /%0d%0a/evil, each posted as a raw form body, must 303 to /. These already passed. To show the test can catch a regression, I temporarily replaced the validator estilo actually calls with a bare /-prefix check. That validator is sessions.SafeReturn from github.com/carlosframework/rastrillo, the old path, because estilo's chrome.go still imports it. Five cases then failed, for example return_to=//evil: 303 to "//evil", want 303 to /. The replace was reverted afterwards.
  4. Error copy for every status. 400, 401, 403, 405, 413, 429 and 503 now have their own words, taken from rastrillo's locales/en.toml where it has some. Any other status gets a general "Something's not right" page. Test: TestErrorPageHasCopyForEveryStatus, which checks that each status has a heading and body, that headings are distinct, and that 418, 409 and 502 get the fallback. Before the fix it failed with NNN: no heading for every status except 404 and 500.
  5. assertion.html comment. On main, §9.3 now appears only once. But the comment still cited it for both the hidden card and the bar/sky placeholder, and one line ran long. §9.3 of 2026-09-23-shared-chrome.md is the hidden-card rule only. The placeholder comes from layout.html's hop handling (r3 C2), and the comment now says so. This is a comment-only change, so there is no test.
  6. Browser tests on a Mac. rastrillo's harness only knows Playwright's Linux layout. A new finder, playwrightChrome, also knows chrome-mac*/Google Chrome for Testing.app/... and the older Chromium.app, and picks the newest revision. The browser tests set RASTRILLO_CHROME from it when the variable is unset and no Chromium is on PATH. make test-browser now runs the whole package with -tags browser instead of -run 'OnTap'. Test: TestPlaywrightChromeFindsEveryCacheLayout, in an untagged file so the plain gate runs it. With only the Linux layout (the harness's behaviour) it failed got "" for all three macOS cases. Before this change, make test-browser failed with no chromium found. It now passes, and TestNoZoomOnTap really drives Chromium (about 21s).

The team-home lede and the footer "Source"/"Home" link are untouched, as asked.

Gate

make test passes, except that test-race stops on internal/provisioner on macOS ("build constraints exclude all Go files"; the package is //go:build linux). That happens on main too. The rest of test-race was run by hand and passed. make test-browser passes.

Review: not yet reviewed. A Fable whole-branch review is required before merge (AGENTS.md).

LLM

Fixes the six non-blocking findings from Fable's reviews of #80/#81. Every fix with a regression test was shown failing first. Needs a Fable review before merge.