Emoji code not loaded responsibly

[superpoincare]

I think the Wordpress emoji code is not loaded responsibly.

I did some experimentation.

When I disable emojis and move all external js to footer and even load external styles asynchronously with some above the fold css inlined - except Google Fonts - I observe is that fonts are fetched immediately after Google fonts css is downloaded.

On the other hand, if I don't disable emojis, both the initial fetch of Google Fonts css and the subsequent fetch of fonts are delayed. Both.

My conclusion is that the javascript delays both link fetches and CSSOM building.

This I tested on Browserstack on OSX on Chrome and Safari.

Sorry, don't have a demo as this is complicated.

[superpoincare]

Apologies to bump the thread.

I notice the following:

I suppose when a browser has emoji support, the javascript shouldn't load wp-emoji-release.min.js and instead uses native emojis.

However, I observe that it is loading in iPhone 6S with iOS 9, Chrome and Safari on OS X (Sierra Beta and El Capitan).

[lukecavanagh]

You do also see the wp-emoji-release.min.js from loading in the header on site load. But it can be blocked from loading in the header.

remove_action( 'wp_head', 'print_emoji_detection_script', 7 );
remove_action( 'admin_print_scripts', 'print_emoji_detection_script' );
remove_action( 'wp_print_styles', 'print_emoji_styles' );
remove_action( 'admin_print_styles', 'print_emoji_styles' );

[peterwilsoncc]

Replying to superpoincare:

My conclusion is that the javascript delays both link fetches and CSSOM building.

This I tested on Browserstack on OSX on Chrome and Safari.

Sorry, don't have a demo as this is complicated.

Are you able to run the comparison on webpagetest.org and add a link to the ticket? It will help a great deal, thanks.

[superpoincare]

Replying to peterwilsoncc:

I find it difficult to separate noise from trend in webpagetest.

I am hence linking to two screenshots of Chrome developer tools. The version used is Chrome 53 dev on OS X Sierra beta on Browserstack.

​http://imgur.com/a/LzACf

(Note: style.min.css is loaded using link rel="preload" and not render-blocking)

I think what is happening is that even though the file wp-emoji-release.min.js is being asked to be inserted after DOM content is loaded is that it is being discovered by the browser and hence rendering blocked.

Also, Pagespeed insights, which is a good tool otherwise, probably has some bug and doesn't detect it. Perhaps it just sees that the js file has been added after DOM content is loaded, so doesn't complain.

Also worth noting is that according to ​http://caniemoji.com/ OS X Chrome has emoji support, so the js file shouldn't be loading. But anyway, I observe the same behaviour on Chrome on Windows which doesn't have emoji support.

[superpoincare]

Replying to lukecavanagh:

Sure one can do that. But that's not the aim. I raised a ticket from a community point of view. If the aim was to disable, I'd just use the plugin disable emojis (which I have used in the tests in the previous comment) and not raise a ticket.

[peterwilsoncc]

I found the WPT results, I can take a look at the data:

FIOS (emoji loaded at top)
​https://www.webpagetest.org/video/compare.php?tests=160825_V4_12NP,160825_CT_12K0

Custom (emoji loaded at bottom)
​https://www.webpagetest.org/video/compare.php?tests=160824_F2_28V5%2C160824_70_2449&thumbSize=200&ival=100&end=full

Clicking the labels to the left of the filmstrips will take you to the individual results.

[superpoincare]

I found out that the code checks whether unicode9 is also supported: ​https://github.com/WordPress/WordPress/blob/ea6e749c7137f1acd7570f78b4ea756e65adace0/wp-includes/js/wp-emoji-loader.js#L98

So wp-emoji-release.min.js should be loading in Chrome on Mac, as is the case now. Works as expected. ✔️

About loading asynchronously, I checked load times by explicitly using

c.async=true;

in the final output. This doesn't change much, implying that the script is anyway loading asynchronously. ✔️

So I think the only possible source of delay - if at all there is one - is if it takes time to feature detect.

[peterwilsoncc]

Replying to superpoincare:

So I think the only possible source of delay - if at all there is one - is if it takes time to feature detect.

Thanks for the research, it's greatly appreciated.

Canvas can be slow, so it's something to look into ​(ref). Another cause may be that the emoji script is ​delaying onload, which delays the loading of your google fonts.

[tollmanz]

I would also like to recommend that the emoji script is loaded more responsibly. Proving the impact via WPT/Dev Tools/{$your_favorite_profiler} is really tricky and likely to lead to bad decisions. I recently observed a 3 second delay in HTML parsing due to the emoji load, but have had a heck of a time reproducing that same result. These types of tests are subject to network, hardware, site construction, and other random variance that makes specific questions like these nearly impossible to answer.

The issue is that the JS is loaded in both a render and HTML parser blocking manner. This means that under the right conditions, the impact of this seemingly innocuous script could be quite dramatic. As developers, when we know best practices for a specific problem, we should default to those practices and argue to not do them if we are so compelled.

I would recommend that we include an async attribute to the script tag that wraps the emoji JS, as well as add an async attribute to the script that it potentially loads (defer would be even better, but it does not enjoy great cross-browser support). Further, the script should be loaded in the wp_footer, instead of the wp_head.

If there are compatibility concerns, we could add some filters to allow people to change the behavior. We should strive to follow best practices for performant loading of this script, regardless of how minor the impact may be.

[peterwilsoncc]

@tollmanz I agree, I'd like to do some research over the course of the release cycle. The async attribute on inserted scripts is implied.

Does defer hurt browsers lacking support, Can I Use indicates ​browser support has improved?

[desrosj]

This needs a patch to be considered for 4.7.

[peterwilsoncc]

@pento for tests are you happy to trim them down to:

• latest version of unicode
• pride flag
• diversity

In terms of JS loading, I'll look at the affect of waiting until window.load before inserting the script to avoid deleying event. I'll do this in combination with @tollmanz's suggestions above.

[pento]

I'm fine with dropping the simple and unicode8 tests.

• flag: I'm inclined to leave it as a test of both pride flag and country flag. While they're both flags, their implementation is different (white flag + ZWJ + rainbow vs regional indicator letter + regional indicator letter). I recall there being problems in the past of platforms adding everything except country flags.
• unicode9: Leave as is.
• diversity: Leave as is, there are still issues with platforms not implementing diversity.

To avoid blocking the DOM more than we have to, I wonder if it's worth creating the canvas element once, instead of every time browserSupportsEmoji() is called?

Also, the test happens as early as possible, so that it avoids running after the page CSS has been parsed. The CSS reflow it can cause only gets more expensive the later it runs. I would be quite cautious about waiting until window.load to run the test.

[pento]

Now that [38717] is in trunk, I'm changing my recommendations. :-)

Let's only keep flag and emoji4. Until proven otherwise, I expect any platforms implementing Emoji 4 to correctly implement both Unicode 9 and Diversity - we can look at releasing a fix if this turns out to not be the case.

[peterwilsoncc]

In 37817.diff​

• add defer attribute to inserted scripts
• retain only flag, pride flag and emoji4 tests (@pento, correct me if I misunderstood comment:17)
• creates canvas element once only

I'm in two minds as to whether the parsing event should be delayed until the window.onload event runs.

I agree re: delaying the tests until render.

[superpoincare]

I think there's still a chance that a browser has emoji4 support and not unicode9 support. Rainbow flag is emoji4, not unicode 9. Apple already has limited emoji4 support and no unicode9 support, although the latest diff should work.

I was thinking in the other ticket about Woman shrugging, Type 4 ​http://emojipedia.org/woman-shrugging-type-4/

Shrug is in unicode9 but has no skin tones there. Emoji4 has shrug with skin tones. Any browser which can display this emoji4 emoji would automatically include rainbow flag and also unicode9. So detecting this will detect both unicode9, emoji4. In effect only one detection might be needed. (My analysis might be a bit speculative).

[pento]

For 37817.diff​, I'm inclined to change the AU flag test to UN, so it's a flag from emoji4. That's probably going to give us the best result for "does this client render all flags?".

As @superpoincare points out, rainbow flag is emoji4, so that would make the test two flags from emoji4. I'm inclined to keep the two tests, though, until we see that clients implement both. (Tech companies have historically been fairly awful at implementing anything considered "controversial" (see: middle finger emoji), so I wouldn't be surprised if some of them "forget" to implement the rainbow flag.)

We still need to keep the separate flag and emoji4 tests, however. Historically, clients have not been particularly vigorous about implementing flags, so it's good for us to maintain the three states of replacing emoji characters: replace nothing, replace only flags, and replace everything.

[superpoincare]

Good points, pento.

Peter, good idea about creating canvas once. I think adding defer won't do anything because from what I see in the code, the script is added after DOMContentLoaded. I think defer just executes before the DOMContentLoaded event. So by the time a script element has been created, the DOMContentLoaded is already past. So the behaviour should be the same either way (with or without defer).

[peterwilsoncc]

In 37817.2.diff​:

• Flag Test: UN Flag, Pride Flag
• Emoji 4: woman technologist, medium skin tone
• otherwise as above

@superpoincare I kept the defer attribute as it delays the initial parsing of the script until just before the DOM ready event fires.

[superpoincare]

Hi Peter,

Yeah makes sense. I did some tests: with 4.6.1, 4.6.1 with emoji disabled and 4.7-alpha with your diff.

The way I used your diff was by upgrading to the alpha-channel using the beta tester plugin, disabling emoji with remove_action and adding the inline emoji JS by hand in the child theme by manually minifying the latest code (from your diff) with uglifyjs2

Shows good improvement. The way I was experimenting as I mentioned before was to see when fonts files are actually loaded after the Google fonts css is downloaded: since fonts are loaded when the browser can decide which ones to download, one can observe if emoji loader is interfering CSSOM/DOM construction.

The result is here: ​http://www.webpagetest.org/video/compare.php?tests=161016_VZ_cff2dbe633cda84a18207adc37c2d352,161016_MH_077d7f6a83db076a2bb2e69f2a6b1f64,161016_8Y_595b5692971777c061696893035783b4#

The fonts are fetched almost immediately once the css is loaded in the latest code, which suggests that there's good improvement.

[pento]

Thanks for the performance testing, @superpoincare! That's a solid improvement. :-)

37817.2.diff​ reduces the minified JS from 2255 bytes to 1740 bytes, which is pretty sweet.

[peterwilsoncc]

In 38869:

Emoji: Load polyfill responsibly.

Improve performance of Emoji tests and loading of the polyfill.

Reduces the number of tests to determine browser support for emoji to those most likely to fail. Adds the defer flag to the loaded scripts for browsers lacking support.

Props superpoincare for perf testing, peterwilsoncc.
Fixes #37817.

[peterwilsoncc]

Note: commit [38869] includes changes made to src/wp-admin/js/bookmarklet.min.js made during the grunt:precommit step.

[superpoincare]

Hi Peter,

The minified output of wp-emoji-loader.js is also affected by the change.

More specifically, line 5016 in formatting.php:

​https://github.com/WordPress/WordPress/blob/84aebb98969200d5e34a58bad0a8ae2e3f0bcc5c/wp-includes/formatting.php#L5016

Or does it happen automatically at your end?

[pento]

It looks like the GitHub repo has ​stopped syncing for some reason.

The code is correct in the ​build repo.