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.

[jane]

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

[Denis-de-Bernardy]

Plugin material?

[mikeschinkel]

Replying to Denis-de-Bernardy:

Plugin material?

+1

[westi]

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?

[Denis-de-Bernardy]

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

[markjaquith]

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

[devinreams]

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.

[jane]

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.

[nacin]

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.

[kirasong]

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.

[jane]

@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.

[jane]

@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.

[kencook]

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

[chipbennett]

+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.

[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.

[jane]

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.

[chipbennett]

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.

[RMarks]

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.

[Mamaduka]

Replying to RMarks:

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

[neoxx]

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

[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?

[brandondove]

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.

[Mamaduka]

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

[Otto42]

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/

[TomAuger]

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

[DrewAPicture]

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

[jorbin]

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

[Otto42]

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.

[chipbennett]

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.

[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?

[Otto42]

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.

[TomAuger]

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

[DrewAPicture]

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?

[brandondove]

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.

[TomAuger]

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.

[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.

[brandondove]

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.

[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.

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

[TomAuger]

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.

[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.

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?

[TomAuger]

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.

[TomAuger]

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?

[nacin]

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

[TomAuger]

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

[nacin]

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.

[TomAuger]

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.

[TomAuger]

Early patch with basic add image functionality

[TomAuger]

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.

[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.

[brandondove]

ImageICO PHP library

[nacin]

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.

[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/

[dd32]

"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

[mikeschinkel]

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

[jorbin]

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.

[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.

[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.

[TomAuger]

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

[TomAuger]

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.

[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.

[brandondove]

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.

[jorbin]

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.

[TomAuger]

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.

[TomAuger]

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.

[scribu]

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

[brandondove]

GPLv2 version of ImageICO php lib

[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.

[TomAuger]

Replying to scribu:

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

Yep. Will do.

[TomAuger]

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.

[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.

[chipbennett]

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.

[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)

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?

[chipbennett]

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.

[nacin]

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.)

[nacin]

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.

[nacin]

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.

[TomAuger]

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.

[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.

[nacin]

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. :-)

[TomAuger]

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

[TomAuger]

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.

[TomAuger]

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?

[azaozz]

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.

[TomAuger]

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.

[azaozz]

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).

[jorbin]

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

[jorbin]

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.

[ocean90]

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

[tomAuger]

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.

[tomAuger]

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.

[nacin]

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.

[tomAuger]

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.

[tomAuger]

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.

[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'] ) ) {

What's wrong with the heredoc approach?

What Nacin said.

[tomAuger]

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.

[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?

I asked because of this line

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

[azaozz]

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.

[ocean90]

Note to my comment:105

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

[TomAuger]

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.

[TomAuger]

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?

[TomAuger]

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.

[azaozz]

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 :)

[acsearles]

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.

[toscho]

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.

[TomAuger]

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.

[TomAuger]

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.

[TomAuger]

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?

[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.

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.

[jorbin]

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.

[tomauger]

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.

[Otto42]

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

[tomauger]

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

[tomauger]

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

[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.

[bryceadams123]

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?

[WraithKenny]

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

[tomauger]

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...

[ubernaut]

is this still alive? it's a great idea!

[WraithKenny]

I suggested it as a potential plugin for 3.8, but no one really seemed excited to work on it...

[brandondove]

Replying to WraithKenny:

I suggested it as a potential plugin for 3.8, but no one really seemed excited to work on it...

I'm certainly happy to work on this. It's been on my list of things to do for a while.

[paaljoachim]

Replying to brandondove:

Replying to WraithKenny:

I suggested it as a potential plugin for 3.8, but no one really seemed excited to work on it...

I'm certainly happy to work on this. It's been on my list of things to do for a while.

Awesome!

[celloexpressions]

IE11 supports PNGs for favicons, so we no longer need to worry about creating ICOs. Because IE8 is approaching EOL (techinically WinXP is, but that's the only reason to still run IE8, and that happens a week before when WordPress 3.9 is planned for), and IE9 and 10 auto-update (albeit very slowly and not in all installations), I don't think we need to worry too much about older versions long-term. Even if this feature were launched today, it would work for well over 80% of users with only PNGs, and that number's going to keep going up (probably rapidly).

I don't think this project necessitates a feature plugin; it's pretty straightforward without the ICO part. Maybe an IRC meeting to nail down the implementation. I think we could take the following simplified approach for a long-overdue first-pass:

• Upload button in Settings->general. No need for media library integration, since pictures and content-type images aren't generally appropriate.
• On upload, resize and crop down to 32px square (via functions enhanced in 3.5). I see no reason for a crop UI (32px displayed as 16px is too tiny); if the user wants a different crop that can be done outside of WordPress, or we could add that in a future iteration.
• If the user uploads an ICO file, use it directly.
• Once a favicon has been uploaded, display it on Settings->General with an "upload new favicon" button and a remove link.
• Bonus: duplicate this UI in the customizer, next to the site title & tagline options there.
• Perhaps we should consider NOT saving the image as an attachment, and rather just saving its url in wp_options. Favicons are fundamentally different than everything else the media library is used for, and worst-case scenario the user re-uploads the image if they want to add it to a post, as a header image, etc.
• Plugins and hooks: if we also save the location of the originally uploaded image, plugins can access that image and create additional sizes for proprietory OS-specific icons (for example, iOS icons and Windows 8 tiles). We can also add hooks in the upload process, so that custom sizes can be created and handled at the same time.

I think a big reason this still hasn't happened is that it became too complicated for a first-pass. Let's approach it bit by bit; we could even potentially get multiple iterations into the same release. I don't think this is nearly as complex of a feature as some of the patches on this ticket make it.

We probably need to look at a fresh approach, given the age of the existing discussion & patches.

[brandondove]

Refreshed patch based on 4.3-alpha-32500

[brandondove]

Refreshed the latest patch based on 4.3-alpha-32500 in prep for the feature proposal in tomorrow's devchat.

[chriscct7]

Since the discussion hasn't been had in three years:
Do we really want yet another setting for this? This seems counter to the mission as we're actively working on reducing the number of settings. Also why does this need to go in core versus a plugin?

[GaryJ]

The comment from three years ago regarding apple-touch icons is now well out of date. For covering all of the related features in browsers and operating systems, you'd need something like:

<link rel="apple-touch-icon" sizes="57x57" href="/apple-touch-icon-57x57.png">
<link rel="apple-touch-icon" sizes="114x114" href="/apple-touch-icon-114x114.png">
<link rel="apple-touch-icon" sizes="72x72" href="/apple-touch-icon-72x72.png">
<link rel="apple-touch-icon" sizes="144x144" href="/apple-touch-icon-144x144.png">
<link rel="apple-touch-icon" sizes="60x60" href="/apple-touch-icon-60x60.png">
<link rel="apple-touch-icon" sizes="120x120" href="/apple-touch-icon-120x120.png">
<link rel="apple-touch-icon" sizes="76x76" href="/apple-touch-icon-76x76.png">
<link rel="apple-touch-icon" sizes="152x152" href="/apple-touch-icon-152x152.png">
<link rel="apple-touch-icon" sizes="180x180" href="/apple-touch-icon-180x180.png">
<meta name="apple-mobile-web-app-title" content="My Awesome Website">
<link rel="icon" type="image/png" href="/favicon-192x192.png" sizes="192x192">
<link rel="icon" type="image/png" href="/favicon-160x160.png" sizes="160x160">
<link rel="icon" type="image/png" href="/favicon-96x96.png" sizes="96x96">
<link rel="icon" type="image/png" href="/favicon-16x16.png" sizes="16x16">
<link rel="icon" type="image/png" href="/favicon-32x32.png" sizes="32x32">
<meta name="msapplication-TileColor" content="#2b5797">
<meta name="msapplication-TileImage" content="/mstile-144x144.png">
<meta name="application-name" content="My Awesome Website">
<meta name="theme-color" content="#182148">
<link rel="manifest" href="/manifest.json">

Unless Core is going to support all of these image creations and manifest generation, then I think this feature should be plugin material.

[johnbillion]

We're actually going to be discussing this in tonight's #core meeting.

[emiluzelac]

Replying to GaryJ:

The comment from three years ago regarding apple-touch icons is now well out of date. For covering all of the related features in browsers and operating systems, you'd need something like:

<link rel="apple-touch-icon" sizes="57x57" href="/apple-touch-icon-57x57.png">
<link rel="apple-touch-icon" sizes="114x114" href="/apple-touch-icon-114x114.png">
<link rel="apple-touch-icon" sizes="72x72" href="/apple-touch-icon-72x72.png">
<link rel="apple-touch-icon" sizes="144x144" href="/apple-touch-icon-144x144.png">
<link rel="apple-touch-icon" sizes="60x60" href="/apple-touch-icon-60x60.png">
<link rel="apple-touch-icon" sizes="120x120" href="/apple-touch-icon-120x120.png">
<link rel="apple-touch-icon" sizes="76x76" href="/apple-touch-icon-76x76.png">
<link rel="apple-touch-icon" sizes="152x152" href="/apple-touch-icon-152x152.png">
<link rel="apple-touch-icon" sizes="180x180" href="/apple-touch-icon-180x180.png">
<meta name="apple-mobile-web-app-title" content="My Awesome Website">
<link rel="icon" type="image/png" href="/favicon-192x192.png" sizes="192x192">
<link rel="icon" type="image/png" href="/favicon-160x160.png" sizes="160x160">
<link rel="icon" type="image/png" href="/favicon-96x96.png" sizes="96x96">
<link rel="icon" type="image/png" href="/favicon-16x16.png" sizes="16x16">
<link rel="icon" type="image/png" href="/favicon-32x32.png" sizes="32x32">
<meta name="msapplication-TileColor" content="#2b5797">
<meta name="msapplication-TileImage" content="/mstile-144x144.png">
<meta name="application-name" content="My Awesome Website">
<meta name="theme-color" content="#182148">
<link rel="manifest" href="/manifest.json">

Unless Core is going to support all of these image creations and manifest generation, then I think this feature should be plugin material.

I don't know if we need to add all apple-touch-icon and icon sizes, do we? That seems like an unnecessary bloat to me. We can either load the highest resolution and let it resize, or add low and high if really needed.

See even if you add one, it will take less than all of them combined, unless there is a logic where they're loaded according to the screen size.

<link rel="apple-touch-icon" href="touch-icon.png">

---

References:

​Is there any need for the 6 different apple-touch icons when just one will work?
​Use one apple-touch-icon instead of six
​Configuring Web Applications

[GaryJ]

Replying to emiluzelac:

I don't know if we need to add all apple-touch-icon and icon sizes, do we? That seems like an unnecessary bloat to me. We can either load the highest resolution and let it resize, or add low and high if really needed.

I agree that it's all badly implemented, and I hope that one day all parties just support them all being referenced in a single manifest.json.

The last line of the table on the ​Apple website recommends 4 different sizes alone.

The ​FAQ here outlines most of the files I gave as an example, and links through to further info.

The manifest.json is a ​working draft spec from the W3C, implemented by Chrome and being worked towards by Firefox. The ​browserconfig.xml is from MS for similar features.

Do all sites need support for all of this? No, definitely not, but only considering handling a simple .ico is missing a great deal of the current site identity battle.

[chipbennett]

To me, the obvious solution, then, is for core to provide basic support, and have the output be filterable, so that Plugins can add all the Apple nonsense.

[toscho]

I agree with Chip. Create the favicon, then provide a hook with the attachment ID, and let plugins do the rest. The most important point is that we discourage theme authors from handling favicons.

[RDall]

Replying to chipbennett:

To me, the obvious solution, then, is for core to provide basic support, and have the output be filterable, so that Plugins can add all the Apple nonsense.

While yes Apple doesn't have a bunch of different icons. The problem is not only with them. If you look at a favicon generator like ​http://realfavicongenerator.net/ There is a mess of sizes settings and versions that all support different phones, different computer systems (looking at your Windows 8) etc… etc… etc… 

I saw a ​twitter conversation about this basically calling for a standard and someone replied "SVG for the win" as it can be scaled to what ever the device / browser needs. While browser support for basic SVG is across the board according to ​http://caniuse.com/#feat=svg Getting the user to actually upload a proper SVG is another story. So we would be stuck with uploading PNG's the other issue is if you look at the favicon generator some formats have use of a background and others don't. (Yup looking at you Windows 8 AGAIN)

So my question is if we only going to support the basic Favicon? If we are then are we going to have to direct people to plugins that will support ever size required by every device and product.

Or are we going to support a favicon for every "bloody" format known to mankind.

If we don't I see a lot of:

User would see Favicon support Yeah! and then upload an image and then another user would load the site on their phone and say he/she doesn't see it. And so on…

[grantpalin]

Replying to chipbennett:

To me, the obvious solution, then, is for core to provide basic support, and have the output be filterable, so that Plugins can add all the Apple nonsense.

Agree that the all-round solution looks like basic support in core, allowing for plugins to extend as needed. Just enough to be useful, yet no more than needed (in core).

[ramiy]

Why strict ourselves only to "favicon"?
We can create a new API to add new elements to the HEAD.
To add LINK elements, META elements and other goodies.

[ramiy]

This will be controlled from the code, not from the settings panel.

register_head_element( 'link', array( 'rel'=>'shortcut icon', 'href'=>$favicon_link ) );
register_head_element( 'link', array( 'rel'=>'apple-touch-icon', 'sizes'=>'57x57', 'href'=>'/apple-touch-icon-57x57.png' ) );
register_head_element( 'link', array( 'rel'=>'apple-touch-icon', 'sizes'=>'60x60', 'href'=>'/apple-touch-icon-60x60.png' ) );
register_head_element( 'link', array( 'rel'=>'apple-touch-icon', 'sizes'=>'72x72', 'href'=>'/apple-touch-icon-72x72.png' ) );
register_head_element( 'link', array( 'rel'=>'manifest', 'href'=>'/manifest.json' ) );
register_head_element( 'meta', array( 'name'=>'application-name', 'content'=>'My Awesome Site' ) );
register_head_element( 'meta', array( 'name'=>'apple-mobile-web-app-title', 'content'=>'My Awesome Site' ) );
register_head_element( 'meta', array( 'name'=>'msapplication-TileColor', 'content'=>'#2b5797' ) );
register_head_element( 'meta', array( 'name'=>'msapplication-TileImage', 'content'=>'/mstile-144x144.png' ) );

[Kelderic]

Things to consider:

1. iOS
2. Android Chrome
3. Windows 8 (and 10) tiles

When using Real Favicon Generator, I end up with a zip file containing 27 png image files, one xml file, one json file, and one ico. This covers everything.

The process for Real Favicon Generator is:

• Upload Image
• Customize iOS:
  • Use Dedicated Picture, or Add Background Color, and Margins
• Customize Android Chrome
  • Use Dedicated Picture, or add margins, background, drop shadow, etc.
• Customize Win8 Tile
  • Use Dedicated Picture, or add background color or not.
• Add cache buster, decide compression, decide scaling algorithm, and add App Name.

This is a lengthy and in-depth process, but it does allow for all possibilities.

Others have suggested having a simple setting and allowing it to be added to by a plugin. Wouldn't it be better in the long run though to use the media manager, with some special views, to handle it all?

[chriscct7]

There was a discussion yesterday about putting this in the customizer. I wanted to copy over from Slack that there's a tiny js library called favicon.js that can be used to do live favicon changes. It's so small we could just roll it ourselves to avoid a dependency.

[emiluzelac]

Replying to chriscct7:

There was a discussion yesterday about putting this in the customizer. I wanted to copy over from Slack that there's a tiny js library called favicon.js that can be used to do live favicon changes. It's so small we could just roll it ourselves to avoid a dependency.

Would that be this? ​https://github.com/Dlom/favicon.js and take a look at ​http://lab.ejci.net/favico.js/ which can also change dynamically :)

[chriscct7]

Replying to emiluzelac:

Would that be this? ​https://github.com/Dlom/favicon.js and take a look at ​http://lab.ejci.net/favico.js/ which can also change dynamically :)

Yep, that's what I was specifically looking at (the first one). There's also a couple more as well on GitHub (for the most part they are all under MIT license).

[emiluzelac]

Replying to chriscct7:

Replying to emiluzelac:

Would that be this? ​https://github.com/Dlom/favicon.js and take a look at ​http://lab.ejci.net/favico.js/ which can also change dynamically :)

Yep, that's what I was specifically looking at (the first one). There's also a couple more as well on GitHub (for the most part they are all under MIT license).

Awesome!

[obenland]

I set up a proof-of-concept plugin to add site icons to general settings:
​https://wordpress.org/plugins/wporg-site-icon/

Please feel free to test it and leave your feedback here!

[ryan]

​https://make.wordpress.org/flow/2015/06/20/set-and-remove-site-icon-iphone-6/
​https://make.wordpress.org/flow/2015/06/20/set-and-remove-site-icon-mac-and-chrome/

Tag: ​https://make.wordpress.org/flow/tag/site-icon/

[jb510]

@obenland I installed the plugin on 4.3-32900.

First attempt to upload a favicon.ico succeeded but when setting I got a notice that:
The selected image is smaller than 512px in width.

It's the standard 16x16, 32x32, 48x48 made by IcoMaker (OS X app I've used for years)
:(
I tried another one created with realfavicongenerator that I think is 16x16, 24x24, 32x32, 64x64 and it didn't work either.
:(
I finally created and uploaded a 520px PNG which worked, yay!
:)

I haven't actually tested visibility across devices/browsers, but FWIW realfavicongenerator is the only test suite I've found to be reliable in the past.

Three other bits of feedback:

1. This is a bit odd "Site Icon creates a favicon for your site and more." -- What more?
2. Discoverability is really poor at the bottom of general settings. I'd have assumed I'd find it under general next to the site/home URLs.
3. I know we still can't upload SVGs through media uploader, but SVG favicon support _is_ finally starting to come to browsers (FireFox betas),

[obenland]

Replying to jb510:

Thanks so much for testing it, Jon!

1. This is a bit odd "Site Icon creates a favicon for your site and more." -- What more?

App icons. Yes, that certainly can be more descriptive.

2. Discoverability is really poor at the bottom of general settings. I'd have assumed I'd find it under general next to the site/home URLs.

I think that's where I'll place it in the core patch. The only hook on that page is at the very bottom, which is why it ended up there.

3. I know we still can't upload SVGs through media uploader, but SVG favicon support _is_ finally starting to come to browsers (FireFox betas),

For v1 that feels like plugin territory (just like ico conversion), which doesn't mean it can't be added later.

[obenland]

First pass at site icon for settings.

[jb510]

Welcome. FWIW my first thought was that it should support the complete favicon spec across devices (browserconfig.xml, apple icons, Ms icons, etc...). That's a mess though, totally support the simple solution with the ability for plugins to over ride it with support for more later.

I'm still not clear if uploading .ico files is supposed to work or not? Regardless, better instruction is needed to clarify or warn against uploading .ico files since that what a lot of people will assume to upload.

[dd32]

I'm still not clear if uploading .ico files is supposed to work or not? Regardless, better instruction is needed to clarify or warn against uploading .ico files since that what a lot of people will assume to upload.

It's not designed to accept .ico, nor is it designed to create them.

Realistically few people will have .ico files or attempt to upload them IMHO..

[obenland]

Browser image, to be added in src/wp-admin/images/.

[tyxla]

Updating the patch from @obenland 16434.4.diff to fix the single equals operator and use relative height when scaling the preview in the admin.

[scruffian]

@tyxla - I think there's something wrong with the patch you created - I can't apply it.

[tyxla]

The latest patch (16434.5.diff) addresses 2 issues:

1. Fixing the preview image crop & resize functionality when using the GD image manipulation library - it was not accepting a zero height when cropping, so we're now calculating the height relatively to the width.
2. Fixing the single equals operator with a double equals operator in order for it to work as expected.

[jancbeck]

This is a bug report for the 16434.5.diff patch:

• the whole cropping step is not working well on devices with smaller screens because the crop-preview is not responsive. In theory the cropping should work though because the jCrop script supports that.
• In No-JS the edit/remove link buttons are merged into one ugly button.
• The media select modal can not be closed.
• In No-JS the final submit buttons label "Crop and Publish" is misleading since No-JS does not actually crop the image. It just downscales it.
• In No-JS, the browser favicon and app icon preview image is missing a "max-width: 100%" css styling.

These issues apply to 16434.5.diff as well.

[tyxla]

Updatign last patch to include minor no-js improvements

[tyxla]

The last patch I have submitted addresses some of the no-JS issues as reported by @jancbeck:

1. The edit/remove link buttons will now appear as regular separate buttons
2. The Crop and Publish button will now appear as just Publish when JS is disabled. This will avoid misleading the user, having the crop step appearing as a preview step before the user actually saves the icon.

Concerning the favicon previews missing max-width:100%, I think that it would not achieve the expected result. If the user uploads a rectangle images, they will receive a non-square preview there, which would mislead them about the end result.

I confirm the rest of the issues, except the one with the media modal - it is perfectly closeable for me (including the X button and the closing if you click outside of the popup).

[markjaquith]

Not sure what's going on here:

[obenland]

Replying to markjaquith:

Not sure what's going on here:
{Image}

It's a media modal issue, caused by floating the dropdown left to accommodate the search bar, unrelated to Site Icon.

[markjaquith]

browser.png is low-resolution, and is the old "aqua" OS X style.

Also can we lose the drop shadow on the window?

[flixos90]

fixed image cropping bug when Javascript was disabled

[flixos90]

Also, this last patch adjusts the message presented to the user about the required image size with Javascript disabled.

The image will be automatically resized and cropped to 512x512 pixels, but since it is an icon, the image is obviously recommended to be a square image.

[Daniel Koskinen]

I'm at WordCamp Europe and am about to do some accessibility testing on this right now.

[Daniel Koskinen]

OK here are some initial findings from keyboard usage:

• tabbing to and opening modal via keyboard works as expected
• tabbing to the media library and selecting an existing file from the library work mostly as expected
• I’m a bit concerned about the tab order: if all you’re intending to do is select an image and set is as the site icon, the tab order works as expected. However if you intend to edit the attachment details (which doesn’t actually make sense in this context), after tabbing through the details I would expect the focus to move to the submit button. It could be that a better option would be to hide the details completely in this view, since a site icon is not going to have any alt text or description.
• Uploading a file via the keyboard works fine, but the modal loses focus after this (confirmed in Chrome and Safari), and you have to tab through the whole page in the background to get back.
• In Firefox the Upload files/Media library tabs do not get focus at all via keyboard
• Confirming that the small sized dropdown is an issue

[markjaquith]

Updated browser chrome. Patch will have to be updated for size reasons

[celloexpressions]

In-progress Customizer patch is now on #29211. We're still waiting for the initial commit for the settings version so that the base API can be reused with only the administration layer changing. The experience will be smoother there since cropping is handled directly in the media modal so you never leave the page/break the flow. If this is planning on being saved up until right before beta, I would appreciate knowing that sooner rather than later so that I can just copy the API to build into the Customizer version so that it can also be ready in time, but that seems like a silly way to go about this.

Sidenote: we should use a more generic browser chrome image if we really want to have that there. And we might as well put the site title there too if it isn't already in the patch. We also know the site url, so the start of that would be nicer than "www.". In the Customizer, I'm planning on just live-updating the icon in the browser window via JS, which most modern browsers support (Slack does this, for example). Seems like that should be adequate? Would appreciate more screenshots of what the patch does posted on this ticket though.

[obenland]

Screenshots and flow: ​http://make.wordpress.org/flow/2015/06/29/site-icons-settings-screenshots/

[obenland]

In 32994:

Introducing Site Icon, favicon management for WordPress.

This v1 marries Jetpack's Site Icon module with the Media Modal, reusing code
from the Custom Header admin. For now, the core-provided icons will be limited
to a favicon, an iOS app icon, and a Windows tile icon, leaving .ico support
and additional icons to plugins to add.

Props obenland, tyxla, flixos90, jancbeck, markjaquith, scruffian.
See #16434.

[markoheijnen]

Unsure why leaving out .ico support. The proof of concept I wrote made this really easy. See ​https://gist.github.com/markoheijnen/abe4f4ed0f52178218a3.

I know people didn't like storing it somewhere else but the code could be a good start to not use wp_crop_image but something custom and maybe better fitting.

[boonebgorges]

[32994] broke a unit test: Tests_Post_GetPostClass::test_taxonomy_classes_hit_cache(). The test references $wpdb->num_queries, and [32994] adds a query to the get_post_class() chain. Specifically, the WP_Site_Icon::get_post_metadata() callback is fired, which results in a call to get_option( 'site_icon' ). Both of these facts are unique to the automated tests: normally, the get_post_metadata callback will only be fired in the admin, and normally it will be cached, and the test suite is an outlier in these ways.

These query-counting tests are pretty fragile, so it's no big deal to increment the count in the test.

[boonebgorges]

In 32997:

Fix Tests_Post_GetPostClass::test_taxonomy_classes_hit_cache() after [32994].

[32994] results in an additional database query during automated testing, so
the query count being compared in this specific test must be incremented
manually.

See #16434.

[kraftbj]

Moving the admin_head action to admin-filters.php

[jipmoors]

when the current attachment has been re-selected just refresh metadata

[jipmoors]

Problem before patch.11: when re-selecting the current site-icon it would get deleted before re-cropping itself so it would be deleted.

[jipmoors]

When an image with specified dimensions has been selected (512x512 on default settings) do we want to generate a new file with the same dimensions or just use that file? When removing the site-icon you probably don't want to remove that file so a new file seems logical but still looks strange in the media library.

Also, when selecting this file we want to skip the crop page, but since that is being loaded after the media-selector the headers have already been sent at the crop_page function. This only leaves an option to javascript-submit the form to go to the next step, or am I missing something?

[obenland]

Replying to jipmoors:

When an image with specified dimensions has been selected (512x512 on default settings) do we want to generate a new file with the same dimensions or just use that file? When removing the site-icon you probably don't want to remove that file so a new file seems logical but still looks strange in the media library.

Yes, new file. In the media library it get's a Site Icon label (in the list view) to give it context. I believe that's missing in grid view, but that shouldn't be too bad.

Also, when selecting this file we want to skip the crop page, but since that is being loaded after the media-selector the headers have already been sent at the crop_page function. This only leaves an option to javascript-submit the form to go to the next step, or am I missing something?

We can use the admin_action_crop_site_icon action to check the image and redirect.

[obenland]

In 33011:

Site Icon: Move admin icon filter to its rightful place.

The action is unrelated to WP_Site_Icon itself.

Props kraftbj.
See #16434.

[jipmoors]

selecting an image with minimal accepted dimensions skips crop dialog and sets image instantly

[jipmoors]

admin_action_crop_site_icon is not being called, figured out load-settings_page_site-icon was the place to hook into and made it work.

[jipmoors]

There seem to be some problems with image cropping on the latest patch. Working on fix.

[jipmoors]

saving settings page doesn't delete site_icon anymore

[Shaped Pixels]

I am just now playing with the beta 4.3, but regarding the site icon…perhaps it would be better to label it as "Favicon" and not "Site Icon" as many end-users may get confused with the current name as most are more familiar with the actual term of favicon. As for the image size, you may want to also consider making a notation to the user before they click to upload an image of the minimum size requirement of 512px (unless there are plans to add this later?)

[kraftbj]

#32861 was marked as a duplicate.

[akibjorklund]

If you have a transparent png as an icon (a lot of logos are transparent), you cannot see the rounded corners of the mobile preview icon without some color on the background. #000 is what iOS uses.

[RDall]

I would say this a blocker as many companies / people / personal branding use a transparency on the graphics for the background. When see a new color added to the mix it throws people off and they think they are doing something wrong. Most just leave the feature as unused if they can't get the image to look like the one uploaded.

Replying to akibjorklund:

If you have a transparent png as an icon (a lot of logos are transparent), you cannot see the rounded corners of the mobile preview icon without some color on the background. #000 is what iOS uses.

[RDall]

Screenshots:

Media Uploaded

Cropping

Finished upload

[obenland]

In 33049:

Site Icon: Show correct background preview for transparent images.

iOS will show transparent parts as black, where as the icon itself stays
transparent when used as a favicon.

See #16434.

[tyxla]

As @jancbeck mentioned on the WordCamp Europe 2015 contributor day, the Site Icon feature crop step is not responsive, which makes it unfriendly for mobile devices with smaller screens.

Patch for that coming in a minute.

[tyxla]

Responsive improvements for the Site Icon, making the feature more friendly for mobile devices with smaller screen size.

[obenland]

In 33051:

Site Icon: Skip cropping if image has the correct size.

Props jipmoors for initial patch.
See #16434.

[obenland]

In 33053:

Site Icon: Improve responsiveness for small screen devices.

Using Jcrop's trueSize argument also allows us to get rid of all that behind
the scenes temp image creating and back and forth calculating of image sizes.

Props tyxla for initial patch.
See #16434.

[ryan]

With the 4.3-beta1-33054 nightly, cropping is skipped entirely. Click/tap Set as Site Icon in the modal and you are immediately directed back to Settings > General. Tested with Macnchrome and iPhone 6+.

[ryan]

Replying to ryan:

With the 4.3-beta1-33054 nightly, cropping is skipped entirely. Click/tap Set as Site Icon in the modal and you are immediately directed back to Settings > General. Tested with Macnchrome and iPhone 6+.

Site Icon: Skip cropping if image has the correct size.

Ah. That's why? Expected with an image straight from a phone's camera roll?

[ryan]

Replying to ryan:

Replying to ryan:

With the 4.3-beta1-33054 nightly, cropping is skipped entirely. Click/tap Set as Site Icon in the modal and you are immediately directed back to Settings > General. Tested with Macnchrome and iPhone 6+.

Site Icon: Skip cropping if image has the correct size.

Ah. That's why? Expected with an image straight from a phone's camera roll?

I was choosing the first image in the gallery view of the media lib to test this. The first image happened to be a cropped site icon from a previous test. I couldn't tell I was picking the already cropped image on iPhone 6+ or Macnchrome.

[ryan]

Cropping is completely broken for me in the nightly.

​https://make.wordpress.org/flow/2015/07/02/changing-site-icon-4-3-beta1-33054-iphone-6/

I'll check against src.

[ryan]

Replying to ryan:

Cropping is completely broken for me in the nightly.

​https://make.wordpress.org/flow/2015/07/02/changing-site-icon-4-3-beta1-33054-iphone-6/

I'll check against src.

Works with src. Build problem?

[ryan]

Shift + reload fixed the site running the nightly for Macnchrome.

Even after clearing all cookies and data in iOS Safari, the crop screen is still busted on my iPhone 6+.

[Clorith]

Might do with a (Minimum 512x512px) helper text behind the Choose Image button to avoid accidental use of older and smaller images like favicons used to be? The current approach is to show an error and a screen with just the upload image form element, and it moves the user out of the Settings > General area they were originally in, which may appear confusing to some.

From ​https://wordpress.org/support/topic/site-icon-size-recommendation

[celloexpressions]

First-pass at site icons and a cropped-image control in the Customizer.

[celloexpressions]

16434.customize.diff​ adds site icons with cropping to the Customizer (and also fixes #29211).

I've posted the flow that this patch introduces with screenshots on make/flow: ​https://make.wordpress.org/flow/2015/07/03/proposed-customizer-site-icon-flow/.

Looking for dev and UI/UX feedback on this, although I won't personally be able to circle back here until next week, so feel free to iterate on this in the meantime.

[lucaspiller]

Make the favicon filters more flexible

[lucaspiller]

I'm trying to update an existing plugin to use the new site_icon_meta_tags filter, but it isn't flexible enough for my needs. As is, unless an attachment has already been uploaded as the site icon it won't be ran.

https://core.trac.wordpress.org/attachment/ticket/16434/16434.flexibility.diff changes that, so it is always ran even if no site icon has been set (with an empty array as the parameters).

[obenland]

Replying to lucaspiller:

I'm trying to update an existing plugin to use the new site_icon_meta_tags filter, but it isn't flexible enough for my needs. As is, unless an attachment has already been uploaded as the site icon it won't be ran.

If there is no site icon set, what would you need to filter the output for?

Tp provide compatibility with existing solutions you could filter the site icon option and provide a custom attachment id as a default if the option is empty.

[lucaspiller]

Replying to obenland:

If there is no site icon set, what would you need to filter the output for?

Tp provide compatibility with existing solutions you could filter the site icon option and provide a custom attachment id as a default if the option is empty.

The plugin lets you specify different favicon attachments for each type (e.g. ico, png, apple touch, metro, etc). In this case I'm not really filtering, but just using the filter to hook into the code which renders the favicons at the right time. In the method called by the filter I replace $meta_tags with my own tags, which refer to each attachment the user specifies.

Hooking into the site icon option would work, but it's still a bit messy as I'm just adding something to work around the flexibility issue - it's not actually used. I think a better solution in my case is to completely remove the wp_site_icon action and role my own. Thanks for the rubber duck debugging!

[dd32]

This introduced two strings using %u rather than the more common %d:

The selected image is smaller than %upx in width.

The selected image is smaller than %upx in height.

I'm thinking we should switch those over to %d for ease of translation.

[ocean90]

16434.15.diff​:

• $( 'link[rel="icon"]' ).attr( 'href', attachment.sizes.thumbnail.url ); won't work if you haven't set a site icon before because wp_site_icon() doesn't print anything if ! has_site_icon().
• The media frame gets a sidebar after an image has been set and you open frame again, see ​https://cloudup.com/c3lzgA0TRNn. The button label is missing too.

[obenland]

Thanks for your feedback @ocean90, I updated the patch.

I wonder if we could also skip cropping automatically on the Customizer side, if the image is the right size.

[celloexpressions]

Thanks for .16.diff. It looks pretty good to me too, a few additional notes:

• apply_filters( 'wp_ajax_cropped_attachment_id', 0, $context ); seems like it should be an action, not a filter, based on the core usage for the site icon case. Additionally, I think this is a case where it may make more sense to have a dynamic hook instead of passing the context as a param. Are there situations where you'd want to filter multiple contexts together? I think it would be most common to filter/add an action here to add custom handling for your custom context, in which case a dynamic hook would probably read better (thinking along the lines of some of the ones for types in WP_Customize_Setting.
• It also looks like anyone wanting to use the cropped image control would have to implement their own handling for saving the attachment when the image is cropped. In the Customizer at least, this should "just work". Default behavior should probably be to save a copy of the attachment and return the new attachment. In most cases it won't be necessary to change that behavior when setting the
• The customizer media control is designed to use the full image url, not the medium size, when rendering the image preview, so we should probably pass the full (cropped, likely 512px) image as the setting's default value. Note that the image can be displayed as wide as ~ 560px on certain screen sizes.
• I like the idea of using the control type as the context, but I'm not sure that it's going to be better for developers in practice. I have a feeling there will be scenarios where you may need to create a custom control just to change the context, where no other changes are necessary. That has major implications in terms of effort required (registering the new type, defining it in PHP and JS, duplicating all of that nasty core CSS just to use the core UI, etc.), whereas being able to set it as a parameter would be fairly straightforward. This is also designed to be used as the context of the attachment once it's created by default, which could potentially allow this control to offer an option to show a library of uploaded/cropped images in the future (like header images).
• "Site Identity" has been my thought for this for some time and I think it works quite well. Would like to hear any other opinions on that/discuss it further but that can potentially happen after a first pass. The concept for this section is/becomes things that identify the site outside of the actual site (ex. in the browser icon/title), and can also show up on the site depending on the theme.

Should be pretty easy to add a check for whether the image is exactly the requested size in wp.customize.CroppedImageControl.onSelect(), calling onSkippedCrop() instead of switching to the cropper state if it's the right size. I'd only do that if flex-height and flew-width are disabled.

[celloexpressions]

Customizer: skip cropping if image is the right size; use full image size as default setting value.

[celloexpressions]

16434.17.diff​ skips cropping automatically in the Customizer if the image is the right size and flex-crop is disabled (meaning the size is mandatory, not recommended, so the image can't be cropped anyway). Also uses the full image size as the default value, keeping with the way WP_Customize_Media_Control works, per Slack discussion with @obenland.

Remaining questions/changes for the customizer side are more related to #29211, and whether there should be default server-side handling for cropped-image attachments so that devs can add cropped-image Customizer controls that "just work" rather than having to roll their own version of attachment duplication and saving. I'd like the developer experience to be as seamless as possible, and also think about potential future enhancements - core or otherwise - that will be enabled by setting the attachment context to the customizer control id by default.

Thinking about it some more, I'm also okay with making the ajax filter based on the control id rather than its type, as long as we have default support for theme and plugin-added controls so that they don't need to use the filter unless they're doing custom stuff.

[ericlewis]

I applaud the work being performed here but I literally cannot unsubscribe from email notifications for this ticket. Should I open a ticket on Meta?

[WraithKenny]

@ericlewis You can unstar (unwatch) the ticket, and then Block notifications should appear as a button.

[ericlewis]

Replying to WraithKenny:

@ericlewis You can unstar (unwatch) the ticket, and then Block notifications should appear as a button.

Perfect. Block didn't show up but I watched and unwatched and then it showed :D

[obenland]

In 33154:

Site Icon: Add Customizer UI.

Second part of the Site Icon feature after [32994] introduced it for Settings.

Props celloexpressions.
See #16434.

[obenland]

Integration tests - First pass.

[obenland]

Second pass at integration tests.

[obenland]

Final integration tests.

[kraftbj]

Noting that we're not currently making and outputting 192x192 sizes used by Android. Currently, it falls back to Apple's icon, but marked as deprecated by Google. See #32964

[jorbin]

The above patch has a few relatively minor changes from @obenland's unit tests. Namely:

• Added @group for all the template tests as well. Went with individual labels since it's reasonable that this test class could contain non site_icon tests.
• Removed the fitler at the end of test_intermediate_image_sizes_with_filter to help with cleanup

[obenland]

In 33181:

Site Icon: Unit tests for WP_Site_Icon and its API.

Props obenland, jorbin.
See #16434.

[obenland]

I'm getting ready to close this as fixed and to trac new bugs in separate tickets.

[ryan]

Vizrec of changing the site icon via customize on an iPhone 6+. Cropping is broken.

​https://make.wordpress.org/flow/2015/07/14/change-site-icon-via-customize-iphone-6/

[ryan]

Vizrec on desktop.

​https://make.wordpress.org/flow/2015/07/14/change-site-icon-via-customize-macnchrome/

[obenland]

I've been working on moving cropping and the site icon preview into the media modal to attain feature parity between the Customizer and Settings versions. 16434.23.diff​ is still a little buggy, but it accomplishes that. I was also able to monkey-patch imgAreaSelect to work on touch devices, I'm waiting to get the bugs from .23.diff fixed before submitting a new patch.

With having both versions look the same and do the same, I'm now wondering if it still makes sense to keep both around or just keep the Customizer one. Opinions?

[jorbin]

Replying to obenland:

With having both versions look the same and do the same, I'm now wondering if it still makes sense to keep both around or just keep the Customizer one. Opinions?

Decisions, not options. I think this is absolutely a site customization and thus belongs in the Customizer and only the Customizer.

[jancbeck]

If you ask me, as someone who joined the development a little later at #wceu, I found it always confusing why both versions exist. The customizer is the future, right?

[toscho]

Then please make it keyboard accessible. The customizer is already hard to use per keyboard.

[obenland]

Replying to toscho:

Then please make it keyboard accessible. The customizer is already hard to use per keyboard.

Besides changing the crop selector everything is keyboard accessible. If you know of a good way to make the crop selector keyboard accessible, please let me know.

[toscho]

Replying to obenland:

If you know of a good way to make the crop selector keyboard accessible, please let me know.

It is almost there. :) You can focus move the selection and move it with the arrow keys. It should shrink and expand with Shift + Arrow.

[obenland]

In 16434.24.diff​:

• Removes Settings version of Site Icon
• Adds preview to the media modal for Customizer version.
• Monkey-patches imgAreaSelect to work on mobile.
• Fixes some mobile styles for the media modal.

[DrewAPicture]

Just tested 16434.24.diff​ at @obenland's request. I'm coming into this blind as I had never used this UI before, which I suppose might be to our benefit.

Overall, the cropping UI looks good on mobile and desktop and seems intuitive and easy to figure out. I came across a couple of things in the Customizer control UI that seem like they could still be fine-tuned:

1. If I crop an image, set it as the icon, and Save & Publish, then I remove the image and try to set it again, it's not listed in the media modal. I'm guessing this is because we didn't refresh the media list. I would've expected the newly-cropped image to be available right away, I just cropped it a minute ago ... :)

2. I wonder if Chrome on Mac is the best choice for a browser icon previewer for 80 percent of our users.

3. After refreshing the page, the newly-cropped image(s) now show up in the media modal as expected. However, I also now have a new button, "Default", that apparently restores the default that I don't remember setting (because I didn't). I don't think it's really clear how the "default" icon is determined, this is probably because it seems like that should be a deliberate action made by the user or the theme author – in which case, I'd expect there to be some kind of visual indication of the default choice.

[celloexpressions]

I'm also +1 for Customizer-only, because we've been spending a lot of effort trying to remove options from (and reduce the number of) settings screens, and encouraging users to do this as part of the Customizer flow is a good step towards the future.

Re: Drew's comments, I'd like to see the browser chrome image at least be more generic so that it can convey the meaning without being a specific identifiable browser and device. The image should be grayscale at the least, as the colored buttons are distracting, and not something you find on Windows, for example. We should remove the default button - I kind of forgot that was there. I don't think we're setting a default value, so that should just be "remove". Should be able to do that in the Site Icon control or with CSS without overriding the entire content_template(), hopefully. Or even better, in WP_Customize_Media_Control, if the default is empty, show the remove button instead of a default button.

Great work bringing this together in the media modal @obenland!

[RDall]

I am against having the site icon in just customizer.

Also when you hit Settings >> General it seem to me the prefect place for the site icon setting.

Also if we go back to the chat from May 13th on the subject:

think it can also be in the customizer, but why wouldn’t it also be in the settings like the site title/description? Sam Slider ​https://wordpress.slack.com/archives/core/p1431551208001000

Also it was discussed in the June 11th meeting chat with more debate:

I am also a fan of making it a setting not in customizer mikehansenme ​https://wordpress.slack.com/archives/core/p1434053413000088

We also never really agreed on whether to put it in the customizer or the General Settings so we decided on both.

We disagreed, so we’re doing both customizer and setting Obenland ​https://wordpress.slack.com/archives/core/p1434055361000202

I personally wanted both only because of the reaction of so many people to the customizer. But I was fraught with peril as this was something I was lobbying for yet didn't have the skill to pull off myself.

My apologies I haven't been able to attend a lot of meeting on the subject or contribute but I hope you consider this feedback and understand that months before we got to this point the decision was no decision at all and to put it in both.

[obenland]

Includes wonderboymusic's feedback

[obenland]

Latest attachment includes feedback from wonderboymusic and changes the "Default" button to "Remove" by removing the default value for the setting.

[obenland]

Replying to celloexpressions:

Re: Drew's comments, I'd like to see the browser chrome image at least be more generic so that it can convey the meaning without being a specific identifiable browser and device. The image should be grayscale at the least, as the colored buttons are distracting, and not something you find on Windows, for example.

I think we have currently more pressing things to focus on in 4.3 than that browser image.

[obenland]

Replying to RDall:

I am against having the site icon in just customizer. [Chat examples]

I was originally against adding it to the Customizer because we already had a working settings version and there was no real preview in the Customizer. That is no longer the case. Bringing cropping and the icon preview into the media modal for the settings version made both versions identical, so having both just feels redundant. I'd encourage you to test the latest patch and see how it feels in the customizer.

I personally wanted both only because of the reaction of so many people to the customizer.

"Many end users of WordPress are non-technically minded. They don’t know what AJAX is, nor do they care about which version of PHP they are using. The average WordPress user simply wants to be able to write without problems or interruption. These are the users that we design the software for as they are ultimately the ones who are going to spend the most time using it for what it was built for."

"So while we consider it really important to listen and respond to those who post feedback and voice their opinions on forums, they only represent a tiny fraction of our end users."
—​WordPress Philosophy

[dd32]

Replying to obenland:

Replying to celloexpressions:

Re: Drew's comments, I'd like to see the browser chrome image at least be more generic so that it can convey the meaning without being a specific identifiable browser and device. The image should be grayscale at the least, as the colored buttons are distracting, and not something you find on Windows, for example.

I think we have currently more pressing things to focus on in 4.3 than that browser image.

I agree that there's more important things, but we should also aim to have the images as clear as possible for the majority of people.
Chrome on mac is not a generic example of a browser, and is unrecognisable by (I would guess) 90%+ of users.

Are there any designers in the house who have something they can suggest here?

[obenland]

In 33329:

Site Icon: Add crop preview to the media modal.

• Monkey patches imgAreaSelect library to support touch events.
• Removes Settings version of Site Icon since it would have been the same flow.
• Removes default value for Customizer setting - there is no default favicon.

Fixes #16434.

[obenland]

In 33335:

Tests: Remove test for delete_site_icon().

Removed in [33329].
H/t wonderboymusic.

See #16434.

[jorbin]

In 57602:

General: Add an option to configure the site icon in general settings.

This restores the site icon setting to its original home on the settings page where it lives with the title and tagline.

The base for this code was originally added in [32994] and then removed in [33329]. The majority of the modification to that version are to remove the no-js pieces and make the workflow completely inline rather than putting the cropping on a separate page.

Additionally, since image crops rely on the ability to upload an image, this setting is gated by the upload_files capability.

Follow-up to: [32994], [33329].

Props jorbin, audrasjb, mukesh27, joedolson, afercia, kebbet, swissspidy, obenland, jameskoster, kjellr, andraganescu, stacimc, mikeschroder, h71, krupajnanda, huzaifaalmesbah.
Fixes #54370.
See #16434.