Skip to content

Fix favicon on API docs not rendered on Google Chrome #413

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

Merged
merged 2 commits into from
May 31, 2018

Conversation

exoego
Copy link
Contributor

@exoego exoego commented Apr 3, 2018

Partially addresses scala-js/scala-js#1701

HTML of API pages, e.g. https://www.scala-js.org/api/scalajs-library/latest/#scala.scalajs.js.package, does not specify favicon using link tag (MDN). In such case, major browsers tends to request "/favicon.ico" on root directory.

Firefox can render the ico file (https://www.scala-js.org/favicon.ico), but Chrome can not.
Unfortunately, I could not figure out the reason...

Anyway, the re-created ico seems to work fine on Chrome and Firefox.
(I just split ico into several png files and merge them via ImageMagick)

The new ico become bigger (300 KB). I hope website master find it is OK, since the file are delivered via CDN (cloudfrare) so that browsers can cache properly.

@sjrd
Copy link
Member

sjrd commented Apr 3, 2018

@ochrons Opinion on this? You're our master of asset size :)

@ochrons
Copy link
Contributor

ochrons commented May 31, 2018

300kB sounds awfully lot for a favicon. It should be converted to a smaller size, say 64x64 which would make it under 3kB. The current is 60kB and it's way too big as well :)

@exoego
Copy link
Contributor Author

exoego commented May 31, 2018

@ochrons
favicon is updated and now 7kb. It includes only 16x16, 32x32, 48x48 and 64x64 in 32bit color.

Copy link
Member

@sjrd sjrd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool, excellent :)

@sjrd sjrd merged commit ce81945 into scala-js:master May 31, 2018
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.

3 participants