From 3d0cb7d98c2eef92a17e5ab6fd09108350e92bb6 Mon Sep 17 00:00:00 2001 From: Jonathan Miller Date: Sun, 12 Jul 2026 18:45:11 -0500 Subject: [PATCH 1/2] fix(branding): app icon follows the Nav Icon upload The app icon (logo.png) is the PWA/web-manifest icon and the navbar fallback, but had no Branding control, so it stayed the shipped default even on a fully branded instance. Uploading the Nav Icon now also writes logo.png (single io.MultiWriter pass), and resetting the Nav Icon reverts both to the built-in default. Closes #773 --- CHANGELOG.md | 1 + routers/web/admin/branding.go | 74 +++++++++++++++++++++++++---------- templates/admin/branding.tmpl | 2 +- 3 files changed, 55 insertions(+), 22 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ccb77732ab..a288494d6b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -63,6 +63,7 @@ - Cherry-pick upstream v1.26.4: walk git log context error handling — regression fix (#38185) ### Fixed +- Admin Branding: uploading a custom **Nav Icon** now also sets the **app icon** (`logo.png`, the PWA / web-manifest icon and navbar fallback), so a branded instance shows its own installable app icon instead of the shipped default; resetting the Nav Icon reverts both (#773) - Fork server binary now compiles: `routers/api/v1/api.go` called `organization.HasOrgOrUserVisible`, which had been renamed to `IsOwnerVisibleToDoer`; the one missed call site broke `go build` of the entire `routers/api/v1` package (CI's Lint & Validate does not run a full build, so it went unnoticed) (#735) - Dev deploy workflow: the build/deploy step referenced runner-side values as `\$TAG` / `\$REGISTRY_TOKEN` inside an unquoted SSH heredoc, deferring expansion to the remote shell where those names are unset — the Docker tag collapsed to an empty `mokogitea:` and every dev deploy failed with `invalid reference format`. Runner values are now injected via an ssh env-prefix and the heredoc is quoted so each `$var` expands in exactly one place (#737) - Repaired unit-test compile and `go vet` failures: `CryptoRandomInt/String/Bytes` now return two values (updated `modules/util/util_test.go`), removed a redundant `&&` condition in `issue_comment.go`, and cleaned up isolated integration-test compile errors (#736) diff --git a/routers/web/admin/branding.go b/routers/web/admin/branding.go index 3ef0020640..f71f5666d6 100644 --- a/routers/web/admin/branding.go +++ b/routers/web/admin/branding.go @@ -137,29 +137,41 @@ func BrandingUpload(ctx *context.Context) { return } - destPath := filepath.Join(imgDir, filename) - dest, err := os.Create(destPath) - if err != nil { - ctx.Flash.Error("Failed to save image") - log.Error("Create %s: %v", destPath, err) - ctx.Redirect(setting.AppSubURL + "/-/admin/branding") - return + // The nav icon doubles as the app icon (logo.png) — used as the PWA / web + // manifest icon and the navbar fallback — so branding the nav icon brands + // the app icon too. Write the uploaded image to both in a single pass. See #773. + targets := []string{filename} + if imageType == "nav-icon" { + targets = append(targets, "logo.png") } - defer dest.Close() - if _, err := io.Copy(dest, file); err != nil { + var writers []io.Writer + for _, name := range targets { + destPath := filepath.Join(imgDir, name) + dest, err := os.Create(destPath) + if err != nil { + ctx.Flash.Error("Failed to save image") + log.Error("Create %s: %v", destPath, err) + ctx.Redirect(setting.AppSubURL + "/-/admin/branding") + return + } + defer dest.Close() + writers = append(writers, dest) + + // Remove any SVG override so the uploaded PNG takes effect. + svgPath := filepath.Join(imgDir, name[:len(name)-4]+".svg") + if fileExists(svgPath) { + os.Remove(svgPath) + } + } + + if _, err := io.Copy(io.MultiWriter(writers...), file); err != nil { ctx.Flash.Error("Failed to write image") - log.Error("Copy to %s: %v", destPath, err) + log.Error("Copy branding image %s: %v", filename, err) ctx.Redirect(setting.AppSubURL + "/-/admin/branding") return } - // Remove SVG override if present - svgPath := filepath.Join(imgDir, filename[:len(filename)-4]+".svg") - if fileExists(svgPath) { - os.Remove(svgPath) - } - ctx.Flash.Success("Branding image updated: " + imageType) log.Info("Branding image uploaded: %s (%d bytes)", filename, header.Size) ctx.Redirect(setting.AppSubURL + "/-/admin/branding") @@ -183,16 +195,36 @@ func BrandingReset(ctx *context.Context) { return } - path := filepath.Join(brandingImageDir(), filename) - if fileExists(path) { + // The nav icon also sets the app icon (logo.png), so resetting it reverts + // both back to the built-in default. See #773. + targets := []string{filename} + if imageType == "nav-icon" { + targets = append(targets, "logo.png") + } + + removedAny := false + failed := false + for _, name := range targets { + path := filepath.Join(brandingImageDir(), name) + if !fileExists(path) { + continue + } if err := os.Remove(path); err != nil { ctx.Flash.Error("Failed to remove custom image") log.Error("Remove %s: %v", path, err) + failed = true } else { - ctx.Flash.Success("Reset to default: " + imageType) - log.Info("Branding reset to default: %s", filename) + removedAny = true + log.Info("Branding reset to default: %s", name) } - } else { + } + + switch { + case failed: + // error flash already set + case removedAny: + ctx.Flash.Success("Reset to default: " + imageType) + default: ctx.Flash.Info("Already using default: " + imageType) } diff --git a/templates/admin/branding.tmpl b/templates/admin/branding.tmpl index 9a71ef9214..0bf72e9acc 100644 --- a/templates/admin/branding.tmpl +++ b/templates/admin/branding.tmpl @@ -54,7 +54,7 @@ Nav Icon {{if .HasNavIcon}}Custom{{else}}Default{{end}} -
Top-left corner across all pages. Square, 30x30px.
+
Top-left corner across all pages. Square, 30x30px. Also sets the app icon (PWA / web manifest); use at least 512x512px for a crisp installed icon.
-- 2.52.0 From f4be31183fd2aaba2ac16c43ce5f868c30e556ad Mon Sep 17 00:00:00 2001 From: Jonathan Miller Date: Sun, 12 Jul 2026 19:02:14 -0500 Subject: [PATCH 2/2] refactor(branding): write each icon target independently on upload Rewind the uploaded stream between targets and write/close each file in turn instead of a shared io.MultiWriter, so a failure writing a later target (e.g. logo.png) can never leave an earlier one (logo-small.png) truncated. Addresses review feedback on #774. --- routers/web/admin/branding.go | 32 +++++++++++++++++++++----------- 1 file changed, 21 insertions(+), 11 deletions(-) diff --git a/routers/web/admin/branding.go b/routers/web/admin/branding.go index f71f5666d6..7ef835ccf8 100644 --- a/routers/web/admin/branding.go +++ b/routers/web/admin/branding.go @@ -145,8 +145,19 @@ func BrandingUpload(ctx *context.Context) { targets = append(targets, "logo.png") } - var writers []io.Writer - for _, name := range targets { + // Write the uploaded image to each target independently (rewinding the + // stream between targets) so a failure on a later target never leaves an + // earlier one truncated/empty. + for i, name := range targets { + if i > 0 { + if _, err := file.Seek(0, io.SeekStart); err != nil { + ctx.Flash.Error("Failed to write image") + log.Error("Seek branding image %s: %v", name, err) + ctx.Redirect(setting.AppSubURL + "/-/admin/branding") + return + } + } + destPath := filepath.Join(imgDir, name) dest, err := os.Create(destPath) if err != nil { @@ -155,8 +166,14 @@ func BrandingUpload(ctx *context.Context) { ctx.Redirect(setting.AppSubURL + "/-/admin/branding") return } - defer dest.Close() - writers = append(writers, dest) + if _, err := io.Copy(dest, file); err != nil { + dest.Close() + ctx.Flash.Error("Failed to write image") + log.Error("Copy to %s: %v", destPath, err) + ctx.Redirect(setting.AppSubURL + "/-/admin/branding") + return + } + dest.Close() // Remove any SVG override so the uploaded PNG takes effect. svgPath := filepath.Join(imgDir, name[:len(name)-4]+".svg") @@ -165,13 +182,6 @@ func BrandingUpload(ctx *context.Context) { } } - if _, err := io.Copy(io.MultiWriter(writers...), file); err != nil { - ctx.Flash.Error("Failed to write image") - log.Error("Copy branding image %s: %v", filename, err) - ctx.Redirect(setting.AppSubURL + "/-/admin/branding") - return - } - ctx.Flash.Success("Branding image updated: " + imageType) log.Info("Branding image uploaded: %s (%d bytes)", filename, header.Size) ctx.Redirect(setting.AppSubURL + "/-/admin/branding") -- 2.52.0