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

Add favicon.ico #756

Merged
merged 1 commit into from
Sep 28, 2024
Merged

Add favicon.ico #756

merged 1 commit into from
Sep 28, 2024

Conversation

Konano
Copy link
Contributor

@Konano Konano commented Sep 5, 2024

In the original intel, there is favicon.ico in the head section.

image

But after the IITC plugin, this line disappeared.

@mxxcon
Copy link

mxxcon commented Sep 5, 2024

But there's no actual icon file, unless I missed it...?

@Konano
Copy link
Contributor Author

Konano commented Sep 5, 2024

But there's no actual icon file, unless I missed it...?

I didn't add the ico file, I just added back the "shortcut icon" that was overwritten by IITC.

@modos189
Copy link
Contributor

In what cases is it required? When the icon is not displayed?

@xscreach
Copy link
Contributor

In what cases is it required? When the icon is not displayed?

This is just how to tell browser where to get icon for the site from. Browsers then use the icon for bookmarks and stuff like that.
The default url is iirc /favicon.ico...

This line is also present in the original intel html code.

But imho browser reads this info from the original html before it's rewritten by our stuff, we are not changing the value... so it might be kinda redundant, but maybe better save than sorry 🤷‍♂️

@Konano
Copy link
Contributor Author

Konano commented Sep 18, 2024

But imho browser reads this info from the original html before it's rewritten by our stuff, we are not changing the value... so it might be kinda redundant

Actually, browsers don't do that, from what I've seen... So if there is no icon url in the rewritten content, the browser will not load the icon.

@modos189 modos189 merged commit e21e7b6 into IITC-CE:master Sep 28, 2024
3 of 4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants