Date settings: check for $wp_locale before using it

[ramonjd]

What?

Check if $wp_locale is set before trying to access its methods.

Why?

If a theme attempts to enqueue scripts in an early hook, e.g., setup_theme, before the textdomain is loaded and $wp_local set, PHP will throw a warning.

Warning: array_values() expects parameter 1 to be array, null given in <b>/home/wpcom/public_html/wp-content/plugins/gutenberg-core/v13.6.0/lib/compat/wordpress-6.1/date-settings.php</b> on line <b>52</b><br />

How?

An isset check.

Testing Instructions

Check that the post publish date works as expected.

Taken from #41648

Testing Instructions

1. Create or edit a post.
2. Open the post date. Observe whether the week starts on e.g. Sunday.
3. Go to Settings → General and change the Week Starts On setting.
4. Create or edit a post.
5. Open the post date. Observe that the calendar has changed.

Screenshots or screencast

Kapture.2022-06-10.at.15.45.03.mp4

[noisysocks]

I was wondering why this doesn't affect Core which also uses $wp_locale.

https://github.com/WordPress/wordpress-develop/blob/299ac4dbb9351371961155ca30d772ba2cebb549/src/wp-includes/script-loader.php#L331

I think it might be because we have a check for did_action( 'init' ) here.

https://github.com/WordPress/wordpress-develop/blob/299ac4dbb9351371961155ca30d772ba2cebb549/src/wp-includes/script-loader.php#L606-L608

Does adding this to the top of gutenberg_update_date_settings fix the issue? If so I'd prefer it since that way it matches what Core does.

if ( ! did_action( 'init' ) ) {
	return;
}

[ramonjd]

I think it might be because we have a check for did_action( 'init' ) here.

Today I Learned. I'll try it out. Thanks for the tip!

[ramonjd]

Does adding this to the top of gutenberg_update_date_settings fix the issue? If so I'd prefer it since that way it matches what Core does.

@noisysocks It makes no difference unfortunately.

What I think we're trying to guard against is a theme or plugin is calling wp_enqueue_script being called before the init, wp_enqueue_scripts, and admin_enqueue_scripts hooks are fired. See: https://codex.wordpress.org/Plugin_API/Action_Reference#Actions_Run_During_a_Typical_Request

The test case is to try to enqueue some scripts before these hooks:

function gutenberg_some_theme_action() {
	wp_add_inline_script( 'gutenberg_register_packages_scripts', "console.log('Hooya!')" );
}
add_action( 'setup_theme', 'gutenberg_some_theme_action' );

WordPress will throw a warning notice:

Notice: Function wp_add_inline_script was called incorrectly. Scripts and styles should not be registered or enqueued until the wp_enqueue_scripts, admin_enqueue_scripts, or login_enqueue_scripts hooks.

Change the hook to init however - add_action( 'init', 'gutenberg_some_theme_action' ); - and we're golden.

I'm not sure why if ( ! did_action( 'init' ) ) { doesn't have any effect in this case.

Regardless I suspect what we're trying to guard against in this PR is an example of "doing it wrong", not something we ought to preemptively act upon, especially since there have been no issues reported.

Does that sound legit? If so, I'll close this PR.

[noisysocks]

Regardless I suspect what we're trying to guard against in this PR is an example of "doing it wrong", not something we ought to preemptively act upon, especially since there have been no issues reported.

Yeah I agree, don't think we should fix a bug that only happens when wp_add_inline_script is used incorrectly especially if there is already a warning that guides developers to the happy path.