Page MenuHomePhabricator

Favicon links matching the defaults should not be sent
Closed, ResolvedPublicFeature

Description

Every page on Wikipedia a MediaWiki install to a root path outputs headers similar to the following:
<link rel="apple-touch-icon" href="http://en.wikipedia.org/apple-touch-icon.png" />
<link rel="shortcut icon" href="/favicon.ico" />

These headers are unnecessary because (in this case) the locations are the default location for the files concerned. They will be checked anyway by user-agents that make use of such files, so pointing to them is a waste of space (137 bytes uncompressed, ~28 bytes gzipped; about 0.2% of the page, but a more significant portion of the <head>).

Apple touch icon reference:
http://developer.apple.com/safari/library/documentation/AppleApplications/Reference/SafariWebContent/ConfiguringWebApplications/ConfiguringWebApplications.html

Shortcut icon reference:
http://msdn.microsoft.com/en-us/library/ms537656%28VS.85%29.aspx

Before these links are output, the rendering code should check that they do not match the defaults, perhaps by expanding them with wfExpandUrl and comparing the two. If so, it should not output the links. (The documentation for these variables should be updated to indicate this feature.)

The protocol and host name of the apple-touch-icon also appears unnecessary; a relative path should be sufficient to specify a path on the current server. If that is removed too, it's worth testing setting an iPhone bookmark with a https: URL, just to ensure it still works.


Version: unspecified
Severity: enhancement
URL: http://en.wikipedia.org/wiki/Main_Page

Event Timeline

bzimport raised the priority of this task from to Low.Nov 21 2014, 10:42 PM
bzimport set Reference to bz19392.
bzimport added a subscriber: Unknown Object (MLST).

Created attachment 6272
Change $wgFavicon default from '/favicon.ico' to false

User agents will look for /favicon.ico without us telling them about it. There is therefore no need to output it by default.

Documentation update required at http://www.mediawiki.org/wiki/Manual:$wgFavicon if committed.

attachment DefaultSettings.php.patch ignored as obsolete

Created attachment 6273
Don't output links if $wgFavicon or $wgAppleTouchIcon are equivalent to their defaults

If the above patch to DefaultSettings is not merged but this one is, an additional check for '/favicon.ico' != $wgFavicon may be appropriate after the null check but before the wfExpandUrl check, to save the expense of expanding.

attachment Skin.php.patch ignored as obsolete

Created attachment 6274
Unified version of patch 6272, patch 6273

Resubmitting in unified format diffed from SVN

Attached:

Reverted in r55588. Brion seems to think it won't work in Firefox, and testing in Chrome seems to say the same thing.

This does not appear to be the case here. Try http://stats.wikifur.com/ for an example. Works for me in both Chrome and Firefox, latest versions.

If you have a negative cache hit for favicon.ico for a site, that will cause the favicon not to show up if you subsequently put a favicon.ico file there but do not clear the cache, and do not explicitly indicate the presence of one in your HTML.

ayg wrote:

In theory this should work fine, at least for favicon. Originally favicon didn't have a link tag at all, it was hardcoded to /favicon.ico. So all browsers should support retrieving it from that location if there's no link tag.

Apple specifies (above) that the sitewide icon be put in the root folder; the <link> is only intended as a page-specific override.

To clarify: "that the apple-touch-icon be put in the root folder..."

sumanah wrote:

Laurence, thank you for your patch. From the comments, I infer that the open question of whether your patch works means that it still needs review, so I've added the "need-review" keyword on this bug. If you'd like to and you have time, I invite you to ensure that your patch still applies to current trunk in SVN, and possibly revise and update it if it doesn't. Thanks again.

michael wrote:

Why not drop the Microsoft file format and standardize on PNG?

<link rel="shortcut icon" href=/favicon.png type=image/png />

Internet Explorer does not support PNG files as favicons.

michael wrote:

I believe MSIE 7 and newer support png favicons. I think MSIE 6 users will have about 100 other shortcomings to worry about before they come to the missing favicon, but I have no problem with providing an .ico file too, if someone wants to make the effort.

(As long as we’re taking pains to provide proprietary formats for obsolete browsers, why don’t we start including video and audio in mpeg4 for the millions of visitors who can’t use .ogg?)

Belief is insufficient in this case. Internet Explorer does not support PNG files as favicons. (On the plus side, Windows 8 *does* support PNG files as tile icons.)
http://www.jonathantneal.com/blog/understand-the-favicon/

The Apple Touch icon part is now fixed with Gerrit change 60777.

We now have 'wgAppleTouchIcon' => array( 'default' => false, ) which means that MediaWiki will not generate links to Touch icons in HTML <head /> tag, and will make iOS use the default 'apple-touch-icon.png' file located in domain docroot, which it looks for by default anyway.

Project-specific icons, and icons for projects without a docroot (for instance $lang.wiktionary.org) should be committed into docroot/bits/apple-touch/ and defined in InitialiseSettings.php, for instance:

'dewiktionary'  => '//bits.wikimedia.org/apple-touch/wiktionary/de.png',

Some good ideas about the actual creation of such icons are gathered in bug 27911.

Jdlrobson claimed this task.
Jdlrobson subscribed.

Please reopen if there is still a problem here that we need to be concerned with. No activity since 7 years ago.

GreenReaper renamed this task from Favicon, apple-touch-icon links matching the defaults should not be sent to Favicon links matching the defaults should not be sent.Jan 13 2021, 12:17 AM
GreenReaper reopened this task as Open.
GreenReaper lowered the priority of this task from Low to Lowest.
GreenReaper updated the task description. (Show Details)

The apple-touch-icon no longer appears to show up in the page source (so, this is no longer impacts T29911 and is probably not Mobile-related), but it looks to me like the default "shortcut icon" is still served on a MediaWiki install. As I understand it, this is still unnecessary because that location will be checked automatically. As such, its removal in such cases remains a potential enhancement. I lowered the priority on the grounds that the apple touch icon portion was longer (more bytes) and was resolved.

(For Wikipedia, it seems this has been set to point to "/static/favicon/wikipedia.ico", so it is no longer using "the default". However, accessing /favicon.ico appears to work, so it is possible that the line is still removable to save a few bytes for Wikimedia. Testwiki likewise uses "/static/favicon/black-globe.ico", but accessing /favicon.ico still serves that file in the same way. Accessing /apple-touch-icon.png likewise serves the specified file in the /static/apple-touch/ directory. These might all be a separate issue relating to WMF config; I could see these references being retained for clarity.)

The favicon can be disabled with $wgFavicon = false
So if I understand correctly, the ask here is that if the favicon is defined as '/favicon.ico'; it is redundant?
If so, wouldn't changing the default to false suffice as a solution here, or am I missing something?

I've tagged a couple of teams that may have input here. I should note definitely not a high priority, just keen to find closure on old tasks like this one dating back from 2009 :)

Krinkle removed a subscriber: wikibugs-l-list.
Krinkle subscribed.

Untagging WMF-specific components, since this is no longer about Wikipedia's configuration. We use non-default values for both of these.

It seems reasonable indeed to let the skin omit these elements in the head if they hold the browser-default value by exact string match (no wfExpandUrl needed). There is no need to change configuration defaults or anything for that, just an inline equality check will do.

Aklapper changed the subtype of this task from "Task" to "Feature Request".Feb 4 2022, 11:02 AM

Proposing for Hackathon. I'd say we can actually do both. While WMF does use non-standard paths today, there is no reason for it to do so since we have favicon.ico proxied to /w/favicon.php in wmf-config with far-future cache expiry headers, which we need regardless because of the various conditions under which browsers can't know about custom icon locations. Given that, we should just use that publicly instead of having both.

However works on it, I'd be happy to guide and deploy relevant wmf-config improvements and MediaWIki core improvements.

Change #1023513 had a related patch set uploaded (by Alistair3149; author: Alistair3149):

[mediawiki/core@master] Don't output links if $wgFavicon or $wgAppleTouchIcon are equivalent to their defaults

https://gerrit.wikimedia.org/r/1023513

Change #1023513 merged by jenkins-bot:

[mediawiki/core@master] OutputPage: Omit browser default <link> for $wgFavicon or $wgAppleTouchIcon

https://gerrit.wikimedia.org/r/1023513

TheDJ assigned this task to alistair3149.