Opened 7 weeks ago
Closed 7 weeks ago
#63944 closed enhancement (fixed)
load_script_textdomain doesn't use plugin's DomainPath for translation lookup
| Reported by: |
|
Owned by: |
|
|---|---|---|---|
| Milestone: | 6.9 | Priority: | normal |
| Severity: | normal | Version: | 5.0 |
| Component: | I18N | Keywords: | has-patch has-unit-tests |
| Focuses: | Cc: |
Description (last modified by )
When load_script_textdomain looks up translations for a script, it doesn't use the plugin's DomainPath header value. That's an unjustified difference between how PHP and JS translations are looked up.
The current workflow is that translations for a script are declared with:
wp_set_script_translations( 'my-script', 'my-domain', $optional_path );
If I omit the $optional_path, translations are looked up only in the "global" wp-content/languages/plugins directory. No "local" plugin path is looked at.
I need to declare the local path explicitly if I want it to be used:
wp_set_script_translations( 'my-script', 'my-domain', plugin_dir_path( __FILE__ ) . 'languages' );
The expected behavior, however, is that I can:
- Call
wp_set_script_translationswithout the$pathargument; - Declare
DomainPath: /languagesin the plugin main file; - Get the translations looked up in plugin's
./languagessubfolder.
This is how PHP translations work, when calling __(), in a plugin's PHP file.
@swissspidy If I understand correctly, respecting DomainPath is a quite recent addition, from 6.8 earlier this year. And this bug looks like an omission?
Fixing this would probably solve also #54797, or at least a big part of it.
Change History (7)
#2
@
7 weeks ago
- Keywords needs-patch needs-unit-tests added
- Milestone changed from Awaiting Review to 6.9
- Type changed from defect (bug) to enhancement
- Version changed from trunk to 5.0
#3
@
7 weeks ago
Hi, while exploring this, I think I found the main issue for this.
In function load_script_textdomain
<?php if ( $path ) { $translations = load_script_translations( $path . '/' . $md5_filename, $handle, $domain ); if ( $translations ) { return $translations; } } $translations = load_script_translations( $languages_path . '/' . $md5_filename, $handle, $domain ); if ( $translations ) { return $translations; }
adding
<?php global $wp_textdomain_registry; if ( isset( $wp_textdomain_registry ) ) { $reg_path = $wp_textdomain_registry->get( $domain, $locale ); if ( $reg_path ) { $reg_path = untrailingslashit( $reg_path ); $translations = load_script_translations( $reg_path . '/' . $md5_filename, $handle, $domain ); if ( $translations ) { return $translations; } } }
should fix it.
If this is fine, I would love to write the patch and unit tests
cc: @swissspidy
#4
@
7 weeks ago
Hmm yeah that could be a valid option as well, delaying the path retrieval until later. Happy to look at any PR with good test coverage :)
This ticket was mentioned in PR #9792 on WordPress/wordpress-develop by @tusharbharti.
7 weeks ago
#5
- Keywords has-patch has-unit-tests added; needs-patch needs-unit-tests removed
## Summary
load_script_textdomain()did not consult a textdomain's registeredDomainPath(the custom path set byload_plugin_textdomain()/ the plugin header). As a result, JS translations required an explicit$pathtowp_set_script_translations()while PHP translations using__()honoredDomainPath.
## Changes
load_script_textdomain()now checks the registry path (if present) and tries loading translations from that path (human-readable filename first, then md5-hashed filename) before falling back toWP_LANG_DIRlookups.- Added unit tests for both plugin domain path check
Trac ticket: https://core.trac.wordpress.org/ticket/63944
I would call this an enhancement, but yes it would definitely help with #54797 (I previously suggested reusing
Domain Paththere after #62244 landed).Inside
wp_set_script_translations/WP_Scripts::set_translations, if$pathis not provided, we can take the$domainarg and look it up inWP_Textdomain_Registryto find the "local" path.