Skip to content

Conversation

@sl4m
Copy link

@sl4m sl4m commented Aug 13, 2021

Hi, this updates #75438 to fix an order of precedence issue for Chrome. Unfortunately, the last favicon defined wins when it comes to Chrome, hence the primary icon being placed last. I brought it up with the Chromium team last year, but so far it's a non-issue.

I've created an example website that mimics the behaviour in Chrome. https://sl4m.github.io/chrome-favicon/

This is what I'm seeing at the moment when viewing https://doc.rust-lang.org/core/index.html in Chrome. It's falling back to the PNG.

Screenshot 2021-08-12 at 21 11 58

@rust-highfive
Copy link
Contributor

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @GuillaumeGomez (or someone else) soon.

Please see the contribution instructions for more information.

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 13, 2021
@GuillaumeGomez
Copy link
Member

GuillaumeGomez commented Aug 13, 2021

What is the behaviour in the other browsers (I'm thinking about firefox mainly)?

@GuillaumeGomez GuillaumeGomez added the T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. label Aug 13, 2021
@sl4m
Copy link
Author

sl4m commented Aug 13, 2021

SVG takes precedence in Firefox. The order in this PR is the same order in the other Rust websites

Screenshot 2021-08-13 at 06 36 36

@GuillaumeGomez
Copy link
Member

Fine by me then, thanks!

@bors: r+ rollup

@bors
Copy link
Collaborator

bors commented Aug 13, 2021

📌 Commit 474ba60 has been approved by GuillaumeGomez

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 13, 2021
@GuillaumeGomez GuillaumeGomez added the A-rustdoc-ui Area: Rustdoc UI (generated HTML) label Aug 13, 2021
@bors
Copy link
Collaborator

bors commented Aug 14, 2021

⌛ Testing commit 474ba60 with merge 32c3f2d...

@bors
Copy link
Collaborator

bors commented Aug 14, 2021

☀️ Test successful - checks-actions
Approved by: GuillaumeGomez
Pushing 32c3f2d to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Aug 14, 2021
@bors bors merged commit 32c3f2d into rust-lang:master Aug 14, 2021
@rustbot rustbot added this to the 1.56.0 milestone Aug 14, 2021
@sl4m sl4m deleted the update-favicon-order branch August 14, 2021 04:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-rustdoc-ui Area: Rustdoc UI (generated HTML) merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants