Twenty Twenty-One: IE specific polyfills are loaded in all browsers

[ismail.elkorchi]

The file /assets/js/polyfills.js must be enqueued only if the used browser is Internet Explorer, but it shouldn't in other browsers.

​In this function, both the navigation and responsive-embeds scripts are registered with /assets/js/polyfills.js dependency for all users. But we can use $is_IE conditional to register the IE polyfills script only when IE is used.

The code will then look something like this :

<?php
function twenty_twenty_one_scripts() {
        // Note, the is_IE global variable is defined by WordPress and is used
        // to detect if the current browser is internet explorer.
        global $is_IE;
        $twenty_twenty_one_scripts_dependencies = array();
        if ( $is_IE ) {
                // If IE 11 or below, use a flattened stylesheet with static values replacing CSS Variables.
                wp_enqueue_style( 'twenty-twenty-one-style', get_template_directory_uri() . '/assets/css/ie.css', array(), wp_get_theme()->get( 'Version' ) );
                
                wp_register_script(
                        'twenty-twenty-one-ie11-polyfills',
                        get_template_directory_uri() . '/assets/js/polyfills.js',
                        array(),
                        wp_get_theme()->get( 'Version' ),
                        true
                );
                
                array_push( $twenty_twenty_one_scripts_dependencies, 'twenty-twenty-one-ie11-polyfills' );
                
        } else {
                // If not IE, use the standard stylesheet.
                wp_enqueue_style( 'twenty-twenty-one-style', get_template_directory_uri() . '/style.css', array(), wp_get_theme()->get( 'Version' ) );
        }

        // RTL styles.
        wp_style_add_data( 'twenty-twenty-one-style', 'rtl', 'replace' );

        // Print styles.
        wp_enqueue_style( 'twenty-twenty-one-print-style', get_template_directory_uri() . '/assets/css/print.css', array(), wp_get_theme()->get( 'Version' ), 'print' );

        // Threaded comment reply styles.
        if ( is_singular() && comments_open() && get_option( 'thread_comments' ) ) {
                wp_enqueue_script( 'comment-reply' );
        }

        // Main navigation scripts.
        if ( has_nav_menu( 'primary' ) ) {
                wp_enqueue_script(
                        'twenty-twenty-one-primary-navigation-script',
                        get_template_directory_uri() . '/assets/js/primary-navigation.js',
                        $twenty_twenty_one_scripts_dependencies,
                        wp_get_theme()->get( 'Version' ),
                        true
                );
        }

        // Responsive embeds script.
        wp_enqueue_script(
                'twenty-twenty-one-responsive-embeds-script',
                get_template_directory_uri() . '/assets/js/responsive-embeds.js',
                $twenty_twenty_one_scripts_dependencies,
                wp_get_theme()->get( 'Version' ),
                true
        );
}

[peterwilsoncc]

Thanks for the report @ismailelkorchi

I think you're correct, ideally the polyfill would be included for IE11 only but I'll leave the 2021 team to decide whether to milestone this ticket as it may have been discussed earlier.

Unfortunately the $is_IE variable is unreliable for sites with full page caching but the wp_get_script_polyfill() function can be used when enqueuing the script similar to how WordPress uses it in the dashboard for wp-polyfills.

[ismail.elkorchi]

Unfortunately the $is_IE variable is unreliable for sites with full page caching

@peterwilsoncc This should also be a problem for the IE CSS, because the Twenty Twenty-One relies on $is_IE to know which CSS file it should enqueue : ​https://github.com/WordPress/wordpress-develop/blob/master/src/wp-content/themes/twentytwentyone/functions.php#L397-L406

[poena]

The cache problems are known and different solutions were discussed. Using $is_IE was still considered the best solution for the CSS.

We should use the best solution for both the polyfill and the CSS. The best solutions are not necessarily the same for both, but I have never used / am not familiar with ​https://developer.wordpress.org/reference/functions/wp_get_script_polyfill/

https://core.trac.wordpress.org/ticket/52098

[peterwilsoncc]

I've created a proof of concept ​pull request for the script loader:

• The twenty-twenty-one-ie11-polyfills asset becomes a loader with a null source (similar to how core used to do jQuery)
• A new script is registered with the actual source linked, twenty-twenty-one-ie11-polyfills-asset
• I've used the loader pattern to:
  • avoid breaking backward-compatibility with any child themes that are using twenty-twenty-one-ie11-polyfills as a dependency
  • avoid having to double up on the pollyfill script for both the responsive embeds and primary navigation script

Sorry about any confusion around using $is_IE for scripts vs styles.

@poena Would you or the TT1 team mind taking a look at the PR and let me know if you'd prefer a different approach or I've missed a required test?

When I test on IE11 I see the following before primary-navigation.js and responsive-embeds.js>

<script id='twenty-twenty-one-ie11-polyfills-js-after'>
( ( ! Element.prototype.matches ) || ( ! Element.prototype.closest ) || ( window.NodeList && ! NodeList.prototype.forEach ) ) || document.write( '<script src="http://localhost:8889/wp-content/themes/twentytwentyone/assets/js/polyfills.js?ver=1.0"></scr' + 'ipt>' );
</script>

But these JS errors are present

Object doesn't support property or method 'forEach'
responsive-embeds.js (18,2)

Object doesn't support property or method 'forEach'
primary-navigation.js (153,3)

Thanks for the reviews folks,

wp_get_script_polyfill() uses document.write() in an attempt to force the polyfill scripts to load synchronously but IE11 still loads them asynchronously using that method.

In 5a7c3a376c18d36b7abdb9d90d6466611a9454dd I tried a different approach but it ended up breaking gutenberg so I reverted it. If anyone is able to take a look at the commit, that would be dandy.

[desrosj]

I'm going to move this to 5.7. If it's completed in time before 5.6.1, we could potentially consider backporting this.

@carolinan @aristath

I figured it out what I was doing incorrectly. The test should be for the presence of the feature rather than the absence of the feature.

Fixed and ready for another look.

I've tested this in IE11 where the polyfill is loading, and Firefox where it is not.

I have tested the update and it works well, thank you.

[desrosj]

In 49865:

Twenty Twenty-One: Only load IE specific polyfills when actually using Internet Exploreer.

The twenty-twenty-one-ie11-polyfills script now has a null source, and the new twenty-twenty-one-ie11-polyfills-asset (which points to the actual polyfills.js source) will be loaded only if IE is detected by through the use of wp_get_script_polyfill().

Because the original script name remains the same, this change is backwards compatible with any code registering twenty-twenty-one-id11-polyfills as a script dependency.

Props poena, ismail.elkorchi, peterwilsoncc
Fixes #52098.

[desrosj]

Reopening for backport.

[desrosj]

In 49868:

Twenty Twenty-One: Only load IE specific polyfills when actually using Internet Exploreer.

The twenty-twenty-one-ie11-polyfills script now has a null source, and the new twenty-twenty-one-ie11-polyfills-asset (which points to the actual polyfills.js source) will be loaded only if IE is detected by through the use of wp_get_script_polyfill().

Because the original script name remains the same, this change is backwards compatible with any code registering twenty-twenty-one-id11-polyfills as a script dependency.

Props poena, ismail.elkorchi, peterwilsoncc.
Merges [49865] to the 5.6 branch.
Fixes #52098.

[desrosj]

Adding a custom keyword for querying tickets included in the Twenty Twenty-One version 1.1 release in the future.