Make WordPress Core

Opened 23 months ago

Closed 20 months ago

Last modified 19 months ago

#57123 closed enhancement (fixed)

Make it easier to switch to a user's locale

Reported by: swissspidy's profile swissspidy Owned by: swissspidy's profile swissspidy
Milestone: 6.2 Priority: normal
Severity: normal Version: 4.7
Component: I18N Keywords: has-patch commit has-unit-tests has-dev-note
Focuses: Cc:

Description

Locale switching was introduced in #26511 to allow doing things like sending emails in the recipient's locale instead of the site's locale.

That's why we see a lot of code like switch_to_locale( get_user_locale( $user ) ) in core and in plugins.

Not only is this very repetitive, it also causes limitations when used in combination with things like the Preferred Languages feature plugin, where one would like to fall back to another locale if the desired one is not available.

That's because the locale switcher lacks context about which user the locale is for.

To improve this, we could introduce a new switch_to_user_locale( $user ) function that a) grabs the user's locale and b) stores the user ID in the locale switcher stack so that at each moment in time we know whose locale we're supposed to be using.

Change History (18)

This ticket was mentioned in PR #3637 on WordPress/wordpress-develop by @swissspidy.


23 months ago
#1

  • Keywords has-patch added; needs-patch removed

#2 @swissspidy
22 months ago

  • Milestone changed from Awaiting Review to 6.2

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


21 months ago

#4 @costdev
20 months ago

  • Keywords changes-requested added

Just updating the ticket's keywords to reflect the current status of the PR.

#5 @swissspidy
20 months ago

  • Keywords commit added; changes-requested removed

#6 @swissspidy
20 months ago

  • Resolution set to fixed
  • Status changed from assigned to closed

In 55161:

I18N: Introduce switch_to_user_locale().

This new function makes it easier to switch to a specific user’s locale by reducing duplicate code and storing the user’s ID as additional context for plugins to consume. Existing usage of switch_to_locale() in core has been replaced with switch_to_user_locale() where appropriate.

Also, this change ensures WP_Locale_Switcher properly filters determine_locale so that anyyone using the determine_locale() function will get the correct locale information when switching is in effect.

Props costdev.
Fixes #57123.
See #26511.

#8 @johnjamesjacoby
20 months ago

This is excellent!

I have a thought about the new method names (get_current_locale, get_current_user_id). Apologies for it being after-commit 😔 and for maybe digging up an old conversation…

Does _current_ convey “global state”, even when used within the confines of a class or Namespace?

Seems like existing global functions that start with get_current_ or restore_current_ established a pattern that “current” could be either “switched” or “originating” depending on the caller.

Is this, perhaps, a decent opportunity to establish a more intentional/explicit/strict naming pattern to differentiate things that are “currently switched” from “reset back to what we started with” ?

Version 0, edited 20 months ago by johnjamesjacoby (next)

#9 @swissspidy
20 months ago

Thanks for raising that. I see how it is confusing that e.g. ::restore_current_locale() refers to the "original" locale and ::get_current_locale() refers the the currently switched to locale.

Suggestion:

Rename ::get_current_locale() to ::get_locale() and ::get_current_user_id() to ::get_user_id().

This way, the existing pattern in core ("current" referring to "originating") is unchanged.

This ticket was mentioned in PR #3952 on WordPress/wordpress-develop by @swissspidy.


20 months ago
#10

  • Keywords has-unit-tests added

This is a follow-up to #3637 / https://core.trac.wordpress.org/changeset/55161 with the following changes:

  • Fix docblock for switch_locale filter. The User ID is false if missing, not null. Props @ocean90.
  • Rename ::get_current_locale() to ::get_locale() and ::get_current_user_id() to ::get_user_id(). Props @JJJ.
  • Add additional test involving restore_previous_locale() as suggested by @ocean90.

Trac ticket: https://core.trac.wordpress.org/ticket/57123

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


20 months ago

#12 @swissspidy
20 months ago

In 55224:

I18N: Improve method names in WP_Locale_Switcher().

This is a follow-up to [55161] to rename ::get_current_locale() to ::get_switched_locale() and ::get_current_user_id() to ::get_switched_user_id() for improved clarity.

Also:

  • Fix docblock for switch_locale filter. The User ID is false if missing, not null.
  • Add additional test involving restore_previous_locale() and improve test cleanup.

And most importantly: happy birthday ocean90! 🎂

Props johnjamesjacoby, ocean90.
See #57123.

This ticket was mentioned in PR #3999 on WordPress/wordpress-develop by @swissspidy.


20 months ago
#14

WP_Textdomain_Registry: Use rtrim instead of untrailingslashit and trailingslashit directly.

Avoids formatting.php dependency and thus prevents errors when using wp_load_translations_early(), for example when in maintenance mode.

Trac ticket: https://core.trac.wordpress.org/ticket/57123

@swissspidy commented on PR #3999:


20 months ago
#15

Let's add a short comment to the file header why both functions cannot be used here. Similar to

https://github.com/WordPress/wordpress-develop/blob/35a678b0e60837c85a16005ec27b1bc867f5dd53/src/wp-includes/functions.php#L2025

Fixed 👍

#16 @SergeyBiryukov
20 months ago

In 55298:

Docs: Correct @since tag for WP_Locale_Switcher::$stack.

The WP_Locale_Switcher::$locales property was replaced with ::$stack in WordPress 6.2.

Follow-up to [38961], [55161], [55224].

See #57123.

Note: See TracTickets for help on using tickets.