Give site admin ability to upload favicon in Settings, General

[jane]

WordPress has come a long way in terms of making it possible for someone completely non-technical to create a professional web site for a person or business. One of the little things that is still annoyingly technical is adding a favicon. WordPress.com does this via the Blavatar feature, a name I wouldn't really want us to adopt b/c this is more CMS-oriented, but the ease of uploading an image, scaling and cropping it, then having it become the favicon is something I do want to adopt. Keeping it a theme-based thing means the non-technical can't control it for their sites, which is lame.

Not specifically related, but possibly relevant: http://core.trac.wordpress.org/ticket/3426

Plugin material?

Replying to Denis-de-Bernardy:

Plugin material?

+1

I'm not sure that this is core material but I can see that from a CMS point of view it is a missing feature.

As for the Plugin Material question - does a popular plugin exist for this?

Replying to westi:

As for the Plugin Material question - does a popular plugin exist for this?

Dunno about popularity, but users seem to have 85 options to choose from... :-)

​http://wordpress.org/extend/plugins/search.php?q=favicon

Scaling images down to favicon size will have very mixed results. What if v1 just allowed them to specify a URL?

This seems more pertinent now that we have the "blavatar" appearing in the slimmed-down network bar (profile drop-down shows blogs you're a member of). Screenshot: ​http://cl.ly/0K2y0m423q133q092b0R

I admittedly have not looked very deeply but there is no way to set this AFAIK.

This is still a missing feature. Every time I see some small business owner super excited about how they now control their online identity with WP, but there's a generic globe in the browser tab instead of their own chosen image, I cringe. We should be better than this, and it shouldn't require a plugin.

add_theme_support( 'favicon' );

Core then provides a UI to upload/specify a favicon. Once specified, we generate the <link> tag automatically.

Theme support is desired basically to ensure that the theme avoids handling/printing an existing favicon. However I don't necessarily find this to be a requirement -- we likely can echo out a favicon without much side effect. Unsure if browsers rely on the last favicon specified.

(We could also even intercept /favicon.ico requests via .htaccess if we're installed at the root and rewrite it to the uploaded file, and thus specify a proper favicon.ico file in the <link>. As it is, though, core does provide for an early exit with a 404 via PHP.)

Ultimately, though, we're going to need a DB option. In particular, because my-sites and the admin bar will need to do get_blog_option() against each blog. Any theme support will not be detectable. Perhaps we could introduce get_blog_options() (or perhaps a deliberately more cryptic name) that attempts to fetch all of the options we need for a single site in a single query.

As an actual implementation, is favicon something for Settings, or for Appearance? Our cognitive approach here will affect our code implementation.

From a host perspective, we get so many hits/"checks" for favicon.ico and favicon.gif that we are actually placing empty files for them in each new domain's directory to lower potential load due to the 404s that would otherwise occur.

This just to say -- I'd be cautious with having PHP do any of the redirection for favicons due to the frequency in which they're hit both by bots and users.

@nacin: "Unsure if browsers rely on the last favicon specified." Seems so, based on experimenting with switching between Pagelines' Platform theme and Twenty Eleven.

When I go to WordCamps or meet business owners, most of them don't even know the word favicon, but they know they want that image to be theirs, not a g or a globe or a blue e. This fits squarely with the stated goal of 3.4 to make your site look the way you want. I don't really care if v1 is something as full-on as wordpress.com's blavatar feature or something as simple as providing a way to specify a URL, but using the uploader with some text about size requirement would be the best user experience.

@nacin: "As an actual implementation, is favicon something for Settings, or for Appearance?"

Settings, General. Though it's appearance-related, from user perspective this is completely tied to the site name in the browser tab and should be edited in the same place.

I think @Jane is right - there will be users trying to upload 100k pixels and complaining about the results.

+1 to having this in core.

It is not Plugin territory, and it is definitely not Theme territory.

+1 to add_theme_support( 'favicon' )

The WPTRT folks will be following this one closely, as its implementation will solve one (albeit minor) headache for us.

Having it in core would be great for MultiSite users, too, and NOT in themes means a site on a network could keep their favicon while switching through themes.

Replying to Ipstenu:

Having it in core would be great for MultiSite users, too, and NOT in themes means a site on a network could keep their favicon while switching through themes.

Exactly. Changing your theme doesn't affect your site name, so why would it affect your logo? It's an image, but it's content, not presentation.

Replying to jane:

This is exactly why it is definitely not Theme territory. I would go even farther, to say that the favicon isn't just content, but rather identity.

Replying to Ipstenu:

+1 Theme

My B2B company uses Multisite+Domain Mapping as a CMS. Each site represents one of our publications. We would want to have different favicons for each theme. Although, we know how to override it in HTML if needed.

Replying to RMarks:

Maybe we can have option for Multisite, to allow use different favicons or just use one general.

+1 for an individual favicon for each site in multi-site with fallback to default favicon as network-setting

What does the world of browser support look like for favicons?

I know that we no longer have to be an ICO file, for example, and it doesn't need to be 16x16 anymore either. So what are the limitations that should be set on the resulting icon, for maximum compatibility across browsers?

We built a plugin that allows a user to upload any image and convert it to favicons. (​http://wordpress.org/extend/plugins/favicon-generator/). I've been retooling the interface because it's long overdue for an update, so maybe it makes sense to stop doing that and refocus our efforts on getting that functionality into core.

One complaint that we get constantly is that favicons don't update right away because of the way browsers cache them. Explaining to a non developer that they have to clear their cache doesn't generally go over very well.

Replying to Otto42:

What does the world of browser support look like for favicons?

I know that we no longer have to be an ICO file, for example, and it doesn't need to be 16x16 anymore either. So what are the limitations that should be set on the resulting icon, for maximum compatibility across browsers?

​http://en.wikipedia.org/wiki/Favicon#Browser_implementation

F'ing IE... (grumble)

Long story short, IE still only supports ICO files, and only 32x32 and 16x16 resolutions.

Every other browser can use PNGs, have alpha channel support, any resolution, etc.

Note: code to output ICO file format from PHP exists in this project: ​http://phpthumb.sourceforge.net/

What do we feel about supporting other devices (iPhone / Android). Markup is slightly different, according to the Wiki article: ​http://en.wikipedia.org/wiki/Favicon#Device_support

Multisite

One big headache I've had some problems with were (non-multisite) sub-directory installs automatically inheriting the parent domain's favicon, so I can't even imagine the issues that might spawn from multiple subfolder installs. So as long as we had the ability to delineate between the sites and their favicons that would be cool.

Core

Multisite aside, inclusion in Settings > General would be great. +1

I think we can do this using a <link rel="SHORTCUT ICON" href="​http://www.mydomain.com/myicon.ico"/> tag in the header which is supported by all of the brosers[1]. One thing we will need to consider is:

User uploads a .png . We convert it to a .ico file so it works in all browsers. Should we do some simple browser sniffing to serve the png if the user is in a browser that supports it and fall back to the ico for browsers that don't.

​http://msdn.microsoft.com/en-us/library/ms537656(VS.85).aspx

If you're going to support IE and add ICO files for them, don't use browser sniffing.

You can use code like this:

<link href="http://example.com/favicon.ico" rel="shortcut icon" type="image/x-icon" />
<link href="http://example.com/favicon.png" rel="icon" type="image/png" />

This appears to work fine to me as long as the "shortcut icon" with the ICO file is first in the HTML.

Alternatively, the really correct way to do this is with conditional tags.

<!--[if IE]>
<link href="http://example.com/favicon.ico" rel="shortcut icon" type="image/x-icon" />
<![endif]-->

Non-IE browsers will then ignore the ICO file.

By the way, to whomever takes the lead on this one: I'm not on any of the other teams, and would be happy to help contribute code.

Is there a benefit to using png files for favicons instead of ico files other than not having to convert to a different file format?

Replying to brandondove:

Is there a benefit to using png files for favicons instead of ico files other than not having to convert to a different file format?

• Alpha channel support allows for transparency, which looks better in the browser because you can let the background color through.

• You can use any size instead of being limited to 32x32 or 16x16.

• PNGs are easily supported. Writing ICOs is kind of a pain.

PNGs are more standard. ICOs are weird. It's not hard to do both if this is done.

Replying to Otto42:

• Alpha channel support allows for transparency, which looks better in the browser because you can let the background color through.

Because we're letting the user upload any image format and doing the resize/crop, it's unlikely we'll want to support transparency. Unless we have some kind of PNG pass through option, whereby if they happen to upload a 24bit PNG, they get the option to save the raw (downscaled) PNG in addition to the ICO. That seems like an enhancement rather than a first release feature though.

• You can use any size instead of being limited to 32x32 or 16x16.

I"m not sure I see how this works - after all, the browser chrome doesn't allow for a larger icon, at least not in any browser I'm familiar with. IE, Chrome, FF etc all put the favicon inside the address bar, and that ain't going to grow.

• PNGs are easily supported. Writing ICOs is kind of a pain.

Yes, but with the lib you point to in an earlier comment this should be moot.

PNGs are more standard. ICOs are weird. It's not hard to do both if this is done.

We could certainly consider saving out both versions and the serving up the ICO based on conditional tags. I suppose the process should be:

1. upload
2. crop if necessary
3. resize if necessasry
4. save out to PNG with imageMagick
5. convert to ICO

Replying to TomAuger:

4. save out to PNG with imageMagick

Probably shouldn't just assume that everyone has imageMagick installed -- none of my servers do. Is this something that can be handled with existing Core functionality?

We use a set of PHP functions to convert various image file formats to ico files. It would allow us to bundle the necessary functions in core. They use a combination of the GD lib and some bit swapping functions to handle the conversion. I'll attach the library so you can all check it out. I talked to the original author of the library and he was fine with us including it in our plugin on the repo.

Replying to brandondove:

I talked to the original author of the library and he was fine with us including it in our plugin on the repo.

I'm not 100% sure, but I believe the library would need to be GPL2 in order to be included in WP core.

It needs to be GPL2 compatible to be in a plugin in the repository as well. Just the author's permission isn't good enough.

So.. you might want to double check on that one.

Replying to Otto42:

It needs to be GPL2 compatible to be in a plugin in the repository as well. Just the author's permission isn't good enough.

So.. you might want to double check on that one.

From the header of the php library:

/*

• License:
• - you can freely use it
• - you can freely distribute sourcecode
• - you can freely modify it as long as you leave my copyright/author info in source code
• - if you developing closesource application, you should add my name at least to "about" page of your web application
• - if you create an amazing modification, please contact me... I can publish link to your webpage if you're interested...
• - if you want to use my script in commercial application for earning money, you should make a donation to me first */

It doesn't specifically say GPL though. I'll see if I can track him down and get him to specify GPLv2 as the license type.

Before Otto's comment, I deleted ico2_1.php because its "license" is incompatible, or at the very least, too vague. brandondove, you should get him to re-release it as GPL, MIT, or some other GPL-compatible license to ensure that you can still use it in a plugin.

24	 * License:
25	 *      - you can freely use it
26	 *      - you can freely distribute sourcecode
27	 *      - you can freely modify it as long as you leave my copyright/author info in source code
28	 *      - if you developing closesource application, you should add my name at least to "about" page of your web application
29	 *      - if you create an amazing modification, please contact me... I can publish link to your webpage if you're interested...
30	 *      - if you want to use my script in commercial application for earning money, you should make a donation to me first

Status update - currently working: generating the proper HTML for the header based on an (as yet) textfield option that can be set under Settings > General. In my dev environment I still have to manually upload the PNG/ICO file to the media library. I don't think this is worth putting up a patch yet. Just thought I'd post an update.

Now for the fun part - the actual upload DB. I intend to replicate the UX of "set featured image". The challenge is that a lot of the thumbnail upload stuff is hard-coded specifically to tie it in to a post (references global $post_id, etc). This has been a pet peeve of mine for some time, happy to have an excuse to refactor that part of media.php.

Replying to nacin:

Before Otto's comment, I deleted ico2_1.php because its "license" is incompatible, or at the very least, too vague. brandondove, you should get him to re-release it as GPL, MIT, or some other GPL-compatible license to ensure that you can still use it in a plugin.

Understood. I have extended an invitation for him to get involved, so we'll see what he says. I'll post an update when I have one. While we wait, is this kind of a code library something that we'd even consider using for this functionality, or are we truly looking to only support png files for newer browsers?

According to this link: ​http://phpthumb.sourceforge.net/demo/docs/phpthumb.license.txt PHPThumb is available under GNU2. It purports to being able to do an ICO export, and we probably don't need more than ​http://phpthumb.sourceforge.net/index.php?source=phpthumb.ico.php to make it work.

Working on a non-AJAX implementation first. The UI is trickier than it first appears. If we avoid the right-hand metabox that is used on the .com blavatar implementation, we get into a collision with the Settings > General form - that is, we need a multipart form to handle the file upload, but you can't nest forms.

Options:

1. Use the metabox after all so its form can live in its own section of the HTML code
2. Put the upload / favicon option above or below the other General options so it can have a different form
3. Put it inside the General settings form, change that form to use multipart, and do the file processing within the code that handles the updating of the general options. I like this approach the least since it could really get messy, and it isn't immediately obvious to the user that clicking "upload" button would also save any other settings they may have accidentally messed with in the Options.

For the time being, putting it on the top of the form (option #2) seems like the easiest approach, but is that giving the favicon too much importance?

Put it at the very top for now. Once that works, we'll shoehorn it to work how we want it to end up.

Has ​https://wpcom.automattic.com/ticket/1294 been resolved?

Replying to TomAuger:

Has ​https://wpcom.automattic.com/ticket/1294 been resolved?

Tom is referring to the avatar-upload code on WP.com, which he has and I will upload here. I wouldn't worry about that — the comment indicates that it is simply an image replication thing having to do with Gravatar, multiple servers, heavy caching, etc. I'd ignore it.

Note - if my (forthcoming) patch gets committed, it also includes the '.ico' MIME TYPE fix from this ticket: http://core.trac.wordpress.org/ticket/11824, so let's remember to circle back and mark it as complete when the time comes.

Early patch with basic add image functionality

So here's what we have so far:

• Rudimentary favicon image upload under Settings > General.
• Crop and save out a .PNG and .ICO file (.ico file format is currently incorrect and does NOT work in IE)
• Original upload AND both thumbs are stored as attachments
• Hooks into wp_head and admin_head to add the x-browser HTML that (once the ICO format is fixed) works

Immediate TODOs before we could even consider releasing this:

• clean up the Settings UI and display the uploaded thumb
• Add the Remove feature
• fix the ICO format or provide a workaround for IE

Later:

• encapsulate in a class
• some error trapping was skipped (marked in TODOs)
• slicker upload UI (at least have javascript click the "upload" button once you've selected a file!
• admin notice or some other method to "close the circle" and let the admin know the favicon was uploaded successfully.

Replying to nacin:

Before Otto's comment, I deleted ico2_1.php because its "license" is incompatible, or at the very least, too vague. brandondove, you should get him to re-release it as GPL, MIT, or some other GPL-compatible license to ensure that you can still use it in a plugin.

JPEXS re-released his ImageICO php library under the GPL. Details can be found here: ​http://www.jpexs.com/php.html. I'll also attach the updated library with corrected license.

ImageICO PHP library

Replying to brandondove:

Replying to nacin:

Before Otto's comment, I deleted ico2_1.php because its "license" is incompatible, or at the very least, too vague. brandondove, you should get him to re-release it as GPL, MIT, or some other GPL-compatible license to ensure that you can still use it in a plugin.

JPEXS re-released his ImageICO php library under the GPL. Details can be found here: ​http://www.jpexs.com/php.html. I'll also attach the updated library with corrected license.

GPLv3 is incompatible with WordPress's license, GPLv2. Sorry I wasn't clear in my initial reply.

Replying to nacin:

GPLv3 is incompatible with WordPress's license, GPLv2. Sorry I wasn't clear in my initial reply.

"The license under which the WordPress software is released is the GPLv2 (or later)..." => ​http://wordpress.org/about/license/

"The license under which the WordPress software is released is the GPLv2 (or later)..." => ​http://wordpress.org/about/license/

Basically: That means, that any software integrated into the core WordPress package must be compatible with GPLv2 AND later, however, WordPress can be used in conjunction with other software under the GPLv2 OR later

Replying to GaryJ:

Replying to nacin:

GPLv3 is incompatible with WordPress's license, GPLv2. Sorry I wasn't clear in my initial reply.

"The license under which the WordPress software is released is the GPLv2 (or later)..." => ​http://wordpress.org/about/license/

Not for plugins, see: http://core.trac.wordpress.org/ticket/16898

I did a little work building upon TomAuger's patch. We are generating some notices, so eliminating those is going to be my next step.

Note that the wp-facvicon.js file is just a copy of the .dev version so that you don't need to have SCRIPT_DEBUG set to true in order to use the patch. Once we have something committable, we'll need to generate the proper file.

UX wise:

I think we should consider is having the upload take place in a lightbox. The experience of uploading a file and having that jump you to a new page seems wrong to me.

Excellent. What do you think about letting .bmp files through the uploader on the JS side (I've let it through on the PHP side). My thinking is that for those users that have legitimate favicons already generated (perhaps from their old, HTML site), they would probably want to just upload them and let them pass-through. That means possibly also supporting a .ico upload.

Okay, what's the final answer on a .ICO converter library? Can we use JPEXS' library (GPL v3) or not, Licensing is not my area of expertise. Would like a final, canon answer on this one - the "GPL V2 or later" to me sounds like it is inclusive of GPL v3 and therefore should allow JPEXS' library.

To be TESTED: does the current PNG upload process preserve transparency (ie: allow for non-square icons)?

I'm not going to be working on this ticket today (Feb 21) but will pick up again tomorrow. Jorbin - do you have experience integrating the Lightbox UI? I will continue with the Remove functionality and general cleanup stuff.

Replying to TomAuger:

Okay, what's the final answer on a .ICO converter library? Can we use JPEXS' library (GPL v3) or not, Licensing is not my area of expertise. Would like a final, canon answer on this one - the "GPL V2 or later" to me sounds like it is inclusive of GPL v3 and therefore should allow JPEXS' library.

Canonical answer: We can not use GPLv3 code in the core. All code in core must be GPLv2 compatible. We release WordPress under the "GPLv2 or later" license, and this is not inclusive. We cannot re-release somebody else's code under a different and incompatible license.

Replying to Otto42:

Replying to TomAuger:

Okay, what's the final answer on a .ICO converter library? Can we use JPEXS' library (GPL v3) or not, Licensing is not my area of expertise. Would like a final, canon answer on this one - the "GPL V2 or later" to me sounds like it is inclusive of GPL v3 and therefore should allow JPEXS' library.

Canonical answer: We can not use GPLv3 code in the core. All code in core must be GPLv2 compatible. We release WordPress under the "GPLv2 or later" license, and this is not inclusive. We cannot re-release somebody else's code under a different and incompatible license.

I've asked jpexs if he's willing to re-release it (again) under GPLv2. My guess is it won't be a problem as his intent is to openly share the library.

Replying to TomAuger:

Excellent. What do you think about letting .bmp files through the uploader on the JS side (I've let it through on the PHP side). My thinking is that for those users that have legitimate favicons already generated (perhaps from their old, HTML site), they would probably want to just upload them and let them pass-through. That means possibly also supporting a .ico upload.

If the backend supports it, we can let it in. I more so wanted to add in some basic checking in order to not make users have to go back if they upload the wrong file. Feel free to edit the list in the JS or just let me know what you want.

I'm going to pick this up tomorrow. I have no problem adding in the lightbox experience once we have everything ironed out.

Basic UX flow that I'm planning -

1) User initiates the process by selecting a file.

2) If the file extension is supported, continue. Else bail and message why.

3) Lightbox appears wich contains the submitted image and allows the user to crop the image

4) Upon completion, the icon will apear next to the image upload form so that the user knows what they have selected.

Update includes new API calls to generate the thumbnail IMG tag, verify existence of a custom favicon, and get the favicon file path. Refactoring to some functions from previous patch.

Replying to jorbin:

Basic UX flow that I'm planning -

1) User initiates the process by selecting a file.

2) If the file extension is supported, continue. Else bail and message why.

3) Lightbox appears wich contains the submitted image and allows the user to crop the image

4) Upon completion, the icon will apear next to the image upload form so that the user knows what they have selected.

Re: 4) Check my latest patch, esp. options-general.php for use of has_custom_favicon() and get_favicon_thumbnail_img()

Next I will be adding the Remove functionality. I suppose I should add a little JS for a confirmation "are you sure?" box.

Nitpick: Why is that function called get_favicon_thumbnail_img()? Shouldn't it be just get_favicon_img()?

GPLv2 version of ImageICO php lib

Just uploaded version 2.3 of the ImageICO php library that has been re-released under the GPLv2 license. As always, you can check the details at jpexs website: ​http://www.jpexs.com/php.html.

Replying to scribu:

Nitpick: Why is that function called get_favicon_thumbnail_img()? Shouldn't it be just get_favicon_img()?

Yep. Will do.

Replying to brandondove:

Just uploaded version 2.3 of the ImageICO php library that has been re-released under the GPLv2 license. As always, you can check the details at jpexs website: ​http://www.jpexs.com/php.html.

This is great. Thanks for championing this. We need to make sure that if we do end up using this library, it's acknowledged in the credits along with the other libs that are used. Might be nice to do a blog post about the new favicon feature and credit the library author.

Before we go too far, I'll try rolling it in to the next patch and let's see if it does what we need it to do.

ImageICO library works great as advertised. Only concern is that it uses fopen() and fwrite() to create the local file, and I wonder whether this could be a problem.

If this is a concern, then there are two strategies:

1. edit/change the library to use more WP core methods
2. capture the ICO on stdout and save it that way.

Any thoughts? I could use some guidance on the best practice here.

Replying to TomAuger:

ImageICO library works great as advertised. Only concern is that it uses fopen() and fwrite() to create the local file, and I wonder whether this could be a problem.

If this is a concern, then there are two strategies:

1. edit/change the library to use more WP core methods
2. capture the ICO on stdout and save it that way.

Any thoughts? I could use some guidance on the best practice here.

+1 to integrating the WordPress Filesystem API into the library.

Replying to chipbennett:

+1 to integrating the WordPress Filesystem API into the library.

So you don't think it would be a problem to refactor that library / ie: alter the source code? (it could use some rehabilitation IMHO)

I'm not that familiar with licensing, so my question is maybe more about that - is it okay to modify the source code of that library? What is the downside of doing that?

Replying to TomAuger:

Replying to chipbennett:

+1 to integrating the WordPress Filesystem API into the library.

So you don't think it would be a problem to refactor that library / ie: alter the source code? (it could use some rehabilitation IMHO)

A problem in what sense: license, or implementation? If it's GPLv2, then license isn't an issue; we can modify it however we want. If it's ease of implementation, I don't know. Probably not something I have the chops for. Groking the Filesystem API is on my list of things to do, but it probably won't happen in time for the 3.4 release.

I just think that, 1) as a matter of principle of using core APIs were possible, and 2) as a matter of not introducing server configuration issues for end users, using the Filesystem API is the right approach.

The Filesystem API doesn't fit here. If we can't take an uploaded PNG and write out ICO, we use the PNG. This is the uploads directory — If we can upload the PNG, we can likely write the ICO to the same place. (The library *could* use an is_writeable() check, and/or some sort of error handling or suppression on the fopen(), but we can let it slide during testing.)

The Filesystem API is only designed for critical tasks. Updates, installs. Things like uploads are not critical and therefore avoid the filesystem API. It's also why permissions are going to be more lax on the uploads directory in many cases, because we need to be able to write files here in a normal process, rather than asking for your FTP credentials. (Not even sure how that'd work anyway.)

I'm not that familiar with licensing, so my question is maybe more about that - is it okay to modify the source code of that library? What is the downside of doing that?

From a licensing perspective, yes. From a maintenance perspective, we try not to. And when we do, we carefully consider why, and even then, we try to float the patch upstream.

So you don't think it would be a problem to refactor that library / ie: alter the source code? (it could use some rehabilitation IMHO)

This library has seen better days. I would not be comfortable including it in core in its current state, unless it receives an audit, at the very least.

Doing some research, it looks like ImageMagick supports creating ICO files. We should probably just use Imagick if it exists, and fallback to PNG-only if it does not. Here's some stuff on php.net ​on ICO to PNG, for example.

I think our efforts are best spent aimed at getting the PNG upload process polished and squared away. If we can generate ICO files at a later point, woo, but that is not necessary for version 1.

The only problem with only supporting PNG is that the entire Internet Explorer userbase would be alienated. This doesn't seem like the "WordPress Way" to me - rolling out a feature that just won't work for a popular browser.

Would we consider releasing the next version with a default WordPress favicon in place (possibly in the root directory) in .png and .ico format? A fallback for when you have not defined one yourself in Settings > General? This would also help with a lot of 404's that servers are getting.

Replying to TomAuger:

Would we consider releasing the next version with a default WordPress favicon in place (possibly in the root directory) in .png and .ico format? A fallback for when you have not defined one yourself in Settings > General? This would also help with a lot of 404's that servers are getting.

No, a few reasons: One, it is difficult to then go ahead and remove that file (otherwise, some browsers will still read it). Two, there's really no reason for a default WordPress favicon on every WP.org site out there. Three, it only works for root-level sites. Four, we already intercept root-level /favicon.ico requests early and return a zero-length image/vnd.microsoft.com response, so 404s are not going to end up being logged if external rewrite rules are in place to map everything to index.php. Some hosts have simply placed favicon.ico files in root directories to even prevent WordPress from loading in these cases, but I don't think that's our responsibility beyond what we already do.

I agree that supporting ICO should be the goal. Let's just not catch ourselves up over it. Still need to build a working feature. :-)

Included ico2_3.php, "remove" functionality, and refactoring of get_favicon_thumbnail_img() to get_favicon_img()

Okay, I'll need some guidance as to what to work on next, shooting for that 29th freeze. Not sure what jorbin is working on for the freeze, but I don't want to get into any duplication of effort. I'm feeling good about where we're at, functionality wise.

Replying to nacin:
You snuck yourself in there before my patch was up, so you'll see that the ico2_3.php is included (and works fine). At this point we have a working (if somewhat blunt) favicon solution. What's next?

Replying to nacin:

Doing some research, it looks like ImageMagick supports creating ICO files. We should probably just use Imagick if it exists, and fallback to PNG-only if it does not...

+1. Also not sure if there should be any support for image manipulation after uploading. The problem is that at such small sizes (16 x 16) most images don't look good. It doesn't matter if they are resized in PHP or by the browser on display.

So an "unaware" user will not be able to add a decent looking favicon by uploading an arbitrary image. Imho it would be much better to include some help text about favicons in general and the requirement for using .ico format image for best browser compatibility. There are many ways a user can generate decent looking .ico file, including online image editing websites that handle exactly that.

Replying to azaozz:

The problem is that at such small sizes (16 x 16) most images don't look good. It doesn't matter if they are resized in PHP or by the browser on display.

Are you suggesting we only allow upload of 16 x 16 or 32 x 32 sized images, and don't allow for crop or resize?

I can see your arguments for this approach. However, all the images (photographic or otherwise) that I have used during the testing have turned out remarkably well. The most common things people will use would be their company logo, or a photo of themselves (ie: mugshot). Both of these would render quite decently with the antialias + interpolation settings that are currently in place in the resizing process.

I don't mind the idea of adding a "help" icon that would provide tips, or some contextual help on the General settings page that talks about good favicons, but it seems to me that it's out of scope for us to include training about how to make your site look good inline or onboard. Otherwise one could argue that we get into a slippery slope where we're now advising on content, so should we also provide tips on how to make a good masthead or background image, or write a good post title?

I believe that for the 80% we give them the ability to upload anything. Worst case scenario, they have a custom ugly favicon instead of the default ugly favicon; more likely they will have a mediocre custom favicon of their face, or their kid, or their logo. If they're smart, they'll take the text out of their logo and just use the icon.

And for the 20%, let's make sure that if we do get an icon in 16x16 or 32x32, or if it's already in ico format, it doesn't get interpolated or otherwise mutilated. So designers can do it themselves with their Photoshop ICO plugin, or some online service.

Replying to TomAuger:

Are you suggesting we only allow upload of 16 x 16 or 32 x 32 sized images, and don't allow for crop or resize?

Not exactly. Best probably would be to expect properly sized images. If the user uploads something above 32x32, it can be cropped and resized automatically and saved as .png (and .ico if possible). We need to allow uploading of favicons and properly include/link them, not make a tool for creating tham :)

I can see your arguments for this approach. However, all the images (photographic or otherwise) that I have used during the testing have turned out remarkably well...

Yes, many would turn well but many would fail too. It would be better if the user knows what the image looks like before uploading.

I don't mind the idea of adding a "help" icon that would provide tips, or some contextual help on the General settings page...

Yes, I meant a tab in the contextual help, probably with a short sentence under the "Upload" button that will send the user there. We won't need to add a lot of text, only 1-2 sentences explaining that favicons are very small images and should be generated by software or on particular websites.

I believe that for the 80% we give them the ability to upload anything.

Right, but also tell them what a favicon is and how they can make a good looking one.

And for the 20%, let's make sure that if we do get an icon in 16x16 or 32x32, or if it's already in ico format, it doesn't get interpolated or otherwise mutilated.

Agree. If an .ico image is uploaded or if the image is already 32x32 we shouldn't touch it at all (don't think we can touch .ico in GD anyways).

remove ico library, clean things up a bit, add in nonces

I took Nacin's advice and aimed for simplicity for v1.

my patch removes the ico2_3 library because reading it made me cry. My patch is based on Tom's and includes nonces and allows the full process ( image upload > image crop > image display by the theme) to take place error free. I think v1 is ready to go in.

This still will need a bit of a UI pass by someone with a better eye then I.

Just my 2 cents:

• echo <<< ... do we really want this?
• instead of FAVICON_SIZE I would prefer apply_filters( 'favicon_size', 32 )
• Because the image will be uploaded to the media library, the image should be marked, see _media_states()
• If I delete the favicon from the media library the options should get reset accordingly (like for theme mods _delete_attachment_theme_mod())
• And should we delete the original image after cropping?
• favicon-upload.php should set a $title

Replying to jorbin:

I think v1 is ready to go in.

Right on Jorbin, this is dynamite.

This still will need a bit of a UI pass by someone with a better eye then I.

I'll get on the UI and will post an update in a few.

Replying to ocean90:

Just my 2 cents:

thanks for the awesome advice, see below...

• echo <<< ... do we really want this?

What's wrong with the heredoc approach? Not that it's THAT many lines of code, but seems quite readable to me.

• instead of FAVICON_SIZE I would prefer apply_filters( 'favicon_size', 32 )

Right you are. Consider it done.

• Because the image will be uploaded to the media library, the image should be marked, see _media_states()
• If I delete the favicon from the media library the options should get reset accordingly (like for theme mods _delete_attachment_theme_mod())

Both of these sound like best practices. I know nothing about either of these methods, so I will need to research a bit before committing to getting them into my next update.

• And should we delete the original image after cropping?

I don't know the answer to this one. It depends on what the philosophy behind the Media Library is - do we keep a record of the full-size image of everything that goes into the library? Or is this a different case and the original scaled document is a throwaway - all we wanted was the smaller size?

• favicon-upload.php should set a $title

Tru dat.

What's wrong with the heredoc approach? Not that it's THAT many lines of code, but seems quite readable to me.

We don't use heredoc or nowdoc anywhere in core. Not saying we shouldn't. We just haven't found a use for it.

Just some comments based on Jorbin's patch.

1. Looks like when we upload a new image, it's not only creating the 32x32 icon PNG but also any number of additional thumbnail versions based on registered thumbnail sizes. This seems to follow standard practices for the Media Library, but I wonder whether this is appropriate here or not? Just a question really.

2. Just to confirm - Jorbin, you dropped the idea of lightboxing the crop process? I didn't see any change to that workflow from my previous patch.

Replying to ocean90:

• favicon-upload.php should set a $title

The title is currently the attachment filename. What should the title be? I'm not sure how the title field of wp_posts should be used in this context.

Replying to tomAuger:

Ah, sorry, I mean $title for <title></title>:

if (! current_user_can( 'manage_options' ) )
    wp_die( __( 'Cheatin’ uh?' ) );

check_admin_referer( 'favicon_upload-options' );

$title = __( 'Crop Favicon' );
$parent_file = 'options-general.php';

if ( isset( $_POST['CROP_AND_SAVE'] ) ) {

What's wrong with the heredoc approach?

What Nacin said.

Replying to ocean90:

Replying to tomAuger:

Ah, sorry, I mean $title for <title></title>:

if (! current_user_can( 'manage_options' ) )
    wp_die( __( 'Cheatin’ uh?' ) );

check_admin_referer( 'favicon_upload-options' );

$title = __( 'Crop Favicon' );
$parent_file = 'options-general.php';

if ( isset( $_POST['CROP_AND_SAVE'] ) ) {

Right on. Done.

What's wrong with the heredoc approach?

What Nacin said.

Hmmm. I find popping in and out of <?php ?> way more distracting than <<< but that's my Perl backgrouond speaking no doubt. Cool, we'll do it that way.

I just saw, that you use the basename as the value for sitefavicon, can't we just use the attachment ID or better, the complete URL?

I asked because of this line

$posts = get_posts( array( 'name' => $favicon_fullname, 'post_type' => 'attachment' ) );

Few more things while testing 16434.2.diff:

• When uploading a "favicon.ico" file, it's uploaded but is not set as favicon. Instead there's the error: "Please only use PNG (.png), JPEG (.jpg) or BMP (.bmp) image files for favicons." (this statement is wrong by itself).

• When uploading an already resized 32x32 image it still goes to the crop screen, would probably be better to skip that step and set the image directly as favicon, converting it to .png if needed.

• The UI on the General Settings screen should probably be after "E-mail Address" or at the bottom. Having it at the top doesn't "feel right".

Also using thickbox wouldn't be advisable. There are plans for scrapping thickbox and modal dialogs in general. Perhaps better would be to insert an iframe right under the UI on the General Settings screen.

Note to my comment:105

A file named äöü.png throws out a notice and no favicon.

Replying to ocean90:

I just saw, that you use the basename as the value for sitefavicon, can't we just use the attachment ID or better, the complete URL?

Thanks for this feedback. Here's the rationale behind using the basename as the value: we need to accommodate both filename.png and filename.ico. When serving up the HTML we will (eventually) have to serve up some non-IE code referencing filename.png and then some IE-conditional code referencing filename.ico.

If we store the attachment ID or the URL, it would have to be one or the other (either the PNG or the ICO version of the file). Then, to serve up the other one, we would have to reverse-engineer the filename (strip out the TLA and substitute the correct one).

Not that that approach is all that difficult (in fact it's probably way easier to substitute the TLA at the end rather than stripping out the basename at the front, like I'm currently doing, but to me it doesn't feel "right".

Now, if there were some kind of parent attachment - perhaps a "favicon" post or something of that ilk - that both the ICO and the PNG attachment record could reference, then I would be comfortable storing that.

For now I don't feel there's a compelling reason to refactor the way it currently works, though I'm open to more opinions on the matter.

Replying to azaozz:

Few more things while testing 16434.2.diff:

thanks for the testing!

• When uploading a "favicon.ico" file, it's uploaded but is not set as favicon. Instead there's the error: "Please only use PNG (.png), JPEG (.jpg) or BMP (.bmp) image files for favicons." (this statement is wrong by itself).

Okay, I'll revise the front-end filter for now to only accept PNG and JPG, and make sure the error message reflects that.

• When uploading an already resized 32x32 image it still goes to the crop screen, would probably be better to skip that step and set the image directly as favicon, converting it to .png if needed.

I'll perform a check on the uploaded size and if it's equal to or less than the default favicon size (as set by the 'favicon-size' filter), then skip the crop step.

• The UI on the General Settings screen should probably be after "E-mail Address" or at the bottom. Having it at the top doesn't "feel right".

I'm happy to put it anywhere, however, the rationale for its position is twofold:

1. Because the favicon appears near the site title in the browser tab, and because the topmost items under Settings > General are all related to the actual website itself (as opposed to the primary user, the timezone, etc), it seems to me that the favicon is most logically grouped there. Also, since it's one of the "flashier" bits of that particular page, there's a certain marketing appeal.

2. More importantly, from a dev perspective, the rest of the page is wrapped in a FORM tag, and FORM tags can't be nested, so I'd either have to break the settings > general form tag, insert the favicon form, open a new form tag after it (which just seems crazy), or do something more sneaky like use an iFrame, which has its own issues. Now, if we do move forward with the iFrame approach for the crop section as you suggest (though that seems problematic to me, see below) then this could be a natural possibility.

I'll defer that decision until I get more feedback on the matter. Perhaps others can weigh in here.

Also using thickbox wouldn't be advisable. There are plans for scrapping thickbox and modal dialogs in general. Perhaps better would be to insert an iframe right under the UI on the General Settings screen.

The only issue I can see with this is that the crop UI is really (really) large at the moment, though arguably we could make the whole thing a bit smaller (though I wouldn't want to go less than 300x300 for the image size). So that's a pretty big chunk of real estate to just insert smack in the middle of the rest of the Settings > General form. Unless I'm not properly understanding your suggestion?

Replying to ocean90:

Note to my comment:105

A file named äöü.png throws out a notice and no favicon.

I'll take a look and see what that's all about.

Replying to TomAuger:

Okay, I'll revise the front-end filter for now to only accept PNG and JPG, and make sure the error message reflects that.

Perhaps it would be better to let the users add favicon.ico files (matching the entire file name) instead of banning uploads of .ico files.

In general letting the user upload an image, then crop it and set it as a favicon implies expectations that it would work. However that won't work in 30%-90% of the browsers (IE), depending on the region. Also if the users are in IE, the favicon won't show even for them.

Thinking the best UX here would be to include some help/explanation in the screen help tabs directing the users to upload ready-made favicon.ico files. Anything else would result in a lot of:

HELP! WP is broken, I uploaded image to show in the browser tab
but it's not working...

I'm happy to put it anywhere, however, the rationale for its position is twofold...
I'll defer that decision until I get more feedback on the matter.

Sounds good. Yes, the favicon is somewhat connected to the site name (identity) but is not "more important" than the name. Perhaps it would be better placed under the site name, tagline and address fields.

The only issue I can see with this is that the crop UI is really (really) large at the moment, though arguably we could make the whole thing a bit smaller (though I wouldn't want to go less than 300x300 for the image size). So that's a pretty big chunk of real estate to just insert smack in the middle of the rest of the Settings > General form. Unless I'm not properly understanding your suggestion?

Yeah, 300x300 or 350x350 seems large enough. The idea is to insert an iframe after uploading the image instead of using thickbox. The iframe would be pretty much the same as in TB but will be placed directly in the page rather than inserted at the bottom and shown with position:fixed in the middle or the screen. To make this work there can be a hidden iframe under the upload field with some basic JS that will $('iframe').slideDown('fast') when something is loaded there, and of course $('iframe').slideUp('fast') when not needed any more. That would make pretty sleek UI :)

Just a question while you're trying to figure out all of the files that need to be created. Since we're already uploading an image at the 300X300 or 350X350 size couldn't we output a few more files to meet apple-touch-icon support? Since we're already here, wouldn't it be easy enough to add this now? For web pages that don’t specify a custom touch icon, a thumbnail screenshot of the page is used instead. Android has a default icon, and some systems fall back to the favicon if it’s available. We could even use a set of <link rel="apple-touch-icon" href="apple-touch-icon.png"> just like favicon. iOS also supports a no HTML option by putting the files in at the root.

There's a few resolutions that can be accommodated with only one file at 114X114. That's how apple does it. But it's recommended to create files with the following names and sizes.

apple-touch-icon.png
apple-touch-icon-precomposed.png
apple-touch-icon-114x114-precomposed.png
apple-touch-icon-72x72-precomposed.png
apple-touch-icon-57x57-precomposed.png

Here's an article explaining everything (​http://mathiasbynens.be/notes/touch-icons) It's from a year ago but the information is still current. The device testing might not be.

Aren’t proprietary image formats like the Apple icon or Opera’s Speed Dial icon plugin territory? Favicons are stable, anything else is a moving target. A hook for plugin authors to add more sizes and link elements should be good enough.

Replying to toscho:

A hook for plugin authors to add more sizes and link elements should be good enough.

This sounds like a good approach, and viable for the current cycle.

Replying to azaozz:

In general letting the user upload an image, then crop it and set it as a favicon implies expectations that it would work. However that won't work in 30%-90% of the browsers (IE), depending on the region. Also if the users are in IE, the favicon won't show even for them.

The more I think about it, the more I feel strongly that releasing the favicon feature without IE / .ico support is just an out-and-out mistake. I think azaozz is right, support will be flooded, and the "build your own .ico and upload it" approach just seems too technical. If they can do that, why can't they just put the favicon.ico file in their site root and be done with it?

Doesn't make sense.

I think that releasing with some subset of ico2_3.php library, or possibly a homegrown version. All you need to do is play with encoding the bytestream, nothing too complicated or proprietary. Should be a matter of a couple hours to extract the relevant portions froom ico2_3, clean it up, add some WP error checking and best practices and roll it into favicon-upload.php.

At the rate this is all going (I mean getting concensus, not the actual development), it looks doubtful for 3.4, unless we can all agree on the exact release spec.

In the meanwhile, I'll work on cleaning up all the stuff that we DO have concensus on, and then see where we are in a day or two.

Replying to azaozz:

Yeah, 300x300 or 350x350 seems large enough. The idea is to insert an iframe after uploading the image instead of using thickbox. The iframe would be pretty much the same as in TB but will be placed directly in the page rather than inserted at the bottom and shown with position:fixed in the middle or the screen. To make this work there can be a hidden iframe under the upload field with some basic JS that will $('iframe').slideDown('fast') when something is loaded there, and of course $('iframe').slideUp('fast') when not needed any more.

Sounds good. But why create some custom JS UI components that would be unique to this feature, rather than leveraging Thickbox, that is used by the MediaCenter and other core features? I guess I just don't understand the need to roll our own here, when there's something perfectly workable.

Sorry, can you clarify that?

Okay, scrolling up, I'm noticing some unanswered issues / questions needing decisions. In an effort to hopefully bring us closer to some decisions, let me just summarize them here and see if we can get some consensus.

1. .ICO or not? If we don't support .ico at all, Explorer is out, so this doesn't seem like an option to me. So either we allow straight by-pass upload of an .ico file, or we figure out how to encode the .ico bytestream by either scavenging from sources we currently have or leveraging another library since GD doesn't support it natively.

2. _media_states() not sure I understand what this is or how it's relevant. Looking for a little guidance on that one.

3. Should we delete the original image after cropping and saving the 32x32 sized thumbs? Or do we leave the full-size attachment in the library? Do we also let it create all the other registered thumbnail sizes for the attachment?

4. Thickbox or not for the crop UI?

5. Location of the actual favicon field in Settings > General (see my comments on why it's currently on top). If we're not putting it on top (or bottom) then I'm looking for some creative suggestions to how to shim it into the page within the current form.

Replying to TomAuger:

Okay, scrolling up, I'm noticing some unanswered issues / questions needing decisions. In an effort to hopefully bring us closer to some decisions, let me just summarize them here and see if we can get some consensus.

1. .ICO or not? If we don't support .ico at all, Explorer is out, so this doesn't seem like an option to me. So either we allow straight by-pass upload of an .ico file, or we figure out how to encode the .ico bytestream by either scavenging from sources we currently have or leveraging another library since GD doesn't support it natively.

I think we need to. The library is a mess though and I think including it as-is is a bad idea. I would rather not include this in the release then include that library right now.

2. _media_states() not sure I understand what this is or how it's relevant. Looking for a little guidance on that one.

We should include it in that function so that the image is properly marked as the favicon.

3. Should we delete the original image after cropping and saving the 32x32 sized thumbs? Or do we leave the full-size attachment in the library? Do we also let it create all the other registered thumbnail sizes for the attachment?

My first thought is that we should treat it the same way as an image uploaded for the custom background and custom header. If we create the thumbnails for those, we should create the thumbnail for this.

4. Thickbox or not for the crop UI?

Not Thickbox, but also having the user leave the page seems like bad UX to me. The more I think about it, the more I think an iframe inline is going to be the best approach.

5. Location of the actual favicon field in Settings > General (see my comments on why it's currently on top). If we're not putting it on top (or bottom) then I'm looking for some creative suggestions to how to shim it into the page within the current form.

Not sure on this one.

Overall, I think we might be best developing this as a plugin that we can then bring in early in 3.5 rather then trying to force this into 3.4.

Replying to jorbin:
Thanks for this input.

1. .ICO or not?

I think we need to. The library is a mess though and I think including it as-is is a bad idea. I would rather not include this in the release then include that library right now.

So basically scavenge and build our own. I'm good with that.

2. _media_states(). Looking for a little guidance on that one.

We should include it in that function so that the image is properly marked as the favicon.

Hmmm. Still looking for more info on this one.

3. Should we delete the original image after cropping?

My first thought is that we should treat it the same way as an image uploaded for the custom background and custom header.

Okay, sounds good.

4. Thickbox or not for the crop UI?

I think an iframe inline is going to be the best approach.

Okay.

5. Location of the actual favicon field in Settings > General (see my comments on why it's currently on top). If we're not putting it on top (or bottom) then I'm looking for some creative suggestions to how to shim it into the page within the current form.

Not sure on this one.

I'ma keep it at the top for now, then.

Overall, I think we might be best developing this as a plugin that we can then bring in early in 3.5 rather then trying to force this into 3.4.

My feeling is that if we can't get the ICO library into this cycle, then we still release for 3.4, but only with a .ico upload passthrough option (no crop, only .ico files permitted). This will work across all browsers and will set the stage for something better in 3.5.

Re-formatted version of the ico library which isn't so painful to read.

Re-integrated ico library, large scale refactoring, same great taste.

Thanks Otto for cleaning up the ico2 library. It still needs a careful eye later, but I think it's eminently workable here.

My patch re-integrates this library (this time, within favicon-upload.php).

Please use DEBUG mode to leverage the JS and CSS changes (maybe this is obvious and doesn't need mentioning).

In this patch:

• Uploading a PNG, JPG or GIF will function as usual. I have NOT implemented the iframe interface in this patch
• Uploading an image that is <= 32px will result in a PASS THROUGH - the cropping UI will be bypassed and the image will be used as-is at that size, converted to PNG and ICO
• Uploading an ICO file will pass through if it is <= 32px. Not sure what will happen if it's greater than 32px.
• will display only the minimum required HTML (if only a PNG is present, will not generate the conditional IE stuff)

To DO:

• iframe crop UI
• _media_states

We agreed that this wasn't quite ready for 3.4 at a recent dev chat before we released beta 1. Punting to clean up milestone.

Replying to jane:

We agreed that this wasn't quite ready for 3.4 at a recent dev chat before we released beta 1. Punting to clean up milestone.

Could this be planned for inclusion in 3.5?

In the mean time, ​https://github.com/chrisbliss18/php-ico was open-sourced (GPL2 or later).

Coolness. I should probably go back into the code and see whether it's taking advantage of the 3.5 media updates, then see if this ever makes its way back to the right pair of eyes...