Make WordPress Core

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: jsnajdr's profile jsnajdr Owned by: swissspidy's profile swissspidy
Milestone: 6.9 Priority: normal
Severity: normal Version: 5.0
Component: I18N Keywords: has-patch has-unit-tests
Focuses: Cc:

Description (last modified by dd32)

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:

  1. Call wp_set_script_translations without the $path argument;
  2. Declare DomainPath: /languages in the plugin main file;
  3. Get the translations looked up in plugin's ./languages subfolder.

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)

#1 @dd32
7 weeks ago

  • Description modified (diff)

#2 @swissspidy
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

I would call this an enhancement, but yes it would definitely help with #54797 (I previously suggested reusing Domain Path there after #62244 landed).

Inside wp_set_script_translations/WP_Scripts::set_translations, if $path is not provided, we can take the $domain arg and look it up in WP_Textdomain_Registry to find the "local" path.

#3 @tusharbharti
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

Version 0, edited 7 weeks ago by tusharbharti (next)

#4 @swissspidy
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 registered DomainPath (the custom path set by load_plugin_textdomain() / the plugin header). As a result, JS translations required an explicit $path to wp_set_script_translations() while PHP translations using __() honored DomainPath.

## 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 to WP_LANG_DIR lookups.
  • Added unit tests for both plugin domain path check

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

#6 @tusharbharti
7 weeks ago

Hi @swissspidy, I have added tests to check for cases where we look for md5 hashed files as well as normal human readable files. If there are other cases that needs to be tests would love to know that.

#7 @swissspidy
7 weeks ago

  • Owner set to swissspidy
  • Resolution set to fixed
  • Status changed from new to closed

In 60728:

I18N: Use Domain Path for script translations if available.

This is a follow-up to [59461] from 6.8, which leveraged Domain Path for just-in-time loading of MO/PHP translation files.

Now, the same is done for script translations, which means plugins/themes shipping with their own translation no longer need to manually specify the translation path when calling wp_set_script_translations().

Props tusharbharti, swissspidy, shailu25, jsnajdr.
See #62244, #54797.
Fixes #63944.

Note: See TracTickets for help on using tickets.