Use custom favicon when viewing static files if it exists #19130

abheekda1 · 2022-03-18T20:05:10Z

Redirect /favicon.ico to /assets/img/favicon.png.

Fix #19109

Gusted · 2022-03-19T11:40:09Z

routers/web/web.go

+	// the custom favicon should be seen when viewing files instead of the default one if it exists,
+	routes.Get("/favicon.ico", func(w http.ResponseWriter, req *http.Request) {
+		faviconPath := ""
+		res, err := http.Get(path.Join(setting.StaticURLPrefix, "/assets/img/favicon.ico"))


It's better to only check this once and not make a request every time when favicon.ico is requested. We can use os.Stat to check if such file exists.

There are a number of logo files that are known to exist, basically the output files here:

gitea/build/generate-images.js

Lines 74 to 78 in fb08d2b

generate(svg, resolve(__dirname, '../public/img/logo.svg'), {size: 32}),

generate(svg, resolve(__dirname, '../public/img/logo.png'), {size: 512}),

generate(svg, resolve(__dirname, '../public/img/favicon.png'), {size: 180}),

generate(svg, resolve(__dirname, '../public/img/avatar_default.png'), {size: 200}),

generate(svg, resolve(__dirname, '../public/img/apple-touch-icon.png'), {size: 180, bg: true}),

If browser accept redirecting ico to either svg or png, I think this is the solution we should go for as favicon.ico generally does not exist for our installations (but users could provide it for compatibilty)

I think it may work to redirect to PNG, I'll give that a shot.

Yeah, it's likely if PNG works, it will work with SVG as well, see https://caniuse.com/link-icon-svg for browser support.

Another option might be to just generate a ico in above script, but that might be more complicated, and I think it should only be a last-resort option if those redirects don't work.

abheekda1 · 2022-03-19T13:28:44Z

Alright, cool! I'll give it a shot.

zeripath · 2022-03-19T19:51:19Z

make lgtm work

zeripath · 2022-03-20T17:29:30Z

I guess we should backport this.

zeripath · 2022-03-20T17:32:00Z

@ADawesomeguy are you able to send a backport PR?

This is my backport script: (if it helps)

#!/bin/sh
PR="$1"
SHA="$2"
VERSION="$3"

if [ -z "$SHA" ]; then
    SHA=$(gh api /repos/go-gitea/gitea/pulls/$PR -q '.merge_commit_sha')
fi

if [ -z "$VERSION" ]; then
    VERSION="v1.16"
fi

echo git checkout origin/release/"$VERSION" -b backport-$PR-$VERSION
git checkout origin/release/"$VERSION" -b backport-$PR-$VERSION
git cherry-pick $SHA && git commit --amend && git push ADawesomeguy backport-$PR-$VERSION && xdg-open https://github.com/go-gitea/gitea/compare/release/"$VERSION"...ADawesomeguy:backport-$PR-$VERSION

abheekda1 · 2022-03-20T17:33:05Z

Do you want me to run that, because yeah sure I can do that. What exactly is a backport?

zeripath · 2022-03-20T17:37:01Z

By backport I mean cherry-pick f96e8be on to release/v1.16,

That cherrypick branch would then be used to open a PR against release/v1.16 titled: Use custom favicon when viewing static files if it exists (#19130) (← it must be this title) the PR initial comment should be:

Backport #19130

Redirect `/favicon.ico` to `/assets/img/favicon.png`.

Fix #19109

…9130) Redirect `/favicon.ico` to `/assets/img/favicon.png`. Fix go-gitea#19109

abheekda1 · 2022-03-20T17:58:48Z

@zeripath like this? https://github.com/ADawesomeguy/gitea/commits/backport-19130-v1.16

zeripath · 2022-03-20T18:24:47Z

yeah basically.

zeripath · 2022-03-20T18:25:22Z

Now create a PR using: release/v1.16...ADawesomeguy:backport-19130-v1.16

abheekda1 · 2022-03-20T19:10:21Z

Alright, done!

…19152) Redirect `/favicon.ico` to `/assets/img/favicon.png`. Fix #19109 Co-authored-by: zeripath <art27@cantab.net>

* giteaofficial/main: Add 1.18 (go-gitea#19151) [skip ci] Updated translations via Crowdin Fix NPE `/repos/issues/search` when not signed in (go-gitea#19154) [skip ci] Updated licenses and gitignores Use custom favicon when viewing static files if it exists (go-gitea#19130) not send notification emails to inactive users (part 2) (go-gitea#19142) Make migrations SKIP_TLS_VERIFY apply to git too (go-gitea#19132) Do not send notification emails to inactive users (go-gitea#19131)

silverwind · 2022-03-21T13:31:17Z

Seems to work fine in Firefox. At some point in the future, we should redirect to the SVG.

## [1.16.5](https://github.com/go-gitea/gitea/releases/tag/1.16.5) - 2022-03-23 * BREAKING * Bump to build with go1.18 (go-gitea#19120 et al) (go-gitea#19127) * SECURITY * Prevent redirect to Host (2) (go-gitea#19175) (go-gitea#19186) * Try to prevent autolinking of displaynames by email readers (go-gitea#19169) (go-gitea#19183) * Clean paths when looking in Storage (go-gitea#19124) (go-gitea#19179) * Do not send notification emails to inactive users (go-gitea#19131) (go-gitea#19139) * Do not send activation email if manual confirm is set (go-gitea#19119) (go-gitea#19122) * ENHANCEMENTS * Use the new/choose link for New Issue on project page (go-gitea#19172) (go-gitea#19176) * BUGFIXES * Fix compare link in active feeds for new branch (go-gitea#19149) (go-gitea#19185) * Redirect .wiki/* ui link to /wiki (go-gitea#18831) (go-gitea#19184) * Ensure deploy keys with write access can push (go-gitea#19010) (go-gitea#19182) * Ensure that setting.LocalURL always has a trailing slash (go-gitea#19171) (go-gitea#19177) * Cleanup protected branches when deleting users & teams (go-gitea#19158) (go-gitea#19174) * Use IterateBufferSize whilst querying repositories during adoption check (go-gitea#19140) (go-gitea#19160) * Fix NPE /repos/issues/search when not signed in (go-gitea#19154) (go-gitea#19155) * Use custom favicon when viewing static files if it exists (go-gitea#19130) (go-gitea#19152) * Fix the editor height in review box (go-gitea#19003) (go-gitea#19147) * Ensure isSSH is set whenever DISABLE_HTTP_GIT is set (go-gitea#19028) (go-gitea#19146) * Fix wrong scopes caused by empty scope input (go-gitea#19029) (go-gitea#19145) * Make migrations SKIP_TLS_VERIFY apply to git too (go-gitea#19132) (go-gitea#19141) * Handle email address not exist (go-gitea#19089) (go-gitea#19121) * MISC * Update json-iterator to allow compilation with go1.18 (go-gitea#18644) (go-gitea#19100) * Update golang.org/x/crypto (go-gitea#19097) (go-gitea#19098) Signed-off-by: Andrew Thornton <art27@cantab.net>

…9130) Redirect `/favicon.ico` to `/assets/img/favicon.png`. Fix go-gitea#19109

Use custom favicon when viewing static files if it exists

d31b8aa

6543 added the type/bug label Mar 18, 2022

6543 added this to the 1.17.0 milestone Mar 18, 2022

Gusted reviewed Mar 19, 2022

View reviewed changes

GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Mar 19, 2022

Redirect to a favicon PNG that always exists

17217ce

abheekda1 requested review from Gusted and silverwind March 19, 2022 13:52

zeripath approved these changes Mar 19, 2022

View reviewed changes

GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Mar 19, 2022

lunny approved these changes Mar 19, 2022

View reviewed changes

GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Mar 19, 2022

Merge branch 'main' into custom-favicon-fix

c76d789

Gusted approved these changes Mar 19, 2022

View reviewed changes

Merge branch 'main' into custom-favicon-fix

0f8d7a2

zeripath merged commit f96e8be into go-gitea:main Mar 19, 2022

abheekda1 deleted the custom-favicon-fix branch March 19, 2022 19:57

zeripath added the backport/v1.16 label Mar 20, 2022

abheekda1 added a commit to abheekda1/gitea that referenced this pull request Mar 20, 2022

Use custom favicon when viewing static files if it exists (go-gitea#1…

aae7eab

…9130) Redirect `/favicon.ico` to `/assets/img/favicon.png`. Fix go-gitea#19109

zeripath mentioned this pull request Mar 20, 2022

Use custom favicon when viewing static files if it exists (#19130) #19152

Merged

zeripath added the backport/done All backports for this PR have been created label Mar 20, 2022

techknowlogick pushed a commit that referenced this pull request Mar 21, 2022

Use custom favicon when viewing static files if it exists (#19130) (#…

1aa5dc7

…19152) Redirect `/favicon.ico` to `/assets/img/favicon.png`. Fix #19109 Co-authored-by: zeripath <art27@cantab.net>

zeripath mentioned this pull request Mar 23, 2022

Changelog for 1.16.5 #19189

Merged

Chianina pushed a commit to Chianina/gitea that referenced this pull request Mar 28, 2022

Use custom favicon when viewing static files if it exists (go-gitea#1…

9eb08df

…9130) Redirect `/favicon.ico` to `/assets/img/favicon.png`. Fix go-gitea#19109

go-gitea locked and limited conversation to collaborators Apr 28, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use custom favicon when viewing static files if it exists #19130

Use custom favicon when viewing static files if it exists #19130

abheekda1 commented Mar 18, 2022 •

edited by zeripath

Loading

Gusted Mar 19, 2022

silverwind Mar 19, 2022

abheekda1 Mar 19, 2022

silverwind Mar 19, 2022

silverwind Mar 19, 2022

abheekda1 commented Mar 19, 2022

zeripath commented Mar 19, 2022

zeripath commented Mar 20, 2022

zeripath commented Mar 20, 2022

abheekda1 commented Mar 20, 2022

zeripath commented Mar 20, 2022

abheekda1 commented Mar 20, 2022

zeripath commented Mar 20, 2022

zeripath commented Mar 20, 2022 •

edited

Loading

abheekda1 commented Mar 20, 2022

silverwind commented Mar 21, 2022

	generate(svg, resolve(__dirname, '../public/img/logo.svg'), {size: 32}),
	generate(svg, resolve(__dirname, '../public/img/logo.png'), {size: 512}),
	generate(svg, resolve(__dirname, '../public/img/favicon.png'), {size: 180}),
	generate(svg, resolve(__dirname, '../public/img/avatar_default.png'), {size: 200}),
	generate(svg, resolve(__dirname, '../public/img/apple-touch-icon.png'), {size: 180, bg: true}),

Use custom favicon when viewing static files if it exists #19130

Use custom favicon when viewing static files if it exists #19130

Conversation

abheekda1 commented Mar 18, 2022 • edited by zeripath Loading

Gusted Mar 19, 2022

Choose a reason for hiding this comment

silverwind Mar 19, 2022

Choose a reason for hiding this comment

abheekda1 Mar 19, 2022

Choose a reason for hiding this comment

silverwind Mar 19, 2022

Choose a reason for hiding this comment

silverwind Mar 19, 2022

Choose a reason for hiding this comment

abheekda1 commented Mar 19, 2022

zeripath commented Mar 19, 2022

zeripath commented Mar 20, 2022

zeripath commented Mar 20, 2022

abheekda1 commented Mar 20, 2022

zeripath commented Mar 20, 2022

abheekda1 commented Mar 20, 2022

zeripath commented Mar 20, 2022

zeripath commented Mar 20, 2022 • edited Loading

abheekda1 commented Mar 20, 2022

silverwind commented Mar 21, 2022

abheekda1 commented Mar 18, 2022 •

edited by zeripath

Loading

zeripath commented Mar 20, 2022 •

edited

Loading