Optimize Social-Logos CSS

[ScottTravisHartley]

This pull request resolves the issue I reported in #13201

This pull request does a few things.

1. It removes the duplicate @font-face rule (to avoid the .eot) version of the file from being downloaded on browsers that don't need/use it.
2. It replaces the data-attribute .woff file with a URL to its source instead.
3. It includes the .woff2 file that has been bundled with the plugin for a while now to further reduce the weight.

Fixes #13201

(I also had a typo in one of the commits where I included woff2 twice so the additional commits were to resolve this).

Testing Instructions

1. Activate the share buttons functionality within jetpack.
2. Add share buttons to frontend.
3. Verify buttons are working as expected with necessary browsers.
4. Profit!

Proposed changelog entry for your changes:

• Optimized social-logos CSS.

[jetpackbot]

	Warnings
⚠️	The PR is missing at least one [Status] label. Suggestions: [Status] In Progress, [Status] Needs Review
⚠️	"Testing instructions" are missing for this PR. Please add some

This is an automated check which relies on PULL_REQUEST_TEMPLATE. We encourage you to follow that template as it helps Jetpack maintainers do their job. If you think 'Testing instructions' or 'Proposed changelog entry' are not needed for your PR - please explain why you think so. Thanks for cooperation 🤖

Generated by 🚫 dangerJS against 2662b58

[jeherve]

@jasmussen Do you think you could take a look at this? You've always been my go-to expert for font optimisations :)

Thank you!

[jasmussen]

Thank you so much for the PR, very much appreciate your work.

So the reason the WOFF font is base64 encoded inline, is actually to solve a challenge with, if I recall correctly, Firefox. Specifically, Firefox blocks cross origin domain requests, unless your server is configured juuust right.

In most cases, this won't be a problem, because the font CSS will live in the same directory as the font file. However as soon as you offload the CSS or the font files to an external CDN — e.g. your website is example.com and a plugin offloads assets to cdn.example.com — Firefox will (or would, last time I checked) block the font from loading.

As mentioned, this can be configured to work on the server. But in many cases people might install Jetpack alongside a CDN plugin and not really know how to configure their server to work with what the CDN plugin does, and the experience will simply appear broken to them.

To be clear, the above is historical context. Things may have changed, but in case they haven't it's important to test whether this is still the case, before merging.

[ScottTravisHartley]

If I recall Internet Explorer does the same behavior and yet we load the .eot file as URL as opposed to being inlined. Also, isn't this fixing a problem that somebody else is creating as Jetpack is following the standard here where the CDN provider may not? However, WordPress Core on a broader note is guilty of this as well when it comes to Dashicons.

It's an interesting issue though because this is a resolution to a problem someone or something else is creating yet the proper course of action isn't my call! The above implementation is certainly the most efficient and echos the standard as closely as possible!

[jeherve]

Firefox blocks cross origin domain requests

This is mentioned here:
https://stackoverflow.com/a/3704578/1045686
http://geoff.evason.name/2010/05/03/cross-domain-workaround-for-font-face-and-firefox/

There are also some more explorations here:
https://www.zachleat.com/web/web-font-data-uris/

WordPress Core on a broader note is guilty of this as well when it comes to Dashicons.

Indeed. I suppose it was implemented that way for the same reasons @jasmussen mentioned above:
https://core.trac.wordpress.org/browser/branches/5.2/src/wp-includes/css/dashicons.css

This was brought up here, but closed as maybelater:
https://core.trac.wordpress.org/ticket/42819

Also related:
WordPress/dashicons#21
WordPress/dashicons#361

If I recall Internet Explorer does the same behavior and yet we load the .eot file as URL as opposed to being inlined.

This doesn't seem to be an issue in Internet Explorer, as far as I can tell:
https://msdn.microsoft.com/en-us/ie/ms530757(v=vs.94)

isn't this fixing a problem that somebody else is creating as Jetpack is following the standard here where the CDN provider may not?

It's definitely fixing a problem that may be fixed on the server directly. That doesn't mean we shouldn't try to play nice and help, though :)

the above is historical context. Things may have changed, but in case they haven't it's important to test whether this is still the case, before merging.

I'll try to test that scenario to see if it is still valid, or if we could go with Scott's changes now.

[jeherve]

The concerns raised here seem to be valid still. I was able to reproduce the issue in different browsers (Chrome and Firefox) when checking out this patch and serving the social logo files (that include the patch) from a CDN on another domain.

When reverting the patch, the errors disappear.

I'm afraid that means we won't be able to fix the problem you raised in #13201 for now, as long as we want to support sites with such a setup.

[jeherve]

Closing this PR as this can't be merged as is right now.