Some part of the UI can be in the wrong language (edge cases)

[afercia]

Hunting for edge cases in the admin, noticed a couple of things. To reproduce on trunk, set your user language and site language to different values, e.g.:

• User profile > Language > English
• Settings > Site Language > Deutsch

First edge case:

the admin is in English but the release video captions default language is German, the link to the video is:
https://videopress.com/embed/GbdhpGF3?hd=true&defaultLangCode=de

Second edge case:

on the settings page, the Timezone select shows the Cities in German (very noticeable for some cities, I've just highlighted a few):

I'd expect both to be in English, since they're part of the admin interface.

[swissspidy]

Introduced in [38705] / #29783.

[ocean90]

38485.patch​ fixes about.php. The other one is wp_timezone_choice() which uses a static var to load the mo file on-the-fly and only once.

[yale01]

fix for wp_timezone_choice

[yale01]

https://core.trac.wordpress.org/attachment/ticket/38485/384851.1.diff fixes determining user locale in wp_timezone_choice()

[yale01]

rework on my previous patch

[yale01]

in 384851.2.diff, the value of current user locale is assigned to the static var $mo_loaded on first run, so when it changes, the new value of $locale is different from the $mo_loaded and the continents-cities get updated

[ocean90]

@yale01 Thanks for your patch! Calling load_textdomain() a second time for the same textdomain doesn't work at the moment, there needs to be a call to unload_textdomain() first.

For back-compat I think wp_timezone_choice() should get an optional $locale parameter which falls back to get_locale() if it's not set. When calling wp_timezone_choice() in the admin we should pass get_user_locale() to the function.

Please take also a look at our ​coding standards for PHP, there are some missing spaces in your patch.

[ocean90]

In 38952:

About Page: Use get_user_language() for the video subtitles.

Rename $locale to $lang_code to avoid stomping of the global $locale.

See #38485.

[yale01]

added locale argument

[yale01]

Thank you @ocean90.

New patch adds $locale argument to wp_timezone_choice() with default to get_locale() and unloads text domain if already loaded.
In admin options, wp_timezone_choice() is now called with get_user_locale() argument.

[ocean90]

Replying to ocean90:

In 38952:

About Page: Use get_user_language() for the video subtitles.

Rename $locale to $lang_code to avoid stomping of the global $locale.

See #38485.

s/get_user_language/get_user_locale/

[swissspidy]

@yale01 Looks like your patch always loads the text domain from scratch, even when no locale is passed.

Uploaded a new patch, see 38485.diff​. It would be great if y'all could give it a try. Works great on my install.

[yale01]

Hi @swissspidy, you're right, and I also realized later that the diff filename is wrong. Apologies :(

Your patch is clearly better, but it still re-loads the text domain when the $locale argument has the same not null value in different calls, and it can go worse if in the first call it has a value and the second one is null.

Maybe we could store the $locale value in a static var and compare it against the fresh argument ?

[swissspidy]

Yeah a static var is probably the easiest solution, even though it makes the code even more untestable. See 38485.2.diff​ for a new patch.

[davecpage]

In testing i've noticed that plugins which use load_plugin_textdomain such as Akismet show in the site language e.g. Spanish, even though the user language is set to English. This appears to be because within the function it uses get_locale() https://core.trac.wordpress.org/browser/trunk/src/wp-includes/l10n.php#L796 rather than the newer get_user_locale().

However does this raise the problem of plugins which have a mix of content in the admin and the frontend?

[swissspidy]

@davecpage Good catch, thanks! 38485-load-textdomain.diff​ addresses this.

However does this raise the problem of plugins which have a mix of content in the admin and the frontend?

Using that patch, a different translation will be loaded on the front end and the back end, so no problems there.

[swissspidy]

Use user locale in _load_textdomain_just_in_time()

[swissspidy]

In 39068:

I18N: Show available timezones in the user's locale on the settings screen.

Adds a $locale parameter to wp_timezone_choice() to only reload translations when necessary.

Props yale01.
See #38485.

[swissspidy]

In 39069:

I18N: Use the user's locale when loading text domains in the admin.

Leverages get_user_locale() in load_*_textdomain() and _load_textdomain_just_in_time() to always load translations in the user's language when in the admin.

Fixes #38485.

[johnbillion]

@swissspidy How does this, and [39069] in particular, affect admin-ajax.php calls on the front end? Could it result in an Ajax call returning a response in the user's language when the front end of the site is shown in the site language?

[eirikurn]

Hey @swissspidy,

The change broke our vip-quickstart staging server (which is updating to svn master every hour).

It looks like the vip-scanner plugin is translating some strings when it's imported. This causes a call to get_user_locale, but wp_get_current_user has not been defined yet, as pluggable.php is imported later.

[swissspidy]

@johnbillion Good reminder. The whole user language feature actually affects Ajax calls. I'll have a closer look tomorrow.

@eirikurn Is the code of the plugin available somewhere so I can have a look?

[eirikurn]

VIP Scanner Plugin: ​https://github.com/Automattic/vip-scanner
VIP Quickstart environment (autoupdates to Wordpress trunk): ​https://github.com/Automattic/vip-quickstart

Thank you,

[eirikurn]

Here's the stack trace:

<b>Fatal error</b>: Uncaught Error: Call to undefined function wp_get_current_user() in /srv/www/wp/wp-includes/l10n.php:92
Stack trace:
#0 /srv/www/wp/wp-includes/l10n.php(868): get_user_locale()
#1 /srv/www/wp/wp-includes/l10n.php(896): _load_textdomain_just_in_time('vip-scanner')
#2 /srv/www/wp/wp-includes/l10n.php(122): get_translations_for_domain('vip-scanner')
#3 /srv/www/wp/wp-includes/l10n.php(202): translate('Name of theme', 'vip-scanner')
#4 /srv/www/wp-content/plugins/vip-scanner/vip-scanner/config-vip-scanner-wpcom-form.php(6): __('Name of theme', 'vip-scanner')
#5 /srv/www/wp-content/plugins/vip-scanner/vip-scanner/config-vip-scanner.php(4): include_once('/srv/www/wp-con...')
#6 /srv/www/wp-content/plugins/vip-scanner/vip-scanner.php(47): require_once('/srv/www/wp-con...')
#7 /srv/www/wp/wp-settings.php(263): include_once('/srv/www/wp-con...')
#8 /srv/www/wp-config.php(97): require_once('/srv/www/wp/wp-...')
#9 /srv/www/wp/wp-load.php(42): require_once('/srv/www/wp-con...')
#10 /srv/www/wp/wp-admin/admin.php(31): re in <b>/srv/www/wp/wp-includes/l10n.php</b> on line <b>92</b>

I just verified that this also broke the staging site of another of our VIP sites. So I would think that all vendors running vip-quickstart are unable to log into the admin after tonight.

[johnbillion]

In 39070:

I18N: Revert [39069] as it needs some more work.

See #38485

[AramZS]

I can confirm this issue.

After some testing it is clear that some plugins are calling the translation functions before the user functions proposed to be added to the translation process in this ticket have been setup. As a result, anyone calling _x or load_plugin_textdomain at an earlier level than the intended action call of init (those functions may be available earlier than init, but that's what I tested with) described in the documentation will cause a fatal PHP error when the translation function attempts to call the user function before it has been set up.

Arguably these errors would only occur if the plugin's text domain is setup outside of the recommended methodology, but considering those setups worked perfectly before this patch and then cause total site failure after, perhaps we want some gentler transition or depreciation process?

[ocean90]

Replying to johnbillion:

In 39070:

I18N: Revert [39069] as it needs some more work.

See #38485

I think the "some more work" is adding a function_exists( 'wp_get_current_user' ) call to get_user_locale().

[ocean90]

38485-load-textdomain.3.diff​ is 38485-load-textdomain.2.diff​ plus the use of function_exists( 'wp_get_current_user' ). Just like get_current_user_id() does.

[swissspidy]

38485-load-textdomain.4.diff​ adds a bunch of tests to ensure these functions all use get_user_locale() in the admin.

Anyone getting translated strings via Ajax will get strings in the user's locale when they are logged in (as admin-ajax.php is in the admin, after all). Developers should use switch_to_locale() if that's not desired or ideally leverage the REST API. We'll document this in the dev-note.

[swissspidy]

In 39125:

I18N: Move load_textdomain() tests to separate file.

See #38485.

[swissspidy]

In 39127:

I18N: Use the user's locale when loading text domains in the admin.

Leverages get_user_locale() in load_*_textdomain() and _load_textdomain_just_in_time() to always load translations in the user's language when in the admin.

This re-introduces [39069], but now with additional tests and a function_exists( 'wp_get_current_user' ) check in get_user_locale() in case it gets used early.

Props swissspidy, ocean90.
Fixes #38485.

[westonruter]

r39127 is causing a fatal error in Customize Posts plugin:

Uncaught Error: Call to undefined function get_user_by() in /srv/www/wordpress-develop/src/wp-includes/l10n.php:96
Stack trace:
#0 /srv/www/wordpress-develop/src/wp-includes/l10n.php(713): get_user_locale()
#1 /srv/www/wordpress-develop/src/wp-content/plugins/customize-posts/php/class-customize-posts-plugin.php(55): load_plugin_textdomain('customize-posts')
#2 /srv/www/wordpress-develop/src/wp-content/plugins/customize-posts/customize-posts.php(34): Customize_Posts_Plugin->__construct()
#3 /srv/www/wordpress-develop/src/wp-settings.php(298): include_once('/srv/www/wordpr...')
#4 /srv/www/wordpress-develop/src/wp-config.php(114): require_once('/srv/www/wordpr...')
#5 /srv/www/wordpress-develop/src/wp-load.php(37): require_once('/srv/www/wordpr...')
#6 /srv/www/wordpress-develop/src/wp-admin/admin.php(31): require_once('/srv/www/wordpr...')
#7 /srv/www/wordpress-develop/src/wp-admin/customize.php(13): require_once('/srv/www/wordpr...')
#8 {main}
  thrown in <b>/srv/www/wordpress-develop/src/wp-includes/l10n.php

[ocean90]

In 39134:

I18N: In get_user_locale() ensure that $user_id isn't a falsy value before calling get_user_by().

Fixes #38485.

[Presskopp]

When I set site language to english, but in user profile german (or the other way 'round) the top admin bar shows in the opposite language.

[swissspidy]

@Presskopp Do you mean in the back end or the front end? Because on the front end that's currently expected.

[Presskopp]

I made a screenshot. I don't see this as expected, if it is. I can do the same when I turn around english/german. Should the adminbar not reflect the language setting in profile?

[ocean90]

@Presskopp It's currently expected because we don't use user's locale for the front end. See #38643 for changing that in a future release.

[Presskopp]

Perfect. Missed that one. Thank you.