I18N: Simplify WP_Textdomain_Registry::has() - #7735
Conversation
swissspidy
changed the title
get_path_from_lang_dir for more accurate resultsWP_Textdomain_Registry
Test using WordPress PlaygroundThe changes in this pull request can previewed and tested using a WordPress Playground instance. WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser. Some things to be aware of
For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation. |
…ered. Removing the `$domains_with_translations` property also removed the only condition in `WP_Textdomain_Registry::has()` that covers the state `set_custom_path()` leaves behind: `current` unset because the last lookup missed, while `all` still holds a resolved path for another locale. In that state `has()` started returning false, and because `load_plugin_textdomain()` and `load_theme_textdomain()` only register a path and hand the loading off to `_load_textdomain_just_in_time()`, the already known translations stopped being loaded entirely. That state is only reachable once a custom path has been registered, so check for the custom path directly instead. This keeps the scan (and the deprecated property) out of `get_path_from_lang_dir()` while restoring the previous behaviour. Adds tests covering `has()` for every state it can be asked about, including text domains that merely share a prefix with a translated one, and an end-to-end test for the locale-switch-then-`load_plugin_textdomain()` case. See #62348.
8024eb9 to
bfeaf3a
Compare
swissspidy
changed the title
WP_Textdomain_RegistryWP_Textdomain_Registry::has()
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the Unlinked AccountsThe following contributors have not linked their GitHub and WordPress.org accounts: @claude. Contributors, please read how to link your accounts to ensure your work is properly credited in WordPress releases. Core Committers: Use this line as a base for the props when committing in SVN: To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
Started as an attempt to fix prefix matching in
get_path_from_lang_dir(). That turned out not to be a real bug, but removing the machinery around it is one, so this ends up somewhere different than it began.Translation files were matched by prefix:
With
do-somethinganddo-something-elseinstalled and only the latter translated,do-somethingreally does end up in$domains_with_translations. That part reproduces.It can't change behaviour, though.
$domains_with_translationsis read in exactly one place — the third condition ofhas()— and that condition is only reached whencurrent[$domain]is unset whileall[$domain]is non-empty. Sinceset()always assignscurrent, the only route into that state isset_custom_path().Replaying every operation sequence up to length 3 over
_load_textdomain_just_in_time(),load_textdomain(),load_plugin_textdomain()andload_theme_textdomain(), across 11 language-file layouts: the third condition is reached in 508 states, and in every one of them the text domain has translations of its own, prefix sibling or not. Making the match stricter changes nothing observable.What this PR does
get_path_from_lang_dir().$domains_with_translations.has()checksisset( $this->custom_paths[ $domain ] )in its place.The third condition is only reachable once a custom path has been registered, so the check is equivalent by construction — and it takes per-file work out of a loop that runs on every lookup, which is what the scan was costing in the first place.
Trac ticket: https://core.trac.wordpress.org/ticket/62348
This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.