Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Print skip-link-focus-fix inline instead of enqueueing as blocking script #1323

Closed

Conversation

westonruter
Copy link

Changes proposed in this Pull Request:

Enqueueing scripts is very costly since the browser will block rendering until the script is loaded. What's more is that it is only applicable in IE11, so it is particularly bad to have this blocking script loaded for 90% of browsers which will never use it. (If IE11 supported conditional comments, those could have been used here, but it does not.) Blocking scripts should be minimized as much as possible, either by adding the async attribute or by outputting the script inline.

While core doesn't yet have built-in support for async, core does currently add an inline script for adding the emoji logic (print_emoji_detection_script()/_print_emoji_detection_script()). A similar approach is proposed here for the skip-link-focus-fix script. Just like for the emoji detection script, the skip-link-focus-fix is small in size and it is only applicable to IE11.

Related issue(s):

#1206

*/
function _s_skip_link_focus_fix() {
echo '<script>';
echo file_get_contents( get_template_directory() . '/js/skip-link-focus-fix.js' );
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is file_get_contents still banned from wp.org? If it is, can you push to get it allowed.

@joshmcrty
Copy link
Contributor

It should be possible to add an async (or defer) attribute using the script_loader_tag filter. See https://developer.wordpress.org/reference/hooks/script_loader_tag/ and https://matthewhorne.me/defer-async-wordpress-scripts/ for an example.

@westonruter
Copy link
Author

@joshmcrty it is indeed possible. However, defer at least is still not ideal. As noted in the corresponding PR wprig/wprig#139:

While WP Rig is using defer this still causes a performance penalty since it blocks DOMContentLoaded from firing. So printing the script inline is a better approach.

It's true that async avoids this issue. Nevertheless, avoiding an extra HTTP request for such a small script seems beneficial as in the case of the emoji detection script.

@Ismail-elkorchi
Copy link
Contributor

@westonruter Thank you for the pull request. Can you please update it to include the changes that were made in WordPress/twentynineteen#47 ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants