git724/forgejo
1
0
Fork 0
Code Issues Pull requests 3 Projects 1 Releases Packages Wiki Activity

ui: improve error pages (#7274)

Browse source 
* add testing
* make each page accessible via `/devtest/error`
* allow translating the `Page not found` part of the title
* code: improve consistency, remove unused
* devtest: put index page in a container to fix alignment
* 500: make navbar more like the real one, remove fake menu button
* deadcode: remove unused `func NotFound`: it was added in bdd32f152d and the only usage was removed in 1bfb0a24d8

Preview:
https://codeberg.org/attachments/1b75afb3-e898-410f-be02-f036a5400143

Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/7274
Reviewed-by: Gusted <gusted@noreply.codeberg.org>
Reviewed-by: Beowulf <beowulf@beocode.eu>
This commit is contained in:
0ko 2025-03-28 19:50:43 +00:00
commit 51ff4970ec
11 changed files with 106 additions and 32 deletions
Download patch file Download diff file Expand all files Collapse all files

3
.deadcode-out
View file

@ -210,9 +210,6 @@ forgejo.org/modules/zstd
Writer.Write
Writer.Close
forgejo.org/routers/web
NotFound
forgejo.org/routers/web/org
MustEnableProjects

3
modules/testlogger/testlogger.go
View file

@ -363,6 +363,9 @@ var ignoredErrorMessage = []string{
// TestDatabaseCollation
`[E] [Error SQL Query] INSERT INTO test_collation_tbl (txt) VALUES ('main') []`,
// TestDevtestErrorpages
`ErrorPage() [E] Example error: Example error`,
}
func (w *testLoggerWriterCloser) recordError(msg string) {

5
options/locale_next/locale_en-US.json
View file

@ -16,5 +16,6 @@
"incorrect_root_url": "This Forgejo instance is configured to be served on \"%s\". You are currently viewing Forgejo through a different URL, which may cause parts of the application to break. The canonical URL is controlled by Forgejo admins via the ROOT_URL setting in the app.ini.",
"themes.names.forgejo-auto": "Forgejo (follow system theme)",
"themes.names.forgejo-light": "Forgejo light",
"themes.names.forgejo-dark": "Forgejo dark"
}
"themes.names.forgejo-dark": "Forgejo dark",
"error.not_found.title": "Page not found"
}

13
routers/web/devtest/devtest.go
View file

@ -1,9 +1,11 @@
// Copyright 2023 The Gitea Authors. All rights reserved.
// Copyright 2025 The Forgejo Authors. All rights reserved.
// SPDX-License-Identifier: MIT
package devtest
import (
"errors"
"net/http"
"path"
"strings"
@ -42,6 +44,17 @@ func FetchActionTest(ctx *context.Context) {
ctx.JSONRedirect("")
}
func ErrorPage(ctx *context.Context) {
if ctx.Params("errcode") == "404" {
ctx.NotFound("Example error", errors.New("Example error"))
return
} else if ctx.Params("errcode") == "413" {
ctx.HTML(http.StatusRequestEntityTooLarge, base.TplName("status/413"))
return
}
ctx.ServerError("Example error", errors.New("Example error"))
}
func Tmpl(ctx *context.Context) {
now := time.Now()
ctx.Data["TimeNow"] = now

7
routers/web/home.go
View file

@ -1,5 +1,6 @@
// Copyright 2014 The Gogs Authors. All rights reserved.
// Copyright 2019 The Gitea Authors. All rights reserved.
// Copyright 2025 The Forgejo Authors. All rights reserved.
// SPDX-License-Identifier: MIT
package web
@ -112,9 +113,3 @@ func HomeSitemap(ctx *context.Context) {
log.Error("Failed writing sitemap: %v", err)
}
}
// NotFound render 404 page
func NotFound(ctx *context.Context) {
ctx.Data["Title"] = "Page Not Found"
ctx.NotFound("home.NotFound", nil)
}

2
routers/web/web.go
View file

@ -1,4 +1,5 @@
// Copyright 2017 The Gitea Authors. All rights reserved.
// Copyright 2023 The Forgejo Authors. All rights reserved.
// SPDX-License-Identifier: MIT
package web
@ -1661,6 +1662,7 @@ func registerRoutes(m *web.Route) {
m.Any("/devtest", devtest.List)
m.Any("/devtest/fetch-action-test", devtest.FetchActionTest)
m.Any("/devtest/{sub}", devtest.Tmpl)
m.Get("/devtest/error/{errcode}", devtest.ErrorPage)
}
m.NotFound(func(w http.ResponseWriter, req *http.Request) {

11
services/context/context_response.go
View file

@ -1,4 +1,5 @@
// Copyright 2023 The Gitea Authors. All rights reserved.
// Copyright 2024 The Forgejo Authors. All rights reserved.
// SPDX-License-Identifier: MIT
package context
@ -66,7 +67,10 @@ func (ctx *Context) RedirectToFirst(location ...string) string {
return setting.AppSubURL + "/"
}
const tplStatus500 base.TplName = "status/500"
const (
tplStatus404 base.TplName = "status/404"
tplStatus500 base.TplName = "status/500"
)
// HTML calls Context.HTML and renders the template to HTTP response
func (ctx *Context) HTML(status int, name base.TplName) {
@ -153,8 +157,8 @@ func (ctx *Context) notFoundInternal(logMsg string, logErr error) {
}
ctx.Data["IsRepo"] = ctx.Repo.Repository != nil
ctx.Data["Title"] = "Page Not Found"
ctx.HTML(http.StatusNotFound, base.TplName("status/404"))
ctx.Data["Title"] = ctx.Locale.TrString("error.not_found.title")
ctx.HTML(http.StatusNotFound, tplStatus404)
}
// ServerError displays a 500 (Internal Server Error) page and prints the given error, if any.
@ -177,7 +181,6 @@ func (ctx *Context) serverErrorInternal(logMsg string, logErr error) {
}
}
ctx.Data["Title"] = "Internal Server Error"
ctx.HTML(http.StatusInternalServerError, tplStatus500)
}

21
templates/devtest/list.tmpl
View file

@ -1,10 +1,21 @@
{{template "base/head" .}}
<ul>
{{range .SubNames}}
<li><a href="{{AppSubUrl}}/devtest/{{.}}">{{.}}</a></li>
{{end}}
</ul>
<div role="main" class="page-content ui container">
<ul>
{{range .SubNames}}
<li><a href="{{AppSubUrl}}/devtest/{{.}}">{{.}}</a></li>
{{end}}
</ul>
<article>
<h2>Error pages</h2>
<ul>
<li><a href="./error/404">Not found</a></li>
<li><a href="./error/413">Quota exhaustion</a></li>
<li><a href="./error/500">Server error</a></li>
</ul>
</article>
</div>
<style>
ul {

17
templates/status/500.tmpl
View file

@ -16,19 +16,14 @@
</head>
<body>
<div class="full height">
<nav class="ui secondary menu">
<div class="ui container tw-flex">
<div class="item tw-flex-1">
<a href="{{AppSubUrl}}/" aria-label="{{ctx.Locale.Tr "home"}}">
<img width="30" height="30" src="{{AssetUrlPrefix}}/img/logo.svg" alt="{{ctx.Locale.Tr "logo"}}" aria-hidden="true">
</a>
</div>
<div class="item">
<button class="ui icon button disabled">{{svg "octicon-three-bars"}}</button>{{/* a fake button to make the UI looks better*/}}
</div>
<nav id="navbar" aria-label="{{ctx.Locale.Tr "aria.navbar"}}">
<div class="navbar-left ui secondary menu">
<a class="item" id="navbar-logo" href="{{AppSubUrl}}/" aria-label="{{ctx.Locale.Tr "home"}}">
<img width="30" height="30" src="{{AssetUrlPrefix}}/img/logo.svg" alt="{{ctx.Locale.Tr "logo"}}" aria-hidden="true">
</a>
</div>
</nav>
<div class="divider tw-my-0"></div>
<div role="main" class="page-content status-page-500">
<div class="ui container" >
<style> .ui.message.flash-message { text-align: left; } </style>

53
tests/integration/devtest_error_test.go Normal file
View file

@ -0,0 +1,53 @@
// Copyright 2025 The Forgejo Authors. All rights reserved.
// SPDX-License-Identifier: GPL-3.0-or-later
package integration
import (
"net/http"
"testing"
"forgejo.org/modules/setting"
"forgejo.org/modules/test"
"forgejo.org/routers"
"github.com/stretchr/testify/assert"
)
// `/devtest/error/{errcode}` provides a convenient way of testing various
// error pages sometimes which can be hard to reach otherwise.
// This file is a test of various attributes on those pages.
func TestDevtestErrorpages(t *testing.T) {
defer test.MockVariableValue(&setting.IsProd, false)()
defer test.MockVariableValue(&testWebRoutes, routers.NormalRoutes())()
t.Run("Server error", func(t *testing.T) {
// `/devtest/error/x` returns 500 for any x by default.
// `/500` is simply for good look here
req := NewRequest(t, "GET", "/devtest/error/500")
resp := MakeRequest(t, req, http.StatusInternalServerError)
doc := NewHTMLParser(t, resp.Body)
assert.EqualValues(t, "500", doc.Find(".error-code").Text())
assert.Contains(t, doc.Find("head title").Text(), "Internal server error")
})
t.Run("Page not found",
func(t *testing.T) {
req := NewRequest(t, "GET", "/devtest/error/404").
// Without this header `notFoundInternal` returns plaintext error message
SetHeader("Accept", "text/html")
resp := MakeRequest(t, req, http.StatusNotFound)
doc := NewHTMLParser(t, resp.Body)
assert.EqualValues(t, "404", doc.Find(".error-code").Text())
assert.Contains(t, doc.Find("head title").Text(), "Page not found")
})
t.Run("Quota exhaustion",
func(t *testing.T) {
req := NewRequest(t, "GET", "/devtest/error/413")
resp := MakeRequest(t, req, http.StatusRequestEntityTooLarge)
doc := NewHTMLParser(t, resp.Body)
assert.EqualValues(t, "413", doc.Find(".error-code").Text())
})
}

3
tests/integration/repo_test.go
View file

@ -1,4 +1,5 @@
// Copyright 2017 The Gitea Authors. All rights reserved.
// Copyright 2023 The Forgejo Authors. All rights reserved.
// SPDX-License-Identifier: MIT
package integration
@ -691,7 +692,7 @@ func TestCommitView(t *testing.T) {
// Really ensure that 404 is being sent back.
doc := NewHTMLParser(t, resp.Body)
doc.AssertElement(t, `[aria-label="Page Not Found"]`, true)
doc.AssertElement(t, `[aria-label="Page not found"]`, true)
})
t.Run("Too short commit ID", func(t *testing.T) {