Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Favicon not used when viewing files #19109

Closed
FWDekker opened this issue Mar 16, 2022 · 7 comments · Fixed by #19130
Closed

Favicon not used when viewing files #19109

FWDekker opened this issue Mar 16, 2022 · 7 comments · Fixed by #19130
Labels
good first issue Likely to be an easy fix hacktoberfest
Milestone

Comments

@FWDekker
Copy link
Contributor

Gitea Version

1.15.9

Operating System

FreeBSD 13

Browser Version

Firefox 98.0

Can you reproduce the bug on the Gitea demo site?

No

Description

Cannot reproduce at try.gitea.io because it does not use a non-standard favicon. See this file on my self-hosted Gitea instance as an example.

When looking at a file directly in the browser, such as an avatar or PDF file, the default favicon is used instead of the custom favicon. At least in Firefox, the browser displays the favicon at example.com/favicon.ico when viewing a file. On HTML pages, the browser correctly uses example.com/assets/img/logo.svg.

I have configured the favicon by adding the files favicon.ico, favicon.png, logo.png, and logo.svg to $GITEA_CUSTOM/public/img/.

Screenshots

Two tabs showing different favicons for the same Gitea instance.

On the left: correct favicon is used for an HTML page.
On the right: default favicon is used for an image.

@silverwind
Copy link
Member

silverwind commented Mar 16, 2022

Favicons are now SVG, so /favicon.ico route is unused and it redirects to root. It might be possible to redirect /favicon.ico to /assets/img/logo.svg file in case the browser would accept that. Or possible just serve the SVG on that route directly.

@techknowlogick
Copy link
Member

Probably a redirect would be better so that browsers don't hit both and cache both

@abheekda1
Copy link
Contributor

If this isn't already being tackled and someone is willing to give a pointer I'd be happy to take a look at fixing this issue :)

@silverwind
Copy link
Member

silverwind commented Mar 18, 2022

You can add the redirect right after

gitea/routers/web/web.go

Lines 97 to 100 in 04fcf23

// this png is very likely to always be below the limit for gzip so it doesn't need to pass through gzip
routes.Get("/apple-touch-icon.png", func(w http.ResponseWriter, req *http.Request) {
http.Redirect(w, req, path.Join(setting.StaticURLPrefix, "/assets/img/apple-touch-icon.png"), 301)
})
:

Note that I'm not sure if browser are going to accept such a redirect on favicon, but it's worth a try and won't hurt even if it's there.

@abheekda1
Copy link
Contributor

Seems that the correct favicon is stored at /assets/img/favicon.ico so I'm going to give a redirect to that location a shot.

@abheekda1
Copy link
Contributor

@silverwind it seems that the custom favicon at that path does not exist if the default one is used, meaning that redirecting to it may result in no favicon at all. Is there a better implementation that should allow me to remedy that?

Here's my current solution:

	routes.Get("/favicon.ico", func(w http.ResponseWriter, req *http.Request) {
		faviconPath := ""
		res, err := http.Get(path.Join(setting.StaticURLPrefix, "/assets/img/favicon.ico"))
		if err != nil || res.StatusCode != 200 {
			faviconPath = "/favicon.ico"
		} else {
			faviconPath = "/assets/img/favicon.ico"
		}
		http.Redirect(w, req, path.Join(setting.StaticURLPrefix, faviconPath), 301)
	})

zeripath pushed a commit that referenced this issue Mar 19, 2022
Redirect `/favicon.ico` to `/assets/img/favicon.png`.

Fix #19109
@FWDekker
Copy link
Contributor Author

Awesome! Thank you, all! :-)

abheekda1 added a commit to abheekda1/gitea that referenced this issue Mar 20, 2022
techknowlogick pushed a commit that referenced this issue Mar 21, 2022
…19152)

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

Fix #19109

Co-authored-by: zeripath <art27@cantab.net>
@zeripath zeripath added this to the 1.16.5 milestone Mar 23, 2022
Chianina pushed a commit to Chianina/gitea that referenced this issue Mar 28, 2022
@go-gitea go-gitea locked and limited conversation to collaborators Apr 28, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
good first issue Likely to be an easy fix hacktoberfest
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants