amadan

oficina / home Public

Review nits from the shared-chrome and rastrillo-path reviews

Fixes for the non-blocking findings on the merged branches (see this
branch's discussion). The coordinating session checked them, on Paul's
standing instruction to roll in fixes.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Squashed from chore/review-nits into main.
claude 3bc1cffd95914936162d7c8205e383e91b7b91b4 parent 83c5d16
7 files changed, +359 −21
  • Makefile +7 −4
  • internal/home/chrome.go +51 −13
  • internal/home/chromefinder_test.go +103 −0
  • internal/home/render.go +1 −1
  • internal/home/shared_chrome_test.go +166 −0
  • internal/home/templates/assertion.html +5 −3
  • internal/home/zoom_browser_test.go +26 −0

CI passed — run details

CI log 
amadan: .amadan/timeout: this job's budget is 15m0s (this runner allows up to 30m0s)
=== vet ===
CGO_ENABLED=0 go vet ./...
CGO_ENABLED=0 go vet -tags browser ./internal/home/
cd handoff && CGO_ENABLED=0 go vet ./...
cd roster && CGO_ENABLED=0 go vet ./...
cd connect && CGO_ENABLED=0 go vet ./...
=== fmt ===
=== build ===
CGO_ENABLED=0 go build ./...
CGO_ENABLED=0 GOOS=linux GOARCH=arm64 go build ./cmd/home ./cmd/oficina ./internal/operatorlaunch
cd handoff && CGO_ENABLED=0 go build ./...
cd roster && CGO_ENABLED=0 go build ./...
cd connect && CGO_ENABLED=0 go build ./...
=== go-test ===
CGO_ENABLED=0 go test -count=1 ./...
?   	amadan.net/oficina/home/cmd/home	[no test files]
ok  	amadan.net/oficina/home/cmd/home-placeholder	0.629s
?   	amadan.net/oficina/home/cmd/oficina	[no test files]
ok  	amadan.net/oficina/home/internal/guardstest	0.035s
ok  	amadan.net/oficina/home/internal/home	12.244s
ok  	amadan.net/oficina/home/internal/home/directorydata	1.118s
ok  	amadan.net/oficina/home/internal/home/logtest	0.007s
ok  	amadan.net/oficina/home/internal/home/seal	0.006s
ok  	amadan.net/oficina/home/internal/home/sealtest	0.015s
ok  	amadan.net/oficina/home/internal/operator	5.093s
ok  	amadan.net/oficina/home/internal/operatorlaunch	4.181s
ok  	amadan.net/oficina/home/internal/provisioner	43.266s
cd handoff && CGO_ENABLED=0 go test -count=1 ./...
ok  	amadan.net/oficina/home/handoff	0.326s
ok  	amadan.net/oficina/home/handoff/seal	0.002s
cd roster && CGO_ENABLED=0 go test -count=1 ./...
ok  	amadan.net/oficina/home/roster	5.080s
cd connect && CGO_ENABLED=0 go test -count=1 ./...
ok  	amadan.net/oficina/home/connect	0.059s
ok  	amadan.net/oficina/home/connect/linkstore	0.179s
=== race ===
go test -race -count=1 ./internal/home/ ./internal/operator/ ./internal/operatorlaunch/ ./internal/provisioner/ ./internal/home/directorydata/
ok  	amadan.net/oficina/home/internal/home	112.985s
ok  	amadan.net/oficina/home/internal/operator	64.617s
ok  	amadan.net/oficina/home/internal/operatorlaunch	22.333s
ok  	amadan.net/oficina/home/internal/provisioner	286.786s
ok  	amadan.net/oficina/home/internal/home/directorydata	15.930s
cd handoff && CGO_ENABLED=1 go test -race -count=1 ./...
ok  	amadan.net/oficina/home/handoff	9.229s
ok  	amadan.net/oficina/home/handoff/seal	1.009s
cd roster && CGO_ENABLED=1 go test -race -count=1 ./...
ok  	amadan.net/oficina/home/roster	6.617s
cd connect && CGO_ENABLED=1 go test -race -count=1 ./...
ok  	amadan.net/oficina/home/connect	1.633s
ok  	amadan.net/oficina/home/connect/linkstore	3.776s
diff --git a/Makefile b/Makefile
index b3c941b..47c04de 100644
--- a/Makefile
+++ b/Makefile
@@ -126,13 +126,16 @@ test-race:
cd roster && CGO_ENABLED=1 go test -race -count=1 ./...
cd connect && CGO_ENABLED=1 go test -race -count=1 ./...
-# The browser checks, in a real Chromium (on PATH, RASTRILLO_CHROME, or a
-# Playwright cache). Deliberately NOT part of `test`: the gate CI runs on
+# The browser checks, in a real Chromium (RASTRILLO_CHROME, on PATH, or
+# the newest in a Playwright cache: Linux's ~/.cache or macOS's
+# ~/Library/Caches). Deliberately NOT part of `test`: the gate CI runs on
# every push must not need a browser. Run it before shipping a change to
# the CSS or to rastrillo's pin — TestNoZoomOnTap is the iOS zoom-on-tap
-# check. test-vet vets these files with the tag, so they keep compiling.
+# check. Every test in the package runs, the browser ones (the `browser`
+# tag) with them, so none is left out by a -run filter that predates it.
+# test-vet vets these files with the tag, so they keep compiling.
test-browser:
- CGO_ENABLED=0 go test -tags browser -count=1 -run 'OnTap' ./internal/home/
+ CGO_ENABLED=0 go test -tags browser -count=1 ./internal/home/
.PHONY: credits
credits:
diff --git a/internal/home/chrome.go b/internal/home/chrome.go
index beb34b0..9915db0 100644
--- a/internal/home/chrome.go
+++ b/internal/home/chrome.go
@@ -58,7 +58,7 @@ func (a *App) onCredential(r *http.Request) bool {
// sign-in, sent and signed-out pages, whose corner stays empty. hop marks
// a page that moves on by itself: its bar is the brand alone, as plain
// text, so the page still holds exactly one link, the way on.
-func (a *App) pageFor(r *http.Request, data any, door, hop bool) page {
+func (a *App) pageFor(r *http.Request, status int, data any, door, hop bool) page {
p := page{Data: data}
brand := chrome.App{Name: suiteName, Slug: "oficina", Home: "/", Icon: versioned("/static/brand/icon.svg")}
if hop {
@@ -74,7 +74,7 @@ func (a *App) pageFor(r *http.Request, data any, door, hop bool) page {
p.Theme = chrome.ThemeAttr(chrome.Appearance(r))
p.Chrome = chrome.Bar{App: brand, Labels: barLabels}
if _, ok := sessions.Current(r); ok {
- p.Chrome.Account = a.account(r)
+ p.Chrome.Account = a.account(r, status)
} else if !door {
p.Chrome.SignIn = &chrome.Link{Href: signInHref(r), Label: "Sign in"}
}
@@ -84,7 +84,7 @@ func (a *App) pageFor(r *http.Request, data any, door, hop bool) page {
// account is the directory's account menu. The directory cannot read the
// address (it holds a person ref, never an address: the one rule), so the
// identity row says only "Signed in" and the trigger is the person glyph.
-func (a *App) account(r *http.Request) *chrome.Account {
+func (a *App) account(r *http.Request, status int) *chrome.Account {
return &chrome.Account{
TriggerLabel: "Account",
Labels: accountLabels,
@@ -93,10 +93,8 @@ func (a *App) account(r *http.Request) *chrome.Account {
{Href: a.origins.Credential + "/passkey/codes", Label: "Recovery codes"},
},
Appearance: &chrome.AppearanceMenu{
- Action: "/appearance",
- // The path alone: no directory page reads its query, and a
- // query is where a crafted link would carry its text.
- Return: r.URL.EscapedPath(),
+ Action: "/appearance",
+ Return: appearanceReturn(r, status),
Options: chrome.AppearanceOptions(chrome.Appearance(r), "Light", "Dark", "System"),
},
Help: []menu.Item{{Href: "/help/open-source", Label: "Open source"}},
@@ -104,6 +102,20 @@ func (a *App) account(r *http.Request) *chrome.Account {
}
}
+// appearanceReturn is where choosing an appearance comes back to: this
+// page, if it is one a GET can come back to, otherwise the directory. Only
+// a GET that drew a 200 qualifies. A 404's or an error page's own path is
+// known not to work, and the path of a page drawn for a POST is POST-only,
+// so a 303 back to it would land on a 404. The path alone: no directory
+// page reads its query, and a query is where a crafted link would carry
+// its text.
+func appearanceReturn(r *http.Request, status int) string {
+ if r.Method != http.MethodGet || status != http.StatusOK {
+ return "/"
+ }
+ return r.URL.EscapedPath()
+}
+
// signInHref starts the directory's sign-in and comes back to this page.
// An error page comes back to the directory instead: its own URL is the
// one thing known not to exist.
@@ -130,10 +142,34 @@ func wantsHTML(r *http.Request) bool {
return strings.Contains(r.Header.Get("Accept"), "text/html")
}
-// errorCopy is estilo spec §3.2's copy for the statuses Home answers.
-var errorCopy = map[int]struct{ page, heading, body string }{
- http.StatusNotFound: {"Page not found", "We can’t find that page", "The link may be out of date, or the page may have moved."},
- http.StatusInternalServerError: {"Something went wrong", "Something went wrong", "Try again in a moment. If it keeps happening, quote the reference below."},
+// errorWords is one status's error page: the tab title, the heading and
+// the line under it.
+type errorWords struct{ page, heading, body string }
+
+// errorCopy is estilo spec §3.2's copy for the statuses Home answers, and
+// rastrillo's error copy (locales/en.toml) for the rest an HTTP handler
+// can plausibly be asked to draw. Any other status gets errorFallback.
+var errorCopy = map[int]errorWords{
+ http.StatusBadRequest: {"Bad request", "That didn’t go through", "Something in what was sent wasn’t right. Go back and try again."},
+ http.StatusUnauthorized: {"Sign in needed", "You need to sign in", "This page is only for someone who is signed in. Sign in, then try again."},
+ http.StatusForbidden: {"No access", "You can’t see this", "Your account doesn’t have access here. If you think it should, ask whoever runs this site."},
+ http.StatusNotFound: {"Page not found", "We can’t find that page", "The link may be out of date, or the page may have moved."},
+ http.StatusMethodNotAllowed: {"Not available", "That can’t be done here", "This page doesn’t take that kind of request. Go back and try again from the page itself."},
+ http.StatusRequestEntityTooLarge: {"Too large", "That was too large", "What was sent is bigger than this page accepts. Try again with less."},
+ http.StatusTooManyRequests: {"Too many tries", "Too many tries", "Wait a minute, then try again."},
+ http.StatusInternalServerError: {"Something went wrong", "Something went wrong", "Try again in a moment. If it keeps happening, quote the reference below."},
+ http.StatusServiceUnavailable: {"Briefly unavailable", "We’re briefly unavailable", "Oficina is being updated or is busy. Try again in a minute."},
+}
+
+// errorFallback is the error page for a status with no words of its own.
+var errorFallback = errorWords{"Something’s not right", "Something’s not right", "That request couldn’t be completed. Go back and try again."}
+
+// errorCopyFor is status's words, or the general ones.
+func errorCopyFor(status int) errorWords {
+ if c, ok := errorCopy[status]; ok {
+ return c
+ }
+ return errorFallback
}
// errorPage answers status as estilo's error page, under the bar, for a
@@ -145,7 +181,7 @@ func (a *App) errorPage(w http.ResponseWriter, r *http.Request, status int) {
http.Error(w, strings.ToLower(http.StatusText(status)), status)
return
}
- c := errorCopy[status]
+ c := errorCopyFor(status)
home := chrome.Link{Href: "/", Label: "Go to Oficina"}
if a.onCredential(r) {
home.Href = a.origins.Directory + "/"
@@ -155,7 +191,9 @@ func (a *App) errorPage(w http.ResponseWriter, r *http.Request, status int) {
data.Ref, data.RefLabel = rastrillo.NewRef(), "Reference:"
a.logger.Error("error page", "ref", data.Ref, "path", r.URL.Path)
if r.Method == http.MethodGet {
- data.Home = chrome.Link{Href: r.URL.RequestURI(), Label: "Try again"}
+ // Beside the way home, not instead of it: the way home is the
+ // one link known to work.
+ data.Back = &chrome.Link{Href: r.URL.RequestURI(), Label: "Try again"}
}
}
a.renderPage(w, r, status, "error.html", errorView{chrome.Title(c.page, suiteName), data})
diff --git a/internal/home/chromefinder_test.go b/internal/home/chromefinder_test.go
new file mode 100644
index 0000000..32c1e97
--- /dev/null
+++ b/internal/home/chromefinder_test.go
@@ -0,0 +1,103 @@
+package home
+
+import (
+ "os"
+ "path/filepath"
+ "sort"
+ "strconv"
+ "strings"
+ "testing"
+)
+
+// playwrightChrome is the newest Chromium in a Playwright browser cache
+// under home, or "". rastrillo's harness looks in Linux's cache alone
+// (~/.cache/ms-playwright/chromium-*/chrome-linux64/chrome), so on a Mac
+// `make test-browser` found nothing unless RASTRILLO_CHROME was set by
+// hand. The browser tests (zoom_browser_test.go) hand it this. It lives
+// in an untagged file so the plain suite tests the finder itself.
+func playwrightChrome(home string) string {
+ var hits []string
+ for _, layout := range []string{
+ // Linux, as the harness already knows it.
+ filepath.Join(".cache", "ms-playwright", "chromium-*", "chrome-linux64", "chrome"),
+ // macOS since Playwright moved to Chrome for Testing.
+ filepath.Join("Library", "Caches", "ms-playwright", "chromium-*", "chrome-mac*", "Google Chrome for Testing.app", "Contents", "MacOS", "Google Chrome for Testing"),
+ // macOS before it.
+ filepath.Join("Library", "Caches", "ms-playwright", "chromium-*", "chrome-mac*", "Chromium.app", "Contents", "MacOS", "Chromium"),
+ } {
+ m, _ := filepath.Glob(filepath.Join(home, layout))
+ hits = append(hits, m...)
+ }
+ if len(hits) == 0 {
+ return ""
+ }
+ sort.SliceStable(hits, func(i, j int) bool { return chromiumRevision(hits[i]) > chromiumRevision(hits[j]) })
+ return hits[0]
+}
+
+// chromiumRevision is the N of the chromium-N directory in path, or -1.
+func chromiumRevision(path string) int {
+ for _, part := range strings.Split(filepath.ToSlash(path), "/") {
+ if rest, ok := strings.CutPrefix(part, "chromium-"); ok {
+ if n, err := strconv.Atoi(rest); err == nil {
+ return n
+ }
+ }
+ }
+ return -1
+}
+
+func TestPlaywrightChromeFindsEveryCacheLayout(t *testing.T) {
+ touch := func(t *testing.T, path string) {
+ t.Helper()
+ if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil {
+ t.Fatal(err)
+ }
+ if err := os.WriteFile(path, nil, 0o755); err != nil {
+ t.Fatal(err)
+ }
+ }
+ cft := func(home, rev, arch string) string {
+ return filepath.Join(home, "Library", "Caches", "ms-playwright", "chromium-"+rev, "chrome-"+arch, "Google Chrome for Testing.app", "Contents", "MacOS", "Google Chrome for Testing")
+ }
+
+ t.Run("none", func(t *testing.T) {
+ if got := playwrightChrome(t.TempDir()); got != "" {
+ t.Errorf("empty home: %q", got)
+ }
+ })
+ t.Run("macOS, Chrome for Testing", func(t *testing.T) {
+ home := t.TempDir()
+ want := cft(home, "1234", "mac-arm64")
+ touch(t, want)
+ if got := playwrightChrome(home); got != want {
+ t.Errorf("got %q, want %q", got, want)
+ }
+ })
+ t.Run("macOS, newest revision wins", func(t *testing.T) {
+ home := t.TempDir()
+ touch(t, filepath.Join(home, "Library", "Caches", "ms-playwright", "chromium-1148", "chrome-mac", "Chromium.app", "Contents", "MacOS", "Chromium"))
+ touch(t, cft(home, "1228", "mac-arm64"))
+ want := cft(home, "1234", "mac-arm64")
+ touch(t, want)
+ if got := playwrightChrome(home); got != want {
+ t.Errorf("got %q, want %q", got, want)
+ }
+ })
+ t.Run("macOS, older Chromium.app", func(t *testing.T) {
+ home := t.TempDir()
+ want := filepath.Join(home, "Library", "Caches", "ms-playwright", "chromium-1148", "chrome-mac", "Chromium.app", "Contents", "MacOS", "Chromium")
+ touch(t, want)
+ if got := playwrightChrome(home); got != want {
+ t.Errorf("got %q, want %q", got, want)
+ }
+ })
+ t.Run("Linux", func(t *testing.T) {
+ home := t.TempDir()
+ want := filepath.Join(home, ".cache", "ms-playwright", "chromium-1234", "chrome-linux64", "chrome")
+ touch(t, want)
+ if got := playwrightChrome(home); got != want {
+ t.Errorf("got %q, want %q", got, want)
+ }
+ })
+}
diff --git a/internal/home/render.go b/internal/home/render.go
index f63ed7f..2ac21ab 100644
--- a/internal/home/render.go
+++ b/internal/home/render.go
@@ -108,7 +108,7 @@ func (a *App) renderPage(w http.ResponseWriter, r *http.Request, status int, nam
return
}
var buf bytes.Buffer
- if err := t.ExecuteTemplate(&buf, "layout", a.pageFor(r, data, doorPages[name], hopPages[name])); err != nil {
+ if err := t.ExecuteTemplate(&buf, "layout", a.pageFor(r, status, data, doorPages[name], hopPages[name])); err != nil {
a.logger.Error("render", "page", name, "err", err)
a.renderFailed(w, r, name)
return
diff --git a/internal/home/shared_chrome_test.go b/internal/home/shared_chrome_test.go
index 95feba1..8d9890a 100644
--- a/internal/home/shared_chrome_test.go
+++ b/internal/home/shared_chrome_test.go
@@ -168,6 +168,172 @@ func TestAppearanceSetsTheTheme(t *testing.T) {
if !strings.Contains(body, `value="dark" aria-pressed="true"`) {
t.Error("Dark is not shown as the current appearance")
}
+
+ // return_to is the attacker's half of the form: anything that would
+ // leave the directory goes to "/" instead. Raw form bodies, as a
+ // hostile page would post them: %09 and %5c decode to a tab and a
+ // backslash, which browsers strip or fold into "/" before navigating.
+ for _, hostile := range []string{"//evil", "/%09/evil", "/%5c/evil", "/%5C/evil", "https://evil", "https:%2F%2Fevil", "/%0d%0a/evil"} {
+ rec := f.request("POST", "/appearance", directoryOrigin, "mode=dark&return_to="+hostile, person)
+ if rec.Code != http.StatusSeeOther || rec.Header().Get("Location") != "/" {
+ t.Errorf("return_to=%s: %d to %q, want 303 to /", hostile, rec.Code, rec.Header().Get("Location"))
+ }
+ }
+}
+
+// Appearance comes back to the page it was chosen on only if that page
+// is there to come back to: a GET that drew a 200. A 404's own path, an
+// error page's, or the path of a POST that drew a page would 303 back to
+// a page that is not there (a POST-only path answers a GET with a 404).
+func TestAppearanceReturnsOnlyToARealPage(t *testing.T) {
+ f := newMountedChooser(t)
+ person := f.session(t, chooserPerson)
+ directory := f.p.directory.Router()
+ returnTo := regexp.MustCompile(`name="return_to" value="([^"]*)"`)
+ for _, c := range []struct {
+ name, target, want string
+ }{
+ {"the directory", "/", "/"},
+ {"open source", "/help/open-source", "/help/open-source"},
+ {"a 404", "/no/such/page", "/"},
+ {"a GET to a POST-only path", "/appearance", "/"},
+ } {
+ t.Run(c.name, func(t *testing.T) {
+ body := chromeGet(directory, directoryOrigin+c.target, acceptHTML, person).Body.String()
+ m := returnTo.FindStringSubmatch(body)
+ if m == nil {
+ t.Fatalf("no Appearance return_to on %s", c.target)
+ }
+ if m[1] != c.want {
+ t.Errorf("return_to %q, want %q", m[1], c.want)
+ }
+ })
+ }
+ // A page drawn in answer to a POST, or with any status but 200.
+ for _, c := range []struct {
+ method string
+ status int
+ }{
+ {http.MethodPost, http.StatusOK},
+ {http.MethodGet, http.StatusServiceUnavailable},
+ {http.MethodGet, http.StatusInternalServerError},
+ } {
+ r := httptest.NewRequest(c.method, directoryOrigin+"/help/open-source", nil)
+ r.Header.Set("Accept", acceptHTML)
+ r.AddCookie(person)
+ var got string
+ f.p.directory.sessions.Middleware(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+ got = f.p.directory.account(r, c.status).Appearance.Return
+ })).ServeHTTP(httptest.NewRecorder(), r)
+ if got != "/" {
+ t.Errorf("%s drawing a %d: return_to %q, want \"/\"", c.method, c.status, got)
+ }
+ }
+}
+
+// A 500 for a GET offers to try the page again beside the way home; it
+// does not replace the way home, which is the one link known to work.
+func TestServerErrorKeepsTheWayHome(t *testing.T) {
+ f := newMountedChooser(t)
+ for _, c := range []struct {
+ name string
+ app *App
+ method string
+ target string
+ home string
+ tryAgain bool
+ }{
+ {"directory, GET", f.p.directory, http.MethodGet, directoryOrigin + "/help/open-source?x=1", "/", true},
+ {"directory, POST", f.p.directory, http.MethodPost, directoryOrigin + "/appearance", "/", false},
+ {"id.*, GET", f.p.credential.App, http.MethodGet, Origin + "/passkey/codes", directoryOrigin + "/", true},
+ } {
+ t.Run(c.name, func(t *testing.T) {
+ r := httptest.NewRequest(c.method, c.target, nil)
+ r.Header.Set("Accept", acceptHTML)
+ rec := httptest.NewRecorder()
+ c.app.serverError(rec, r)
+ body := rec.Body.String()
+ if rec.Code != http.StatusInternalServerError {
+ t.Fatalf("status %d", rec.Code)
+ }
+ if !strings.Contains(body, `<a rst-btn="primary" href="`+c.home+`">Go to Oficina</a>`) {
+ t.Errorf("no Go to Oficina link to %s: %s", c.home, errorCTA(body))
+ }
+ again := `<a rst-btn href="` + strings.TrimPrefix(strings.TrimPrefix(c.target, directoryOrigin), Origin) + `">Try again</a>`
+ if got := strings.Contains(body, ">Try again</a>"); got != c.tryAgain {
+ t.Errorf("Try again shown %v, want %v: %s", got, c.tryAgain, errorCTA(body))
+ }
+ if c.tryAgain && !strings.Contains(body, again) {
+ t.Errorf("Try again does not retry the page (want %s): %s", again, errorCTA(body))
+ }
+ })
+ }
+}
+
+// errorCTA is the error page's links, for a failure message.
+func errorCTA(body string) string {
+ if i := strings.Index(body, "<p rst-error-cta>"); i >= 0 {
+ end := strings.Index(body[i:], "</p>")
+ if end > 0 {
+ return body[i : i+end]
+ }
+ }
+ return "(no rst-error-cta)"
+}
+
+// Every status the error page can be asked for has its own words; one
+// nobody wrote words for gets a general page, never a blank heading.
+func TestErrorPageHasCopyForEveryStatus(t *testing.T) {
+ f := newMountedChooser(t)
+ headings := map[string]int{}
+ for _, status := range []int{
+ http.StatusBadRequest, http.StatusUnauthorized, http.StatusForbidden,
+ http.StatusNotFound, http.StatusMethodNotAllowed, http.StatusRequestEntityTooLarge,
+ http.StatusTooManyRequests, http.StatusInternalServerError, http.StatusServiceUnavailable,
+ } {
+ heading := checkErrorCopy(t, f.p.directory, status)
+ if other, ok := headings[heading]; ok {
+ t.Errorf("%d and %d share the heading %q", status, other, heading)
+ }
+ headings[heading] = status
+ }
+ // Statuses with no words of their own: a general page.
+ for _, status := range []int{http.StatusTeapot, http.StatusConflict, http.StatusBadGateway} {
+ checkErrorCopy(t, f.p.directory, status)
+ }
+}
+
+var (
+ errorHeading = regexp.MustCompile(`<h1 rst-error-title>([^<]*)</h1>`)
+ errorBody = regexp.MustCompile(`<p rst-error-body>([^<]*)</p>`)
+)
+
+// checkErrorCopy draws status's error page and fails on any part of its
+// copy left blank. It returns the heading.
+func checkErrorCopy(t *testing.T, a *App, status int) string {
+ t.Helper()
+ r := httptest.NewRequest(http.MethodGet, directoryOrigin+"/x", nil)
+ r.Header.Set("Accept", acceptHTML)
+ rec := httptest.NewRecorder()
+ a.errorPage(rec, r, status)
+ body := rec.Body.String()
+ if rec.Code != status {
+ t.Errorf("%d: drew a %d", status, rec.Code)
+ }
+ title := titleOf(body)
+ h := errorHeading.FindStringSubmatch(body)
+ b := errorBody.FindStringSubmatch(body)
+ if title == "" || strings.HasPrefix(title, " ·") || strings.HasPrefix(title, "·") {
+ t.Errorf("%d: tab title %q", status, title)
+ }
+ if h == nil || strings.TrimSpace(h[1]) == "" {
+ t.Errorf("%d: no heading", status)
+ return ""
+ }
+ if b == nil || strings.TrimSpace(b[1]) == "" {
+ t.Errorf("%d: no body copy", status)
+ }
+ return h[1]
}
// estilo's error page on both origins, for a browser; everything else
diff --git a/internal/home/templates/assertion.html b/internal/home/templates/assertion.html
index 3c7887a..723462b 100644
--- a/internal/home/templates/assertion.html
+++ b/internal/home/templates/assertion.html
@@ -1,9 +1,11 @@
{{define "title"}}Continue to Oficina · Oficina{{end}}
{{define "head"}}
{{/* The page submits itself (static/assertion.js), so its card stays
- hidden for 1.5 s and shows only if the submit stalls; the bar and the
- sky's placeholder show at once (layout.html, hop). With scripts off nothing submits, so noscript shows it
- at once. Spec shared-chrome §9.3. */}}
+ hidden for 1.5 s and shows only if the submit stalls. With scripts
+ off nothing submits, so noscript shows it at once. That rule is spec
+ shared-chrome §9.3. The bar and the sky's placeholder sit outside the
+ card and show at once, so the hop is never a blank page: that is
+ layout.html's hop handling (review r3 C2), not §9.3. */}}
<style>
[estilo-handoff] { animation: estilo-reveal 0s 1.5s both; }
@keyframes estilo-reveal { from { visibility: hidden } to { visibility: visible } }
diff --git a/internal/home/zoom_browser_test.go b/internal/home/zoom_browser_test.go
index 9fb14ad..23b5f9d 100644
--- a/internal/home/zoom_browser_test.go
+++ b/internal/home/zoom_browser_test.go
@@ -14,6 +14,8 @@ import (
"context"
"fmt"
"net/http"
+ "os"
+ "os/exec"
"strings"
"testing"
"time"
@@ -29,6 +31,29 @@ import (
// zooms for them.
const textControls = `input:not([type=checkbox]):not([type=radio]):not([type=hidden]):not([type=file]):not([type=submit]):not([type=button]):not([type=color]):not([type=range]), select, textarea`
+// useChrome points rastrillo's harness at a Chromium it would not find by
+// itself: when RASTRILLO_CHROME is unset and no Chromium is on PATH, the
+// newest one in a Playwright cache, the macOS layouts included
+// (playwrightChrome, chromefinder_test.go). Call it before harness.New.
+func useChrome(t *testing.T) {
+ t.Helper()
+ if os.Getenv("RASTRILLO_CHROME") != "" {
+ return
+ }
+ for _, name := range []string{"chromium", "chromium-browser", "google-chrome", "google-chrome-stable"} {
+ if _, err := exec.LookPath(name); err == nil {
+ return
+ }
+ }
+ home, err := os.UserHomeDir()
+ if err != nil {
+ return
+ }
+ if p := playwrightChrome(home); p != "" {
+ t.Setenv("RASTRILLO_CHROME", p)
+ }
+}
+
// asHost serves h as if every request had arrived for host. The rig's
// browser talks to http://localhost:<port>, and Home answers only its two
// configured hosts (serveKnownHost); this is the browser-side half of the
@@ -83,6 +108,7 @@ func TestNoZoomOnTap(t *testing.T) {
{docsEntryPath, `button[name="team"]`},
}},
}
+ useChrome(t)
for _, host := range hosts {
rig := harness.New(t, host.handler)
run := func(what string, actions ...chromedp.Action) {