Opened 5 years ago
Last modified 4 years ago
#49263 new enhancement
Switching blog doesn't switch locale
Reported by: | iandunn | Owned by: | |
---|---|---|---|
Milestone: | Awaiting Review | Priority: | normal |
Severity: | normal | Version: | |
Component: | I18N | Keywords: | 2nd-opinion |
Focuses: | multisite | Cc: |
Description
#26511 introduced switch_to_locale()
, but didn't take into account the Multisite use case that @rmccue mentioned in ticket:26511#comment:8.
It seems reasonable to expect that switching to a site with a different locale would switch the locale.
You can see that it currently doesn't by setting up a network with the following conditions and code, and then loading each site.
- The locale of site
1
isen_US
- The locale of site
2
ises_MX
- The locale of site
3
isfr_FR
add_action( 'admin_init', function() {
echo get_locale() . '<br>'; // loaded site
_e( 'Howdy, %s' );
echo '<hr>';
switch_to_blog( 2 ); // spanish
echo get_locale() . '<br>';
_e( 'Howdy, %s' );
restore_current_blog();
echo '<hr>';
switch_to_blog( 3 ); // french
echo get_locale() . '<br>';
_e( 'Howdy, %s' );
restore_current_blog();
echo '<hr>';
switch_to_blog( 1 ); // english
echo get_locale() . '<br>';
_e( 'Howdy, %s' );
restore_current_blog();
echo '<hr>';
echo get_locale() . '<br>'; // back to loaded site
_e( 'Howdy, %s' );
wp_die();
} );
The strings are always translated using the loaded site's locale, rather than the switched site.
A rudimentary way to see the desired effect would be something like this:
function switch_to_blog_locale() {
$locale = get_option( 'WPLANG', 'en_US' ); // bypass get_locale() b/c early return is stuck on the starting site.
switch_to_locale( $locale );
}
add_action( 'switch_blog', 'switch_to_blog_locale' );
...although that doesn't take user locales into account, doesn't restore previous locales, etc.
Related: #44844
Attachments (4)
Change History (12)
#2
@
5 years ago
- Keywords reporter-feedback removed
🤔
My first thought would be to cache expensive operations like those in a transient, but I'm guessing you're thinking of some other examples where that wouldn't work well? Or is that a concern even with the site names in the toolbar?
Another idea would be to add a new parameter to switch_to_blog()
which would let the caller explicitly define what should be switched. e.g.,
switch_to_blog( 4, null, array( 'blog_id' => true, 'table_prefix' => true, 'object_cache' => true, 'locale' => true, ) );
The default args could be set to the current behavior, so locale
would be false
.
#3
@
5 years ago
🤔
Another idea would be to add a new parameter to switch_to_blog() which would let the caller explicitly define what should be switched. e.g.,
I initially did not like that idea, but it's actually pretty good. Could pass those into the related filters then too.
#4
@
5 years ago
49263.1.diff is a rough start. switch_to_blog()
needs a way to get the new site's locale, but get_locale()
caches the old one. I added a param to bypass that, so that we can reuse the logic there in a DRY way.
That updates $GLOBALS['locale']
, though, which causes switch_to_locale()
to return early. It looks like modularizing the inner logic out of get_locale()
and into something like _get_raw_locale()
might be the best way, but I'm still playing around with things.
Let me know if anyone has any feedback.
We'll also need to account for user locales, and work out how to restore the previous locale and other flags in restore_current_blog()
.
---
I'm also making progress on a workaround for sites that need to fix it before this lands, which shows some of what may be necessary here.
#5
@
5 years ago
49263.2.diff takes the approach of modularizing get_locale()
so that the inner logic is reusable without any side-effects on globals/filters.
That's working at first glance, but still needs lots of testing and refinement, as well as clarity around what's needed to properly restore a locale. Also opinions on whether or not the new _get_raw_site_locale()
should be labeled internal or not.
This ticket was mentioned in Slack in #core-multisite by iandunn. View the logs.
5 years ago
#7
@
5 years ago
switch_to_blog()
's 2nd parameter has been deprecated since r21485, but I'm wondering if we could reuse it, instead of adding a new 3rd param? That would make the devex a bit smoother.
Any thoughts?
The old $validate
param was a bool, so we could maintain back-compat by calling _deprecated_argument()
if the value is anything other than an array. After calling it, the value could be overridden with array()
, so that $default_options
would be used, and the current behavior (not switching the locale) would be maintained).
I scanned the directory for plugins that might be affected by that:
> ag "switch_to_blog\(\s?(.*)," --php ./plugins/ | tee scans/switch-to-blog.txt
Most are false-positives from plugins that include a library that defines its own `switch_to_blog()` function, but that function doesn't ever call Core's switch_to_blog()
with a 2nd argument.
So so if we ignore those:
> cat scans/switch-to-blog.txt |grep -v "freemius" > scans/switch-to-blog-not-freemius.txt
...then we get:
> ./summarize-scan.php scans/switch-to-blog-not-freemius.txt 42 matching plugins Matches Plugin Active installs ======= ====== =============== 1 ibexrentacar 10+ 1 radio-buttons-for-taxonomies 10,000+ 1 wp-survey 10+ 2 linkedin-lite 300+ 1 static-html-output-plugin 10,000+ 1 administrative-shortcodes 70+ 2 threewp-broadcast 1,000+ 1 network-posts-extended 600+ 1 blackbaud-sphere 0+ 1 redactor 10+ 1 wp-appkit 1,000+ 1 blogrollsync 10+ 1 export-readers 0+ 2 avatar-manager 10,000+ 1 multilingual-press 300+ 2 social-networks-auto-poster-facebook-twitter-g 100,000+ 1 pollme 10+ 2 mu-widgets 10+ 1 rss-injection 10+ 1 wp-headfoot 40+ 3 lepress-20 10+ 1 wp-postdate 10+ 1 checkbox-for-taxonomies 100+ 1 buddypress 200,000+ 1 wp-boilerplate 10+ 1 members-blog 10+ 1 quizme 10+ 1 wp-submission 10+ 4 multisite-language-switcher 7,000+ 3 woocommerce-billink 200+ 7 wp-optimize 900,000+ 1 jetpack 5,000,000+ 2 lcimedia-broadcast REMOVED 1 paymill 400+ 1 presstest REMOVED 2 broadcast-mu REMOVED 1 pressroom-by-newswire 60+ 1 single-posts-ext 0+ 1 slimjetpack 8,000+ 1 wp-contactme 100+ 1 blognetworking 10+ 1 blog-linkit 10+
_deprecated_argument()
isn't currently being called for the 2nd param, so adding it would introduce new notices for those plugins. We could mitigate that by emailing them, and mentioning it in a dev-note on make/Core.
Also related to #39210 (which I found to have more conversation & context.)
At a cursory, this probably would use
_load_textdomain_just_in_time()
but then would suffer from the same relative performance issues.It makes me (once again) wish we had a base Switcher class, along with a way to register what happens when something is switched where. There are places where switching between sites (or users/posts/locales eventually) needs to be really speedy (like the top toolbar menu getting Site Names) but then other places where switching would expect for things like translations to be properly loaded up.
Today, I'm inclined to suggest that the best solution is also the worst one, which is to wait until a multisite specific bug appears (like an email getting delivered in the wrong locale or something) and intentionally hooking in the locale change as needed instead of doing it always and unhooking for performance benefits.
If/when locale switching can be cached nicely and super speedy (and it's viable for more broad implementation across core) then we could revisit this specific issue and give it another try?
Thoughts?