WordPress.org

Make WordPress Core

Opened 3 years ago

Last modified 9 months ago

#39210 new defect (bug)

switch_to_locale() unloads all plugin and theme translations

Reported by: gchtr Owned by:
Milestone: Future Release Priority: normal
Severity: normal Version: 4.7
Component: I18N Keywords: has-patch needs-unit-tests dev-feedback
Focuses: Cc:
PR Number:

Description

When using switch_to_locale() in the backend, all translations loaded into the $l10n global will be unloaded, except for the default one. This makes plugin and theme translations unavailable after using switch_to_locale().

In the load_translations method of WP_Locale_Switcher there are two functions called right after each other for each of the currently loaded domains:

unload_textdomain( $domain );
get_translations_for_domain( $domain );

unload_textdomain loads all unloaded functions into the $l10n_unloaded global. Later when _load_textdomain_just_in_time() is called in get_translations_for_domain(), all domains set in $10n_unloaded will be short-circuited in the following statement

// Short-circuit if domain is 'default' which is reserved for core.
if ( 'default' === $domain || isset( $l10n_unloaded[ $domain ] ) ) {
    return false;
}

This results in only the new translations for the domain default being loaded. All plugin and theme translations will be lost. But even when || isset( $l10n_unloaded[ $domain ] is commented out, it doesn’t work, because WordPress then tries to load a language file from the WP_LANG folder, and not from either a theme or plugin directory.

In #26511 it was already mentioned by @rmccue that this might affect emails to be sent in the wrong language: https://core.trac.wordpress.org/ticket/26511#trac-change-8-1430202811399151.

I run into this problem when I want to send notification emails to subscribed email adresses whenever I publish a post.

Test case

I created a minimal theme with a translations for en_EN and de_DE that displays some debug output in the backend and the frontend.

Preparatory steps:

  1. Set site language to German (de_DE).
  2. Set admin user language to English (en_EN). If the language dropdown does not appear, temporarily set the site language to German so the translation is downloaded and then select the users language again.
  3. Install the test theme.

Now an admin notice should appear in the backend that shows debug output. There are string translations saved before and after switch_to_locale. Whenever Untranslated default string shows up, then a translation couldn’t be loaded.

---

Now if this is not intended behavior, I don’t really know how I would approach fixing this.

Attachments (4)

test_theme.zip (4.6 KB) - added by gchtr 3 years ago.
Test theme to show behavior
39210.diff (12.5 KB) - added by yoavf 3 years ago.
39210.2.diff (33.2 KB) - added by swissspidy 2 years ago.
39210.3.diff (3.3 KB) - added by dd32 9 months ago.

Download all attachments as: .zip

Change History (22)

@gchtr
3 years ago

Test theme to show behavior

#1 follow-up: @ocean90
3 years ago

  • Keywords needs-patch added

Hello @gchtr, welcome to WordPress Trac!

Thanks for your detailed report!
The language switcher was introduced in [38961]. The first iterations had support for caching translations during a switch, see 26511.diff:ticket:26511. But this was removed in favour of _load_textdomain_just_in_time(). The committed version had also a change to the POMO library to store the current path of a translation file. But the part in the switcher didn't actually work and got removed in 39330. So yeah, you're right that the switcher can't handle translations by themes/plugins which don't have their translations in WP_LANG_DIR which is used by _load_textdomain_just_in_time().

Sadly we didn't document that in the dev note. It's not really intended behavior, but it's the current behavior. As a workaround you can use the new change_locale action. It gets called when a locale is switched or restored. You can use it to call your custom load_(theme|plugin)_textdomain() function.

This ticket was mentioned in Slack in #core-restapi by atimmer. View the logs.


3 years ago

#3 @gchtr
3 years ago

Thanks for the explanation @ocean90! I see that for performance reasons, it makes total sense to do it like that.

I now use the following workaround, which works fine:

<?php
add_action( 'change_locale', 'force_load_theme_textdomain' );

// Switch to site locale
$site_locale = get_locale();
$locale_switched = switch_to_locale( $site_locale );
<?php
/**
 * Hotfix for WordPress 4.7
 *
 * WordPress removes translations when switch_to_locale is used in backend.
 * Until this error is resolved, we circumvent it by adding a filter
 * to force the site locale for load_theme_textdomain() and then load the missing
 * text domain.
 *
 * @see https://core.trac.wordpress.org/ticket/39210
 */
function force_load_theme_textdomain() {
    add_filter( 'theme_locale', 'force_site_locale_once' );
    load_theme_textdomain( 'my-text-domain', get_template_directory() . '/languages' );
}

/**
 * Forces site locale once.
 *
 * Since 4.7 WordPress always returns the user locale in the backend.
 * This filters tells WordPress to use the site locale once when
 * load_theme_textdomain() is used.
 *
 * @return string   $locale The locale to use.
 */
function force_site_locale_once() {
    // Remove filter to use it only once
    remove_filter( 'theme_locale', 'force_site_locale_once' );

    // Return site locale
    return get_locale();
}

I have to use another filter before using load_theme_textdomain to force-load the site’s text domain and not the user’s, which is the default for the backend.

Now is it really a bug or a matter of documentation (the special case of using switch_to_locale() in the backend)? Am I on an edge case here? Or should this be made more convenient for developers?

Last edited 2 years ago by gchtr (previous) (diff)

#4 @yoavf
3 years ago

39210.diff has a solution to this (very painful) problem. This works by storing all paths from `load_plugin|theme|muplugin' and attempting to load from there again.

@yoavf
3 years ago

#5 @yoavf
3 years ago

  • Keywords has-patch added; needs-patch removed

#6 follow-up: @ocean90
3 years ago

  • Keywords needs-unit-tests added
  • Milestone changed from Awaiting Review to 4.8

Can't $l10n_paths be a property of WP_Locale_Switcher to avoid another global?

I'd really like to change all the load_*_textdomain() functions to behave more like a registration without actually loading the translation. Loading would be handled by _load_textdomain_just_in_time().

#7 in reply to: ↑ 6 @yoavf
3 years ago

Replying to ocean90:

Can't $l10n_paths be a property of WP_Locale_Switcher to avoid another global?

It probably can - I'll see about that

I'd really like to change all the load_*_textdomain() functions to behave more like a registration without actually loading the translation. Loading would be handled by _load_textdomain_just_in_time().

Would you like to explore that in this ticket, or separately?

needs-unit-test

What were you thinking in terms of additional unit tests?

This ticket was mentioned in Slack in #core by flixos90. View the logs.


2 years ago

#9 @ocean90
2 years ago

  • Milestone changed from 4.8 to Future Release

#10 @swissspidy
2 years ago

  • Keywords needs-refresh added

The current patch needs a refresh, but overall seems to work.

I like the registration idea mentioned earlier and I think we can explore that in this ticket.

@swissspidy
2 years ago

#11 @swissspidy
2 years ago

  • Keywords dev-feedback added; needs-refresh removed

Just spent some time on adding a new registry class for storing these paths.

The load_*_textdomain() functions now don't actually load any translations. This only happens when calling translate().

Thus, things like accessing $l10n['default']->headers['Language'] just after switching locales doesn't work unless you call load_*_textdomain() / get_translations_for_domain() between that. That's why the test_switch_to_different_site_locale_if_user_locale_is_set() test is failing right now. If BC in this area is a big concern, $l10n could be turned into an object with magic getters. When someone accesses $l10n['default']->headers, we would manually load the translations for that.

If there's interest, I can put this up on GitHub as a PR for collaboration.

This ticket was mentioned in Slack in #core-i18n by swissspidy. View the logs.


2 years ago

This ticket was mentioned in Slack in #core-i18n by clorith. View the logs.


22 months ago

#14 @pcfreak30
19 months ago

So I spent a good few hours debugging this to find this bug! The bottom line is you can unload translations, but you cant add them back unless they are the core ones. the
_load_textdomain_just_in_time function needs || isset( $l10n_unloaded[ $domain ] )
removed. I was doing this as part of a multisite plugin activation routine with rewrite rules since the language switcher would NOT switch languages properly.

Here is a gist of some of the code in my (public) plugin that I had to use to get my use case fully working: https://gist.github.com/pcfreak30/12a0023518863ad75c2c05c76e8f53da

I think there is a design flaw in this that needs to be looked at again.

Thanks!

#15 @drzraf
13 months ago

@pcfreak30 same couple of hours lost for me too!

Please devs, at least put a ToDo/Warning/whatever alongside the code for switch_to_locale()

Basic use case of what currently fails:

add_action( 'after_setup_theme', function() {
    load_theme_textdomain( 'co', get_template_directory() . '/languages' );
});
// we need to change locale. Eg, according to page template
add_action( 'wp', function() {
    if (is_page() && strtolower(get_field('country_code')) == 'es' && FOOBAR) {
    // Only if languages are installed for core (wp-cli language core install es_ES)
    switch_to_locale("es_ES");
}, 10);
print(__("hello","co")); // expected "hola"

#16 @ocean90
9 months ago

#46230 was marked as a duplicate.

@dd32
9 months ago

#17 in reply to: ↑ 1 @dd32
9 months ago

Replying to ocean90:

So yeah, you're right that the switcher can't handle translations by themes/plugins which don't have their translations in WP_LANG_DIR which is used by _load_textdomain_just_in_time().

Just to add a note that this isn't quite correct (anymore), themes/plugins which have their translations in WP_LANG_DIR and are using _load_textdomain_just_in_time() are definitely affected by this, as [37113]/#37855 cause *any* unloaded textdomain (whether specifically requested, or not) to not be re-loadable.

One work-around is to simply disable the never reload a textdomain functionality using this one-liner:
add_filter( 'change_locale', function() { $GLOBALS['l10n_unloaded'] = array(); } );

Another would be a core change like 39210.3.diff which adds a flag to unload_textdomain() to allow the language switcher to indicate that the textdomain can be reloaded if needed by _load_textdomain_just_in_time().

39210.3.diff doesn't attempt to fix the scenario where a theme/plugin is loading it's own translations, just the WordPress.org-provided translations. @ocean90 what do you think about at least adding that?

Last edited 9 months ago by dd32 (previous) (diff)

This ticket was mentioned in Slack in #meta by dd32. View the logs.


9 months ago

Note: See TracTickets for help on using tickets.