Opened 5 years ago
Last modified 23 months ago
#44937 new enhancement
Add _doing_it_wrong to load_plugin_textdomain
Reported by: |
|
Owned by: | |
---|---|---|---|
Milestone: | Awaiting Review | Priority: | normal |
Severity: | normal | Version: | 4.7 |
Component: | I18N | Keywords: | 2nd-opinion |
Focuses: | Cc: |
Description
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;
- Install clean WP
- Install GitHub Updater <8.3 - (8.2.1 - https://github.com/afragen/github-updater/tree/8.2.1)
- Set Site Locale to de_DE or fr_FR or ....
- Set User Locale to en_US
- Install all translations
- 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
Change History (5)
#2
@
5 years ago
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.
#3
in reply to:
↑ 1
@
5 years ago
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 theinit
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
#4
@
5 years ago
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.
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 theinit
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 :-)