Add favicon to internal routes /favicon.ico

[hahmed]

Summary

When creating a new app and running the rails application (rails welcome page), there is an error with the /favicon.ico request. This PR adds an internal route for the rails template to serve the favicon by sending back an head :ok response.

Previously the error we got:

Started GET "/favicon.ico" for 127.0.0.1 at 2017-06-21 10:10:01 +0100

ActionController::RoutingError (No route matches [GET] "/favicon.ico"):

I have moved the favicon into a header element -- one less route to override.

Other Information

Fixes #29578

To replicate the error, create a new rails app, on chrome run rails s then hard refresh should make a request for the favicon.

Screenshot:

[p8]

The root route to the welcomes_controller gets overridden in apps.
Does this also work as expected for the 'favicon.ico'?
Will it be confusing for new users why the favicon.ico returns a 204 by default?

[p8]

An older but closed PR that tried adding the favicon to /public #29585

[p8]

We could also just inline the icon in the welcome template.

<link rel="icon" href="data:image/svg+xml,%3Csvg%20xmlns='http://www.w3.org/2000/svg'%20viewBox='0%200%2016%2016'%3E%3Ctext%20x='0'%20y='14'%20style='filter:%20invert(100%);'%3E🦄%3C/text%3E%3C/svg%3E" type="image/svg+xml" />

https://stackoverflow.com/a/62438464

This makes sure there are no extra routes, which might still be enabled by default.
And adding your own favicon.ico in /public would still work.

[hahmed]

We could also just inline the icon in the welcome template.

@p8 inlining the favicon works for me 🙂

[zzak]

@hahmed Curious where you got the svg from, does it show up in the screenshot in the description of this PR too?

LGTM though, I generally like this approach but we may want to move the SVG content to a helper that can be tested, this way if Rails ever changes its logo a code search for "logo" will show up. 🤔

[sunny]

Curious where you got the svg from

It’s actually a jpg. Which is not ideal since it has no transparency:

An SVG would be great here. Using this SVG it would look more like this:

we may want to move the SVG content to a helper that can be tested, this way if Rails ever changes its logo a code search for "logo" will show up.

If we do that for the favicon it would mean doing this for all other images on the page (which also include the logo). Perhaps a comment, e.g. <!-- Rails logo --> would be enough, here?

[hahmed]

LGTM though, I generally like this approach but we may want to move the SVG content to a helper that can be tested, this way if Rails ever changes its logo a code search for "logo" will show up. 🤔

I have moved this to a var in the view, added a descriptive name, hope that's ok?

Could you clarify how it could be tested? Also I'm not sure we are testing the other 2 images on the page either...

[p8]

The logo is a bit outdated. I'll try to see if I can generate a SVG from https://www.pngegg.com/en/png-zrbae

[p8]

I've created an SVG that looks like this:

<?xml version="1.0" encoding="UTF-8"?>
<svg width="320px" height="320px" viewBox="0 0 320 320" version="1.1" xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink">
    <title>Icon</title>
    <g id="Icon" stroke="none" stroke-width="1" fill="none" fill-rule="evenodd">
        <g id="Ruby-On-Rails-Logo" transform="translate(5.000000, 20.000000)" fill="#D81E00">
            <path d="M21.9330706,285 L168.175426,285 C147.940858,150.691706 196.882383,66.3142902 315,31.8677536 C315,21.7851284 315,31.8677536 315,21.7851284 C161.25283,24.9116278 63.5638533,112.649918 21.9330706,285 Z" id="Path"></path>
            <polygon id="Path" points="40.4082643 185.91571 12.7326494 174.401663 -1.42108547e-14 205.44271 29.5711966 216.647115"></polygon>
            <polygon id="Path" points="180.473102 268.073643 204.936016 276.410966 204.936016 254.508673 180.489644 244.338107"></polygon>
            <polygon id="Path" points="100.589519 97.2287606 76.6840156 79.1687021 55.2440791 100.183105 81.5143239 117.433725"></polygon>
            <polygon id="Path" points="184.575679 184.896295 207.95203 200.24661 211.727729 181.205266 189.863653 164.68746"></polygon>
            <polygon id="Path" points="261.73401 65.8959446 269.354234 82.32954 285.318256 72.7048596 278.310129 56.9829895"></polygon>
            <polygon id="Path" points="261.9137 16.1845473 255.598479 7.10542736e-15 232.79602 3.48896323 240.463873 20.0252725"></polygon>
            <polygon id="Path" points="211.934119 111.580675 226.722547 127.792601 238.023925 113.903715 223.468375 96.5468685"></polygon>
            <polygon id="Path" points="179.686858 38.4952218 164.641931 20.0252725 139.84762 32.5552992 156.601324 50.9216753"></polygon>
        </g>
    </g>
</svg>

[hahmed]

Awesome, thanks @p8 I have updated and added a screenshot of the new svg logo. Does that look ok now?

[p8]

Thanks @hahmed and @sunny! This looks PR great to me.

[WaKeMaTTa]

Why I never though about it? Good job ma'men!