[GITEA] Improve HTML title on repositories

- The `<title>` element that lives inside the `<head>` element is an important element that gives browsers and search engine crawlers the title of the webpage, hence the element name. It's therefor important that this title is accurate.
- Currently there are three issues with titles on repositories. It doesn't use the `FullName` and instead only uses the repository name, this doesn't distinguish which user or organisation the repository is on. It doesn't show the full treepath in the title when visiting an file inside a directory and instead only uses the latest path in treepath. It can show the repository name twice if the `.Title` variable also included the repository name such as on the repository homepage.
- Use the repository's fullname (which include which user the repository is on) instead of just their name.
- Display the repository's fullname if it isn't already in `.Title`.
- Use the full treepath in the repository code view instead of just the
last path.
- Adds integration tests.
- Adds a new repository (`repo59`) that has 3 depths for folders, which
wasn't in any other fixture repository yet, so the full treepath for
could be properly tested.
- Resolves https://codeberg.org/forgejo/forgejo/issues/1276

(cherry picked from commit ff9a6a2cda)
This commit is contained in:
Gusted 2023-08-18 11:21:24 +02:00 committed by Earl Warren
parent d6dcc34492
commit 76dffc8621
No known key found for this signature in database
GPG key ID: 0579CB2928A78A00
25 changed files with 206 additions and 10 deletions

View file

@ -136,3 +136,17 @@
is_prerelease: false is_prerelease: false
is_tag: false is_tag: false
created_unix: 946684803 created_unix: 946684803
- id: 11
repo_id: 59
publisher_id: 2
tag_name: "v1.0"
lower_tag_name: "v1.0"
target: "main"
title: "v1.0"
sha1: "d8f53dfb33f6ccf4169c34970b5e747511c18beb"
num_commits: 1
is_draft: false
is_prerelease: false
is_tag: false
created_unix: 946684803

View file

@ -608,6 +608,38 @@
type: 1 type: 1
created_unix: 946684810 created_unix: 946684810
# BEGIN Forgejo [GITEA] Improve HTML title on repositories
-
id: 1093
repo_id: 59
type: 1
created_unix: 946684810
-
id: 1094
repo_id: 59
type: 2
created_unix: 946684810
-
id: 1095
repo_id: 59
type: 3
created_unix: 946684810
-
id: 1096
repo_id: 59
type: 4
created_unix: 946684810
-
id: 1097
repo_id: 59
type: 5
created_unix: 946684810
# END Forgejo [GITEA] Improve HTML title on repositories
- -
id: 91 id: 91
repo_id: 58 repo_id: 58

View file

@ -1467,6 +1467,7 @@
owner_name: user27 owner_name: user27
lower_name: repo49 lower_name: repo49
name: repo49 name: repo49
description: A wonderful repository with more than just a README.md
default_branch: master default_branch: master
num_watches: 0 num_watches: 0
num_stars: 0 num_stars: 0
@ -1693,3 +1694,16 @@
size: 0 size: 0
is_fsck_enabled: true is_fsck_enabled: true
close_issues_via_commit_in_any_branch: false close_issues_via_commit_in_any_branch: false
-
id: 59
owner_id: 2
owner_name: user2
lower_name: repo59
name: repo59
default_branch: master
is_empty: false
is_archived: false
is_private: false
status: 0
num_issues: 0

View file

@ -66,7 +66,7 @@
num_followers: 2 num_followers: 2
num_following: 1 num_following: 1
num_stars: 2 num_stars: 2
num_repos: 14 num_repos: 15
num_teams: 0 num_teams: 0
num_members: 0 num_members: 0
visibility: 0 visibility: 0

View file

@ -138,12 +138,12 @@ func getTestCases() []struct {
{ {
name: "AllPublic/PublicRepositoriesOfUserIncludingCollaborative", name: "AllPublic/PublicRepositoriesOfUserIncludingCollaborative",
opts: &repo_model.SearchRepoOptions{ListOptions: db.ListOptions{Page: 1, PageSize: 10}, OwnerID: 15, AllPublic: true, Template: util.OptionalBoolFalse}, opts: &repo_model.SearchRepoOptions{ListOptions: db.ListOptions{Page: 1, PageSize: 10}, OwnerID: 15, AllPublic: true, Template: util.OptionalBoolFalse},
count: 31, count: 32,
}, },
{ {
name: "AllPublic/PublicAndPrivateRepositoriesOfUserIncludingCollaborative", name: "AllPublic/PublicAndPrivateRepositoriesOfUserIncludingCollaborative",
opts: &repo_model.SearchRepoOptions{ListOptions: db.ListOptions{Page: 1, PageSize: 10}, OwnerID: 15, Private: true, AllPublic: true, AllLimited: true, Template: util.OptionalBoolFalse}, opts: &repo_model.SearchRepoOptions{ListOptions: db.ListOptions{Page: 1, PageSize: 10}, OwnerID: 15, Private: true, AllPublic: true, AllLimited: true, Template: util.OptionalBoolFalse},
count: 36, count: 37,
}, },
{ {
name: "AllPublic/PublicAndPrivateRepositoriesOfUserIncludingCollaborativeByName", name: "AllPublic/PublicAndPrivateRepositoriesOfUserIncludingCollaborativeByName",
@ -158,7 +158,7 @@ func getTestCases() []struct {
{ {
name: "AllPublic/PublicRepositoriesOfOrganization", name: "AllPublic/PublicRepositoriesOfOrganization",
opts: &repo_model.SearchRepoOptions{ListOptions: db.ListOptions{Page: 1, PageSize: 10}, OwnerID: 17, AllPublic: true, Collaborate: util.OptionalBoolFalse, Template: util.OptionalBoolFalse}, opts: &repo_model.SearchRepoOptions{ListOptions: db.ListOptions{Page: 1, PageSize: 10}, OwnerID: 17, AllPublic: true, Collaborate: util.OptionalBoolFalse, Template: util.OptionalBoolFalse},
count: 31, count: 32,
}, },
{ {
name: "AllTemplates", name: "AllTemplates",

View file

@ -164,7 +164,7 @@ func renderDirectory(ctx *context.Context, treeLink string) {
if ctx.Repo.TreePath != "" { if ctx.Repo.TreePath != "" {
ctx.Data["HideRepoInfo"] = true ctx.Data["HideRepoInfo"] = true
ctx.Data["Title"] = ctx.Tr("repo.file.title", ctx.Repo.Repository.Name+"/"+path.Base(ctx.Repo.TreePath), ctx.Repo.RefName) ctx.Data["Title"] = ctx.Tr("repo.file.title", ctx.Repo.Repository.Name+"/"+util.PathEscapeSegments(ctx.Repo.TreePath), ctx.Repo.RefName)
} }
subfolder, readmeFile, err := findReadmeFileInEntries(ctx, entries, true) subfolder, readmeFile, err := findReadmeFileInEntries(ctx, entries, true)
@ -343,7 +343,7 @@ func renderFile(ctx *context.Context, entry *git.TreeEntry, treeLink, rawLink st
} }
defer dataRc.Close() defer dataRc.Close()
ctx.Data["Title"] = ctx.Tr("repo.file.title", ctx.Repo.Repository.Name+"/"+path.Base(ctx.Repo.TreePath), ctx.Repo.RefName) ctx.Data["Title"] = ctx.Tr("repo.file.title", ctx.Repo.Repository.Name+"/"+util.PathEscapeSegments(ctx.Repo.TreePath), ctx.Repo.RefName)
ctx.Data["FileIsSymlink"] = entry.IsLink() ctx.Data["FileIsSymlink"] = entry.IsLink()
ctx.Data["FileName"] = blob.Name() ctx.Data["FileName"] = blob.Name()
ctx.Data["RawFileLink"] = rawLink + "/" + util.PathEscapeSegments(ctx.Repo.TreePath) ctx.Data["RawFileLink"] = rawLink + "/" + util.PathEscapeSegments(ctx.Repo.TreePath)

View file

@ -2,7 +2,8 @@
<html lang="{{ctx.Locale.Lang}}" class="theme-{{if .SignedUser.Theme}}{{.SignedUser.Theme}}{{else}}{{DefaultTheme}}{{end}}"> <html lang="{{ctx.Locale.Lang}}" class="theme-{{if .SignedUser.Theme}}{{.SignedUser.Theme}}{{else}}{{DefaultTheme}}{{end}}">
<head> <head>
<meta name="viewport" content="width=device-width, initial-scale=1"> <meta name="viewport" content="width=device-width, initial-scale=1">
<title>{{if .Title}}{{.Title | RenderEmojiPlain}} - {{end}}{{if .Repository.Name}}{{.Repository.Name}} - {{end}}{{AppName}}</title> {{/* Display `- .Repsository.FullName` only if `.Title` does not already start with that. */}}
<title>{{if .Title}}{{.Title | RenderEmojiPlain}} - {{end}}{{if and (.Repository.Name) (not (StringUtils.HasPrefix .Title .Repository.FullName))}}{{.Repository.FullName}} - {{end}}{{AppName}}</title>
{{if .ManifestData}}<link rel="manifest" href="data:{{.ManifestData}}">{{end}} {{if .ManifestData}}<link rel="manifest" href="data:{{.ManifestData}}">{{end}}
<meta name="author" content="{{if .Repository}}{{.Owner.Name}}{{else}}{{MetaAuthor}}{{end}}"> <meta name="author" content="{{if .Repository}}{{.Owner.Name}}{{else}}{{MetaAuthor}}{{end}}">
<meta name="description" content="{{if .Repository}}{{.Repository.Name}}{{if .Repository.Description}} - {{.Repository.Description}}{{end}}{{else}}{{MetaDescription}}{{end}}"> <meta name="description" content="{{if .Repository}}{{.Repository.Name}}{{if .Repository.Description}} - {{.Repository.Description}}{{end}}{{else}}{{MetaDescription}}{{end}}">

View file

@ -0,0 +1 @@
ref: refs/heads/master

View file

@ -0,0 +1,4 @@
[core]
repositoryformatversion = 0
filemode = true
bare = true

View file

@ -0,0 +1 @@
Unnamed repository; edit this file 'description' to name the repository.

View file

@ -0,0 +1,6 @@
# git ls-files --others --exclude-from=.git/info/exclude
# Lines that start with '#' are comments.
# For a project mostly in C, the following would be a good set of
# exclude patterns (uncomment them if you want to use them):
# *.[oa]
# *~

View file

@ -0,0 +1,3 @@
# pack-refs with: peeled fully-peeled sorted
d8f53dfb33f6ccf4169c34970b5e747511c18beb refs/heads/master
d8f53dfb33f6ccf4169c34970b5e747511c18beb refs/tags/v1.0

View file

@ -0,0 +1 @@
d8f53dfb33f6ccf4169c34970b5e747511c18beb

View file

@ -93,9 +93,9 @@ func TestAPISearchRepo(t *testing.T) {
}{ }{
{ {
name: "RepositoriesMax50", requestURL: "/api/v1/repos/search?limit=50&private=false", expectedResults: expectedResults{ name: "RepositoriesMax50", requestURL: "/api/v1/repos/search?limit=50&private=false", expectedResults: expectedResults{
nil: {count: 33}, nil: {count: 34},
user: {count: 33}, user: {count: 34},
user2: {count: 33}, user2: {count: 34},
}, },
}, },
{ {

View file

@ -531,3 +531,18 @@ func GetCSRF(t testing.TB, session *TestSession, urlStr string) string {
doc := NewHTMLParser(t, resp.Body) doc := NewHTMLParser(t, resp.Body)
return doc.GetCSRF() return doc.GetCSRF()
} }
func GetHTMLTitle(t testing.TB, session *TestSession, urlStr string) string {
t.Helper()
req := NewRequest(t, "GET", urlStr)
var resp *httptest.ResponseRecorder
if session == nil {
resp = MakeRequest(t, req, http.StatusOK)
} else {
resp = session.MakeRequest(t, req, http.StatusOK)
}
doc := NewHTMLParser(t, resp.Body)
return doc.Find("head title").Text()
}

View file

@ -444,3 +444,107 @@ func TestGeneratedSourceLink(t *testing.T) {
assert.Equal(t, "/user27/repo49/src/commit/aacbdfe9e1c4b47f60abe81849045fa4e96f1d75/test/test.txt", dataURL) assert.Equal(t, "/user27/repo49/src/commit/aacbdfe9e1c4b47f60abe81849045fa4e96f1d75/test/test.txt", dataURL)
}) })
} }
func TestRepoHTMLTitle(t *testing.T) {
defer tests.PrepareTestEnv(t)()
t.Run("Repository homepage", func(t *testing.T) {
t.Run("Without description", func(t *testing.T) {
defer tests.PrintCurrentTest(t)()
htmlTitle := GetHTMLTitle(t, nil, "/user2/repo1")
assert.EqualValues(t, "user2/repo1 - Gitea: Git with a cup of tea", htmlTitle)
})
t.Run("With description", func(t *testing.T) {
defer tests.PrintCurrentTest(t)()
htmlTitle := GetHTMLTitle(t, nil, "/user27/repo49")
assert.EqualValues(t, "user27/repo49: A wonderful repository with more than just a README.md - Gitea: Git with a cup of tea", htmlTitle)
})
})
t.Run("Code view", func(t *testing.T) {
t.Run("Directory", func(t *testing.T) {
t.Run("Default branch", func(t *testing.T) {
defer tests.PrintCurrentTest(t)()
htmlTitle := GetHTMLTitle(t, nil, "/user2/repo59/src/branch/master/deep/nesting")
assert.EqualValues(t, "repo59/deep/nesting at master - user2/repo59 - Gitea: Git with a cup of tea", htmlTitle)
})
t.Run("Non-default branch", func(t *testing.T) {
defer tests.PrintCurrentTest(t)()
htmlTitle := GetHTMLTitle(t, nil, "/user2/repo59/src/branch/cake-recipe/deep/nesting")
assert.EqualValues(t, "repo59/deep/nesting at cake-recipe - user2/repo59 - Gitea: Git with a cup of tea", htmlTitle)
})
t.Run("Commit", func(t *testing.T) {
defer tests.PrintCurrentTest(t)()
htmlTitle := GetHTMLTitle(t, nil, "/user2/repo59/src/commit/d8f53dfb33f6ccf4169c34970b5e747511c18beb/deep/nesting/")
assert.EqualValues(t, "repo59/deep/nesting at d8f53dfb33f6ccf4169c34970b5e747511c18beb - user2/repo59 - Gitea: Git with a cup of tea", htmlTitle)
})
t.Run("Tag", func(t *testing.T) {
defer tests.PrintCurrentTest(t)()
htmlTitle := GetHTMLTitle(t, nil, "/user2/repo59/src/tag/v1.0/deep/nesting/")
assert.EqualValues(t, "repo59/deep/nesting at v1.0 - user2/repo59 - Gitea: Git with a cup of tea", htmlTitle)
})
})
t.Run("File", func(t *testing.T) {
t.Run("Default branch", func(t *testing.T) {
defer tests.PrintCurrentTest(t)()
htmlTitle := GetHTMLTitle(t, nil, "/user2/repo59/src/branch/master/deep/nesting/folder/secret_sauce_recipe.txt")
assert.EqualValues(t, "repo59/deep/nesting/folder/secret_sauce_recipe.txt at master - user2/repo59 - Gitea: Git with a cup of tea", htmlTitle)
})
t.Run("Non-default branch", func(t *testing.T) {
defer tests.PrintCurrentTest(t)()
htmlTitle := GetHTMLTitle(t, nil, "/user2/repo59/src/branch/cake-recipe/deep/nesting/folder/secret_sauce_recipe.txt")
assert.EqualValues(t, "repo59/deep/nesting/folder/secret_sauce_recipe.txt at cake-recipe - user2/repo59 - Gitea: Git with a cup of tea", htmlTitle)
})
t.Run("Commit", func(t *testing.T) {
defer tests.PrintCurrentTest(t)()
htmlTitle := GetHTMLTitle(t, nil, "/user2/repo59/src/commit/d8f53dfb33f6ccf4169c34970b5e747511c18beb/deep/nesting/folder/secret_sauce_recipe.txt")
assert.EqualValues(t, "repo59/deep/nesting/folder/secret_sauce_recipe.txt at d8f53dfb33f6ccf4169c34970b5e747511c18beb - user2/repo59 - Gitea: Git with a cup of tea", htmlTitle)
})
t.Run("Tag", func(t *testing.T) {
defer tests.PrintCurrentTest(t)()
htmlTitle := GetHTMLTitle(t, nil, "/user2/repo59/src/tag/v1.0/deep/nesting/folder/secret_sauce_recipe.txt")
assert.EqualValues(t, "repo59/deep/nesting/folder/secret_sauce_recipe.txt at v1.0 - user2/repo59 - Gitea: Git with a cup of tea", htmlTitle)
})
})
})
t.Run("Issues view", func(t *testing.T) {
t.Run("Overview page", func(t *testing.T) {
defer tests.PrintCurrentTest(t)()
htmlTitle := GetHTMLTitle(t, nil, "/user2/repo1/issues")
assert.EqualValues(t, "Issues - user2/repo1 - Gitea: Git with a cup of tea", htmlTitle)
})
t.Run("View issue page", func(t *testing.T) {
defer tests.PrintCurrentTest(t)()
htmlTitle := GetHTMLTitle(t, nil, "/user2/repo1/issues/1")
assert.EqualValues(t, "#1 - issue1 - user2/repo1 - Gitea: Git with a cup of tea", htmlTitle)
})
})
t.Run("Pull requests view", func(t *testing.T) {
t.Run("Overview page", func(t *testing.T) {
defer tests.PrintCurrentTest(t)()
htmlTitle := GetHTMLTitle(t, nil, "/user2/repo1/pulls")
assert.EqualValues(t, "Pull Requests - user2/repo1 - Gitea: Git with a cup of tea", htmlTitle)
})
t.Run("View pull request", func(t *testing.T) {
defer tests.PrintCurrentTest(t)()
htmlTitle := GetHTMLTitle(t, nil, "/user2/repo1/pulls/2")
assert.EqualValues(t, "#2 - issue2 - user2/repo1 - Gitea: Git with a cup of tea", htmlTitle)
})
})
}