load_script_textdomain() doesn't load translations when WP installed in a subdirectory with custom content dir

[fierevere]

Translation for Gutenberg editor screen is partial when WP_CONTENT_DIR been redefined. JSON translations are present in WP_CONTENT_DIR/languages but are not used.

Steps to reproduce (fresh WP install):
​https://codex.wordpress.org/Giving_WordPress_Its_Own_Directory

1. install localized wordpress in subfolder ( ​http://localhost:8080/wordpress in my case )
2. move installation to root of domain

create index.php, redefine siteurl

3. define new constants

define( 'WP_CONTENT_DIR', '/siteroot/wp/wp-content' );
define( 'WP_CONTENT_URL', 'http://localhost:8080/wp-content' );
define( 'WP_PLUGIN_DIR', '/siteroot/wp/wp-content/plugins' );
define( 'WP_PLUGIN_URL', 'http://localhost:8080/wp-content/plugins' );

4. ensure everything is working. (It is!)

Try to load editor, (new post/page) - localization is broken.

Expected: geditor-good.jpg
Got: geditor-bad.jpg

[fierevere]

Expected (translated) editor screen

[fierevere]

Broken translation

[fierevere]

#45526 was marked as a duplicate.

[fierevere]

index.php redirector for site root folder. Loads wordpress from /wordpress/

[ocean90]

Related issue: When WordPress is installed in a /wp/ directory, $relative becomes -includes/js/dist/editor.min.js.

Related: #45488

[fierevere]

patch provided for related issue #45488
https://core.trac.wordpress.org/attachment/ticket/45488/45488.diff
did not helped with editor screen translation

[dimadin]

Possibly related: ​https://github.com/WordPress/gutenberg/issues/12670

[herregroen]

This patch should work. The path resolve logic was written expecting the full src as it would be passed to the eventual script tags, which is what allows the easiest handling of custom directories.

A later change during the initial patch means we now get the src from the registered object. The associated filter was run but the earlier fixes of the src were not replicated. Adding these should resolve the issue.

Need to think a bit on how to add a proper unit test for this.

[fierevere]

https://core.trac.wordpress.org/attachment/ticket/45528/45528.patch
applied patch, but still seeing same problem with translation

[tmatsuur]

Replying to fierevere:

https://core.trac.wordpress.org/attachment/ticket/45528/45528.patch
applied patch, but still seeing same problem with translation

Is there a problem with conditional statements that determine whether host name match?

Current:

( ! isset( $src_url['host'] ) || $src_url['host'] !== $content_url['host'] )

In this statement, even if the host names match, it will be false.

I propose the following conditional expression.

( ! isset( $src_url['host'] ) || $src_url['host'] === $content_url['host'] )

[fierevere]

( ! isset( $src_url['host'] ) || $src_url['host'] === $content_url['host'] )

this didnt helped too.

        // If the source is not from WP.
        if ( false === $relative ) {
                return false;
        }

function exiting here with $relative === false

[tmatsuur]

Replying to fierevere:

( ! isset( $src_url['host'] ) || $src_url['host'] === $content_url['host'] )

this didnt helped too.

        // If the source is not from WP.
        if ( false === $relative ) {
                return false;
        }

function exiting here with $relative === false

In my environment it works with this.

if (
	strpos( $src_url['path'], $content_url['path'] ) === 0 &&
//	( ! isset( $src_url['host'] ) || $src_url['host'] !== $content_url['host'] )
	( ! isset( $src_url['host'] ) || $src_url['host'] === $content_url['host'] )
) {
	// Make the src relative the specific plugin or theme.
//	$relative = trim( substr( $src, strlen( $content_url['path'] ) ), '/' );
	$relative = trim( substr( $src_url['path'], strlen( $content_url['path'] ) ), '/' );

If $src is http://localhost/wp-content/plugins/myplugin/assets/js/comon.js,
$relative is assets/js/common.js.

And the translation file myplugin-ja-4a9aafdf461c74b88a1c7041526ae1d6.json is successfully loaded.

[fierevere]

<?php
//      ( ! isset( $src_url['host'] ) || $src_url['host'] !== $content_url['host'] )
        ( ! isset( $src_url['host'] ) || $src_url['host'] === $content_url['host'] )

changing this statement does not help

the piece of debugging code i have:

        $site_url    = wp_parse_url( site_url() );
ob_start(); var_dump($src_url['path']); error_log(ob_get_clean());
ob_start(); var_dump($content_url['path']); error_log(ob_get_clean());
ob_start(); var_dump($site_url['path']); error_log(ob_get_clean());
ob_start(); var_dump($src_url['host']); error_log(ob_get_clean());
ob_start(); var_dump($content_url['host']); error_log(ob_get_clean());
ob_start(); var_dump($site_url['host']); error_log(ob_get_clean());
        // If the host is the same or it's a relative URL.
if (
        strpos( $src_url['path'], $content_url['path'] ) === 0 &&
        ( ! isset( $src_url['host'] ) || $src_url['host'] === $content_url['host'] )
) {
error_log("branch 1");
       // Make the src relative the specific plugin or theme.
        $relative = trim( substr( $src_url['path'], strlen( $content_url['path'] ) ), '/' );
                $relative = explode( '/', $relative );
                $languages_path = WP_LANG_DIR . '/' . $relative[0];
                $relative = array_slice( $relative, 2 );
                $relative = implode( '/', $relative );
        } 
          elseif ( ! isset( $src_url['host'] ) || $src_url['host'] !== $site_url['host'] ) {
error_log("branch 2");
                if ( ! isset( $site_url['path'] ) ) {
error_log("branch 2a");
                        $relative = trim( $src_url['path'], '/' );
                } elseif ( ( strpos( $src_url['path'], $site_url['path'] ) === 0 ) ) {
error_log("branch 2b");
                        // Make the src relative to the WP root.
                        $relative = substr( $src_url['path'], strlen( $site_url['path'] ) );
                        $relative = trim( $relative, '/' );
                }
        } else error_log("branch neither"); 

ob_start(); var_dump($relative); error_log(ob_get_clean());
        // If the source is not from WP.
        if ( false === $relative ) {
                return false;
        }

variable dumps and branch diagnostics:

[Sat Dec  8 12:47:50 2018] string(44) "/wordpress/wp-includes/js/dist/blocks.min.js"

[Sat Dec  8 12:47:50 2018] string(11) "/wp-content"

[Sat Dec  8 12:47:50 2018] string(10) "/wordpress"

[Sat Dec  8 12:47:50 2018] string(9) "localhost"

[Sat Dec  8 12:47:50 2018] string(9) "localhost"

[Sat Dec  8 12:47:50 2018] string(9) "localhost"

[Sat Dec  8 12:47:50 2018] branch neither
[Sat Dec  8 12:47:50 2018] bool(false)

[tmatsuur]

[Sat Dec  8 12:47:50 2018] string(44) "/wordpress/wp-includes/js/dist/blocks.min.js"

[Sat Dec  8 12:47:50 2018] string(11) "/wp-content"

[Sat Dec  8 12:47:50 2018] string(10) "/wordpress"

[Sat Dec  8 12:47:50 2018] string(9) "localhost"

[Sat Dec  8 12:47:50 2018] string(9) "localhost"

[Sat Dec  8 12:47:50 2018] string(9) "localhost"

[Sat Dec  8 12:47:50 2018] branch neither
[Sat Dec  8 12:47:50 2018] bool(false)

The JavaScript file under the wp-includes directory is false under the condition of the previous strpos function.

strpos( $src_url['path'], $content_url['path'] ) === 0

In this case it is not judged whether the host name match.

I think that it is necessary to verify with JavaScript files in the theme and plugins below the wp-content directory.

[fierevere]

Its not plugin/theme, new core editor (block editor/gutenberg) is using json translations too.
They are not loaded with custom WP_CONTENT_DIR

This issue can affect themes/plugins, but it affects core too.

And yes, js files are under wp-includes and translated strings json files in wp-content/languages/

load_script_textdomain()
must handle core js files translation case too, since its used now for editor translation.

[tmatsuur]

Replying to fierevere:

Its not plugin/theme, new core editor (block editor/gutenberg) is using json translations too.
They are not loaded with custom WP_CONTENT_DIR

This issue can affect themes/plugins, but it affects core too.

And yes, js files are under wp-includes and translated strings json files in wp-content/languages/

load_script_textdomain()
must handle core js files translation case too, since its used now for editor translation.

I will understand that.

At the same time I think that this function is an attractive feature of 5.0.0.

Using the wp_set_script_translations function in themes and plugins,
the translation function can be used within JavaScript.

Currently, '[domain]-[locale]-[handle].json' under the theme and plugin can be used.

However, '[domain]-[locale]-[md5-hash].json' can not be used.

I expect that this problem can be solved.
And, I think "Host name comparison condition" is important.

[ocean90]

45528.patch​ also doesn't solve comment:3.

I'm wondering if we need another check with includes_url() and if doing a strict replacement with the full URL would be better, like $path = str_replace( includes_url(), '', $src );. Similar is done in _WP_Editors::editor_settings() for the language files.

[herregroen]

Updated the patch with @tmatsuur's suggestion which should work. Also added a trailingslashit to the site url path comparison which should should solve comment:3 I believe.

Works on my local setup with WP in a subdirectory but I think there's a difference with how it's set up for @fierevere. If anyone could try out this patch that would be awesome.

[tmatsuur]

Replying to herregroen:

Updated the patch with @tmatsuur's suggestion which should work. Also added a trailingslashit to the site url path comparison which should should solve comment:3 I believe.

Thank you for creating the patch file.

[fierevere]

45528.2.patch

works for me with translating core editor.

var_dump for $relative and $md5_filename

[Sat Dec  8 18:34:08 2018] string(36) "wp-includes/js/dist/api-fetch.min.js"

[Sat Dec  8 18:34:08 2018] string(43) "ru_RU-1bba9045bb07c89671c88a3f328548e8.json"

[Sat Dec  8 18:34:08 2018] string(33) "wp-includes/js/dist/blocks.min.js"

[Sat Dec  8 18:34:08 2018] string(43) "ru_RU-1a0cd6a7128913b15c1a10dd68951869.json"

[Sat Dec  8 18:34:08 2018] string(35) "wp-includes/js/dist/keycodes.min.js"

[Sat Dec  8 18:34:08 2018] string(43) "ru_RU-a25d1cc7bf7ca0b4e114f6bea64943f4.json"

[Sat Dec  8 18:34:08 2018] string(37) "wp-includes/js/dist/components.min.js"

[Sat Dec  8 18:34:08 2018] string(43) "ru_RU-7f13c36c641b114bf18cd0bcc9ecc7e0.json"

[Sat Dec  8 18:34:08 2018] string(30) "wp-includes/js/dist/nux.min.js"

[Sat Dec  8 18:34:08 2018] string(43) "ru_RU-0ce75ad2f775d1cac9696967d484808c.json"

[Sat Dec  8 18:34:08 2018] string(33) "wp-includes/js/dist/editor.min.js"

[Sat Dec  8 18:34:08 2018] string(43) "ru_RU-bf0f094965d3d4a95b47babcb35fc136.json"

[Sat Dec  8 18:34:08 2018] string(40) "wp-includes/js/dist/block-library.min.js"

[Sat Dec  8 18:34:08 2018] string(43) "ru_RU-f8f49d9fc4a9cf7d78ec99285417bd9c.json"

[Sat Dec  8 18:34:08 2018] string(36) "wp-includes/js/dist/edit-post.min.js"

[Sat Dec  8 18:34:08 2018] string(43) "ru_RU-8860e58c20c6a2ab5876a0f07be43bd9.json"

[Sat Dec  8 18:34:08 2018] string(41) "wp-includes/js/dist/format-library.min.js"

[Sat Dec  8 18:34:08 2018] string(43) "ru_RU-68f2cec7514bf8563c723a4d675fcfe6.json"

tested this on a live site with more complex directory move set up.
Works too.

PS: WordPress installed in /wp/ ( shall fix Related issue: When WordPress is installed in a /wp/ directory, $relative becomes -includes/js/dist/editor.min.js. )

[babaevan]

45528.2.patch works for me.
Thanks to @fierevere and @herregroen.

[mypacecreator]

It also works in the Japanese locale. Thanks!

[kraftner]

For people looking for a workaround who are not willing to patch core:

It seems as if installing Gutenberg as Plugin fixes the translations until this issue is resolved.

[matteospi]

All is solved right now for me, with Italian language. Thanks! :)

[akirk]

Add support for CDNs that mirror all static assets

[akirk]

Building the proper relative path also fails when the assets are served from another server, like for example for ​Jetpack CDN. The Javascript URL might be something like https://c0.wp.com/c/5.0.1/wp-includes/js/dist/edit-post.min.js.

I've submitted a 45528.3.patch the patch to accommodate for this case.

[swissspidy]

I think all that's needed here are some unit tests and some manual testing as well. Unfortunately I am not around much over the weekend to help out with that. Any volunteers? :-)

This currently breaks I18N for lots of people, so it'd be really great to get this committed as soon as possible.

[akirk]

Add unit tests

[akirk]

I've added some unit tests. I'm not super happy about how it depends on the siteurl option. The default content is http://example.org, we also need to test against http://example.org/wp/. Any thoughts on how to improve that unit test?

[herregroen]

@akirk a somewhat cleaner option might be to temporarily add a filter to site_url instead and remove it again after the assertion?

[akirk]

Improve unit test

[herregroen]

Latest patch looks good to me. I've asked @fierevere on slack to give it a test-run as well.

If that gives no issues I hope to commit it either today or tomorrow, depending on how soon some manual testing can be done.

[fierevere]

45528.5.patch
still works,
works in test environment.
works on live server with WP_CONTENT_DIR in /data/ and WordPress in /wp/

[herregroen]

Added another patch that instead of attempting to resolve CDNs adds a filter that may be used to correctly resolve the relative path.

I don't think core should be responsible for resolving the relative path regardless of the format it takes, that should be up to the plugin or site using the CDN.

@akirk could you check if the filter in the latest patch would be sufficient for your use-case?

[akirk]

@herregroen Sure, that works. A new filter definitely is a viable option. Thanks!

[fierevere]

tested .6

works as fine as .5

[herregroen]

In 44209:

I18N: Fix JavaScript translations for subdirectory installations.

Fixes the load_script_textdomain function not resolving the md5 hash based on the relative path for WordPress installations in a subdirectory. Also adds a filter to allow sites using CDNs or other alternative asset locations to filter the relative path resolution.

Props akirk, fierevere, swissspidy, mypacecreator, babaevan, tmatsuur, ocean90.
Fixes #45528.

[dimadin]

Let me start by saying that JavaScript translations feature was rushed, without consideration what backward compatibility it will break (like mentioned in comment:10:ticket:45425 or #45530), and since WP 5.0 is rushed, without having a chance for proper testings.

I discovered this problem while testing [44232] that is included in 5.0.2-RC1. Accidentally, site I was testing had activated ​plugin I made just recently and it turned out that it prevented JavaScript translations for block editor from loading with 5.0.2-RC1 while it didn't do that with 5.0 and 5.0.1. Since plugin uses script_loader_src filter, it was obviously something related to [44209], and I decided to investigate load_script_textdomain function and how it actually gets relative path it uses.

few hours later

So what load_script_textdomain does is gets handle's _WP_Dependency object and from object's src property (handle's source) tries to get relative path. It assumes that handle's source is in WordPress installation, if it's not, it fails.

Problem is that (as far as I can see) load_script_textdomain unnecessarily applies script_loader_src filter on handle's source before it starts getting relative path. Why unnecessarily? Because script_loader_src filter should change handle's source just before it's printed on a page, while load_script_textdomain function needs raw handle's source, like when it was registered, it doesn't depend on final source used for printing. If source is not on WP, it'll fail anyway. With filter, we are only increasing chances of source not being on WP and failing.

In any case, $src parameter that load_script_textdomain function passes to script_loader_src filter is different than one that is originally passed to the filter in WP_Scripts::do_item() method: it lacks ver query argument. Pre-[44209] it also didn't make full URL for handle's source when it's relative path, this is the reason why I didn't notice problem before (source wasn't full URL).

After removing script_loader_src filter, I didn't see any problem.

Building the proper relative path also fails when the assets are served from another server, like for example for Jetpack CDN.

This is only true for plugins, like Jetpack, that don't use filters to change source and instead ​forcefully change src property of handle's _WP_Dependency object. We shouldn't require that now all plugins that use script_loader_src filter also need to use load_script_textdomain_relative_path filter, this is backward incompatible and unannounced, and it kinda punishes those that used standard way from the start because some didn't.

tl;dr I propose to remove script_loader_src filter from load_script_textdomain function.

[ocean90]

The filter was added as part of [43825]. I don't see a specific need for having the filter here as it's mostly used for plugins to rewrite the URL to a CDN version. I'm fine with removing it now that we also have load_script_textdomain_relative_path to customize the path.

[pento]

In 44288:

i18n: Remove the script_loader_src filter from load_script_textdomain().

This filter is superfluous here, the load_script_textdomain_relative_path should be used for customising the textdomain path.

See #45528.
Props dimadin.

[desrosj]

In 44310:

I18N: Fix JavaScript translations for subdirectory installations.

Fixes the load_script_textdomain function not resolving the md5 hash based on the relative path for WordPress installations in a subdirectory. Also adds a filter to allow sites using CDNs or other alternative asset locations to filter the relative path resolution.

Props akirk, fierevere, swissspidy, mypacecreator, babaevan, tmatsuur, ocean90, herregroen.

Merges [44209] to trunk.

Fixes #45528.

[SergeyBiryukov]

In 44318:

i18n: Remove the script_loader_src filter from load_script_textdomain().

This filter is superfluous here, the load_script_textdomain_relative_path should be used for customising the textdomain path.

Props dimadin.
Merges [44288] to trunk.
See #45528.