From ed5a65bcd20e870d076abffa4446b9f6f5426349 Mon Sep 17 00:00:00 2001 From: Eugene Blikh Date: Sun, 9 Aug 2026 00:39:20 +0300 Subject: [PATCH] chrome: link the favicon through the shared head partial MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The icon href was a literal in the layout, so a build with no static tree — a test, a binary run out of a working copy — asked for /static/logo.svg once per page and got a 404 each time. It is the chrome's FaviconHref now: our own logo when this build ships one, checked the way the stylesheet already was, and NewService's built-in data: URI when it does not. Both s come from srht-head-links, so the guard against is written once. Also pins what the repo-list partial's optional fields do for a service that has no timestamp in its schema: a card with no muted footer, not "0001-01-01". --- web/router.go | 15 +++++++++++++++ web/templates/layout.html | 10 ++++++---- web/web_test.go | 35 +++++++++++++++++++++++++++++++++++ 3 files changed, 56 insertions(+), 4 deletions(-) diff --git a/web/router.go b/web/router.go index dcfa9c69b045ce1d8817d8d9db66e7be87d19a71..bb98aae467d6914f72457d3296d0d90225e7a8f9 100644 --- a/web/router.go +++ b/web/router.go @@ -2,6 +2,7 @@ package web import ( "fmt" + "html/template" "io/fs" "net/http" "os" @@ -35,6 +36,11 @@ const ( devStyleFile = "main.css" ) +// faviconFile is our own icon in the static tree. Unhashed, so it is linked by +// name and not through assets.Resolve — it changes about as often as the +// service is renamed. +const faviconFile = "logo.svg" + // app bundles the parsed templates, the shared chrome and the injected config. // Handlers are methods on *app so they share this state without a global. type app struct { @@ -127,6 +133,15 @@ func newApp(cfg Config) (*app, error) { chromeSvc := chrome.NewService(cfg.Conf, serviceName) chromeSvc.StyleHref = styleHref + // Our own logo when this build ships one, and NewService's built-in data: + // URI when it does not. The href used to be written into the layout, which + // meant a deployment without a static tree — a test, a binary run out of a + // working copy — requested a file that was not there once per page. The + // existence check is the stylesheet's, for the same reason. + if _, err := fs.Stat(static, faviconFile); err == nil { + chromeSvc.FaviconHref = template.URL(assets.NormalizePrefix(assets.DefaultPrefix) + faviconFile) + } + return &app{ cfg: cfg, pages: set, diff --git a/web/templates/layout.html b/web/templates/layout.html index 59a2a52a209923798252a1924d3f210fb9babbf0..ebeab4197b80b6e2ad5c7d326d9e171102e74a58 100644 --- a/web/templates/layout.html +++ b/web/templates/layout.html @@ -24,10 +24,12 @@ {{.Title}} - - {{/* Guarded rather than emitted empty: re-requests the page - it is on, which is one extra page load per page load. */}} - {{if .StyleHref}}{{end}} + {{/* The stylesheet and the favicon both come from the shared partial, which + guards each rather than emitting it empty: re-requests + the page it is on, one extra page load per page load. The favicon href + is the chrome's, so a build that ships no logo links ecore's built-in + data: URI instead of 404ing once per page. */}} + {{template "srht-head-links" .}} {{template "srht-env-banner" .}} diff --git a/web/web_test.go b/web/web_test.go index f1902a7e4ce4bc00c2ff8f4387bde747efa47b5a..18846754cb1be6b583cddd244c49d28bae3da92d 100644 --- a/web/web_test.go +++ b/web/web_test.go @@ -902,6 +902,41 @@ func TestStylesheetFallsBackToTheUnhashedBuildOnlyWhenItExists(t *testing.T) { assert.NotContains(t, bare.Body.String(), `rel="stylesheet"`) } +// The favicon is the chrome's href now, not a literal in the layout: our own +// logo when the build ships one, and ecore's built-in data: URI when it does +// not. The href used to be written into the template unconditionally, so a +// deployment without a static tree asked for a file that was not there once per +// page. +func TestFaviconIsOursWhenShippedAndTheBuiltInOtherwise(t *testing.T) { + dir := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(dir, "logo.svg"), []byte(""), 0o644)) + + shipped := newHarnessWithStatic(t, dir).do("GET", "/", nil, nil) + require.Equal(t, http.StatusOK, shipped.Code) + assert.Contains(t, shipped.Body.String(), `rel="icon" href="/static/logo.svg"`) + + bare := newHarnessWithStatic(t, t.TempDir()).do("GET", "/", nil, nil) + require.Equal(t, http.StatusOK, bare.Code) + assert.Contains(t, bare.Body.String(), `rel="icon" href="data:`, + "a build with no logo links the shared data: URI, never a 404") + assert.NotContains(t, bare.Body.String(), `href="/static/logo.svg"`) +} + +// The listing partial's Updated and Meta are optional in practice and not only +// in ecore's doc comment: this service has no timestamp in its schema, leaves +// both zero, and must get a card with no muted footer rather than "0001-01-01". +func TestDatabaseListingRendersNoTimestampBlock(t *testing.T) { + h := newHarness(t) + h.store.add(&core.Repo{Name: "pub", OwnerID: 1, OwnerName: "alice", Path: "/p", Visibility: core.VisibilityPublic}) + + rec := h.do("GET", "/~alice", nil, nil) + require.Equal(t, http.StatusOK, rec.Code) + body := rec.Body.String() + assert.Contains(t, body, "/~alice/pub") + assert.NotContains(t, body, "0001-01-01", "a zero Updated must render nothing at all") + assert.NotContains(t, body, ``) +} + func TestLogAndTablePages(t *testing.T) { h := newHarness(t) h.store.add(&core.Repo{Name: "db", OwnerID: 1, OwnerName: "alice", Path: "/d", Visibility: core.VisibilityPublic})