Twenty Fourteen: Fix Genericons Loading

[celloexpressions]

Regardless of whether or not Genericons are packaged in core (see #24864), we should load them with wp_enqueue_style('generions', ... ) like we did with Twenty Thirteen (#24595) so that they play more nicely with plugins.

Since the genericons.css file that is packaged with Genericons isn't currently loaded or accounted for, things break pretty severely when a plugin loads Genericons the "right way."

This is what happens to the front page right sidebar when a plugin loads version 2.09 of genericons.css:

The problems come from the following css:

.genericon {
	display: inline-block;
	width: 16px;
	height: 16px;
	-webkit-font-smoothing: antialiased;
	font-size: 16px;
	line-height: 1;
	font-family: 'Genericons';
	text-decoration: inherit;
	font-weight: normal;
	font-style: normal;
	vertical-align: top;
}

We'll need to make some adjustments in Twenty Fourteen's use of the genericon class to fix this. So, in summary, we need to wp_enqueue_script() and make Twenty Fourteen compatible with the default Genericons css.

[celloexpressions]

Twenty Fourteen with a plugin loading Genericons 2.09

[obenland]

Adding the :before pseudo selector in fonts/genericons.css fixed that issue. I'll loop back with Joen if he left that out intentionally or if that should be added to Genericons.

[celloexpressions]

Yes, that looked like the issue. Might also want the :after selector, but I can't think of any reason to apply those styles to the element itself.

[lancewillett]

In 25068:

Twenty Fourteen: load Genericons correctly, and add comments to other CSS enqueue calls. Props obenland, fixes #25085.

[georgestephanis]

Actually we'll need to update Genericons to 3.0, the current release.

The previous iteration was significantly outdated. Patch about to be uploaded.

[georgestephanis]

You will need to purge some transients for the sidebar items -- twentyfourteen caches them there, and it'll not remove the class if you don't purge them.

[lancewillett]

In 25417:

Twenty Fourteen: prefix Genericons enqueue handle so that the theme uses the its own version of the font CSS. Fixes a few display issues when plugins like Jetpack enqueue a different version of Genericons. See #25085.

[lancewillett]

@georgestephanis Thanks for the patch and notes. I think we should use 3.0 if we can, but it'll need a bit more work. Your patch is a great start, but:

• I don't think we need to add the extra div just for the search icon
• The widget title styling is still a bit off

For now with r25417 we'll just use the 2.09 version (even with plugins like Jetpack active that have a newer version of Genericons).

[celloexpressions]

Replying to lancewillett:

In 25417:

Twenty Fourteen: prefix Genericons enqueue handle so that the theme uses the its own version of the font CSS. Fixes a few display issues when plugins like Jetpack enqueue a different version of Genericons. See #25085.

The purpose of this ticket is specifically NOT to do that. If we use a different handle, then we're only trying to step around the problem instead of actually fixing it. Using different handles, multiple versions of Genericons will load (bad for performance) and they will likely conflict with each other. Keep in mind that it's very possible that multiple plugins will load Genericons; sites could quite easily get to the point of having three or more versions of Genericons loading, which is just bad all around.

If we absolutely must load our own specific version of Genericons, the css and font-family name should ALL be prefixed to avoid conflicts. That's also a bad idea, of course, as it makes updates unwieldly.

The only big problem with the raw Genericons css is that it adds styles to .genericon instead of .genericon:before, which is why Twenty Fourteen can get pretty nasty with that (see twenty-fourteen-genericons-2.09-from-plugin.png​). @obenland, did Joen have a reason for doing that, or was it accidental and forgotten to be fixed (see comment 1)? If that's staying, we should adjust our uses of those classes in Twenty Fourteen so that we can use the bundled css.

More broadly, this is really demonstrating that we need Genericons packaged in core (#24864). Having a canonical version would solve all of the problems we're having and make it significantly easier to use them in plugins. There are a lot of arguments that have been made both ways (I summarized them in the most recent comment), but I think that needs serious consideration. We would look at it from the perspective that we're using Dashicons for wp-admin, but those are admin-specific; Genericons are available for front-end uses by both themes and plugins because they're generic and there's interest in having an easily-accessible icon font for WordPress.

Otherwise, we need to figure out a better way to make Genericons tenable for use in both themes and plugins. We should really never load them more than once, but perhaps the only commonly loaded css should define the fonts themselves and everything else should be left to the specific implementation. There also needs to be some sort of a better mechanism for updates; the only big problem with different versions is the addition of new icons in new versions, but I don't think we need to worry about updates breaking things unless the bundled css is significantly changed. With that, all we need is to make wp_enqueue_style do a version-compare if multiple copies of the same item are enqueued.

We should probably update Twenty Thirteen to Genericons 3.0 also. And note that it uses wp_enqueue_style('genericons' to play nicely.

I'll also point out that Jetpack caused all of the issues by loading a different piece of css from everyone else, but since we don't have a standard (which could be documented on Genericons.com), that's excusable. But ​that caused the Genericon'd plugin to switch from doing-it-right as well.

At the moment, if a site were to run Twenty Fourteen, Jetpack, my QuickShare plugin, and Genericon'd (if they wanted the shortcodes, for example), four different versions of Genericons would be loaded. That's not an unlikely set of plugins. And for the record, things still break in Twenty Fourteen in that scenario, even after [25417]. The only thing that that fixed was the fact that Jetpack didn't load the bundled genericons.css file with its wp_enqueue_style call.

[georgestephanis]

I've just gone through all the individual zip releases of Genericons, and saved each as a commit and pushed them up to

​https://github.com/Automattic/Genericons

And it looks like the syntax that Twenty Fourteen is using -- ​http://core.trac.wordpress.org/browser/trunk/src/wp-content/themes/twentyfourteen/fonts/genericons.css -- that of

[class*="genericon"]:before {

doesn't seem to have been in any of the .zip download releases ever available through genericons.com

So if you want to use that in the genericons twentyfourteen style, I mean, go ahead, but be aware that you're intentionally forking genericons.css

Jetpack

Just to be clear, Jetpack just wp_register_style()'s -- it doesn't enqueue it. So Jetpack would not result in an additional version being loaded concurrently on the front-end.

[georgestephanis]

It looks like the non-standard selector traced back to Obenland's patch on #24595

​http://core.trac.wordpress.org/attachment/ticket/24595/24595.patch

[obenland]

The purpose of this ticket is specifically NOT to do that. If we use a different handle, then we're only trying to step around the problem instead of actually fixing it.

I agree, we shouldn't register/enqueue our own version of genericons.

The only big problem with the raw Genericons css is that it adds styles to .genericon instead of .genericon:before, which is why Twenty Fourteen can get pretty nasty with that. @obenland, did Joen have a reason for doing that, or was it accidental and forgotten to be fixed (see comment 1)?

I submitted a patch upstream and I thought he wanted to use that in 3.0. I think he had back compat concerns, not sure.

If that's staying, we should adjust our uses of those classes in Twenty Fourteen so that we can use the bundled css.

Yes, I think that'd be the most painless way to deal with that.

And it looks like the syntax that Twenty Fourteen is using [...] doesn't seem to have been in any of the .zip download releases ever available through genericons.com. So if you want to use that in the genericons twentyfourteen style, I mean, go ahead, but be aware that you're intentionally forking genericons.css.

It looks like the non-standard selector traced back to Obenland's patch on #24595.

The syntax was part of the patch I submitted upstream, Genericons was supposed to pick that up. Since then it turned out that using class attribute selectors in WordPress are not a good idea, so this will be (would have been) reverted eventually.

[obenland]

Not sure what I did wrong with that patch, or if the bins mess up the formatting. But I'm confident this could be a step in the right direction. What do you think?

[georgestephanis]

That patch looks good to me. And yeah, showing bins in diffs is a migraine waiting to happen.

[obenland]

Joen just wrote about ​Do’s and Don’ts of Icon Fonts:

Rule #2: Don’t rely too heavily on the helper CSS, that is intended only for casual use.

[lancewillett]

Looking at 25085.2.diff​, a few notes:

• Why is genericons.css removed? Isn't that needed?
• Isn't LICENSE.txt needed to match the same Genericons 3.0 files?

I'm assuming you're testing with and without Jetpack activated.

[lancewillett]

In 25506:

Twenty Fourteen: more Genericons fixes, merging exact files with latest Genericons 3.0 files for consistency and future ease of merging. Props obenland and georgestephanis, fixes #25085.

[lancewillett]

In 25508:

Twenty Fourteen: revert bad Genericons merge leftovers, see #25085.

[iamtakashi]

Minor style adjustments

[lancewillett]

In 25603:

Twenty Fourteen: Genericons minor style adjustments, props iamtakashi. See #25085.