Add _doing_it_wrong to load_plugin_textdomain

[garrett-eclipse]

Hello,

Came across a race condition when the load_plugin_textdomain is called (for lack of a better term) 'naked' in plugins, most recently Github Updater.

This only affects WP 4.7+ as it relates to the User Locale vs. the Site Locale within the admin area.

If you call load_plugin_textdomain in your plugin outside of a init or plugins_loaded action then your plugin will load it's strings based on the site locale rather than the admin's locale.

The Codex - ​https://codex.wordpress.org/Function_Reference/load_plugin_textdomain

It provides an example using plugins_loaded but doesn't indicate any issue.

I'm unsure the root of the issue but I assume the plugin loads prior to the user so their locale isn't available when the MO object is loaded into the $l10n global.

I don't know if it's possible to make the user locale available to plugins running load_plugin_textdomain prior to init and since the issue isn't present prior to 4.7 feel a _doing_it_wrong prompt will help plugin developers avoid the pitfall.

I haven't tried with other plugins or examples but to reproduce;

1. Install clean WP
2. Install GitHub Updater <8.3 - (8.2.1 - ​https://github.com/afragen/github-updater/tree/8.2.1)
3. Set Site Locale to de_DE or fr_FR or ....
4. Set User Locale to en_US
5. Install all translations
6. Visit GitHub Updater dashboard

And you'll find it's not in en_US it's in the site locale.
*If it's half en and half your site locale those are from missing strings on the site locale.

Then if you replace with 8.3+ (​https://github.com/afragen/github-updater/tree/develop) you'll find the panel in user locale as 8.3 fixed this issue by wrapping load_plugin_textdomain in a plugins_loaded action function.

And I'll wrap up with a concept snippet for the load_plugin_textdomain function;

<?php
if ( ! did_action( 'init' ) && ! did_action( 'plugins_loaded' ) ) {
        _doing_it_wrong(
                'load_plugin_textdomain',
                sprintf(
                        /* translators: 1: function 2: init, 3: plugins_loaded */
                        __( 'The %1$s should be called from the %2$s or %3$ hooks.' ),
                        '<code>load_plugin_textdomain</code>',
                        '<code>init</code>',
                        '<code>plugins_loaded</code>'
                ),
                '4.7.0'
        );
}

And it seems I'm not alone;
​https://status301.net/why-load_plugin_textdomain-would-not-work/

Cheers

[swissspidy]

I'm unsure the root of the issue but I assume the plugin loads prior to the user so their locale isn't available when the MO object is loaded into the $l10n global.

That's the reason, yes.

Plugins are loaded early in wp-settings.php, way before the default translations are loaded and the current user is set up.

See ​https://github.com/WordPress/wordpress-develop/blob/e29d895ffd7388fcb6c73c1c5f6057204096dc03/src/wp-settings.php#L307 and ​https://github.com/WordPress/wordpress-develop/blob/e29d895ffd7388fcb6c73c1c5f6057204096dc03/src/wp-settings.php#L404-L450

This happens after plugins_loaded, so the first time you can be sure that everything is set up is in the init hook.

IMO it's a rule of thumb that you shouldn't run any code directly without hooking into some action. This scenario with load_*_textdomain() (which I know people run into from time to time) is just one example. And it's a known one. After all the blog post you linked to is over 8 years old :-)

[Kau-Boy]

I think this warning is a great idea. I wrote a plugin to overwrite some strings and it always fails, when the plugin just calls the load_plugin_textdomain function outside of an action callback.

[garrett-eclipse]

Thanks @swissspidy I appreciate the 2nd-opinion

Replying to swissspidy:

This happens after plugins_loaded, so the first time you can be sure that everything is set up is in the init hook.

Just to confirm this specific case the load_*_textdomain should be called in the init hook.
So the Codex example should update from plugins_loaded to init - ​https://codex.wordpress.org/Function_Reference/load_plugin_textdomain#Examples

And would the recommended hooks for themes be valid? - ​https://codex.wordpress.org/Function_Reference/load_theme_textdomain

Replying to swissspidy:

IMO it's a rule of thumb that you shouldn't run any code directly without hooking into some action. This scenario with load_*_textdomain() (which I know people run into from time to time) is just one example. And it's a known one. After all the blog post you linked to is over 8 years old :-)

Exposing this would be very helpful to theme/plugin developers where core functions are reliant on other actions.
Aside from localization anomalies, I'm sure others can add to the list of calling core functions too early in their plugin, theme, code.

I do see a use of the _doing_it_wrong function throughout core but wonder if it needs to become more of a convention.

Maybe an addition to the handbook on coding core functions to take into account reliance on actions.
​https://make.wordpress.org/core/handbook/best-practices/coding-standards/php/

These warnings are helpful to developers especially when adopting new (maybe just to them) functions that are reliable on action hooks. It flags edge case issues such as i18n that developers may not catch and if action reliance ever changes due to new features such as user locales the warning can be updated which devs can catch in beta/RC.

A good example in core of _doing_it_wrong in context of actions hooks can be found in _wp_scripts_maybe_doing_it_wrong;
​https://github.com/WordPress/WordPress/blob/56c162fbc9867f923862f64f1b4570d885f1ff03/wp-includes/functions.wp-scripts.php#L36-L52

[garrett-eclipse]

And a last thought;

Would it be useful to have a _doing_actions_wrong function to streamline things and make it elegant because Code is Poetry.

It could possibly take three parameters;
string|array - The action(s) required by this function.
semvar - The version this requirement was introduced/updated
conditional - AND/OR - control if actions are dependent

This would allow the translation to be consistent, adding a single string for i18n similar to;
​https://github.com/WordPress/WordPress/blob/56c162fbc9867f923862f64f1b4570d885f1ff03/wp-includes/functions.wp-scripts.php#L36-L52

This would simply call the _doing_it_wrong function with standardized strings and the current FUNCTION.

[alanfuller]

I think this would be a good idea - to add a doing it wrong warning

Having noticed lots of plugins suffering this and spending several hours tracking through to find the issue.