#45528 closed defect (bug) (fixed)
load_script_textdomain() doesn't load translations when WP installed in a subdirectory with custom content dir
Reported by: | fierevere | Owned by: | herregroen |
---|---|---|---|
Milestone: | 5.0.2 | Priority: | normal |
Severity: | normal | Version: | 5.0 |
Component: | I18N | Keywords: | has-screenshots has-patch has-unit-tests fixed-5.0 |
Focuses: | Cc: |
Description (last modified by )
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
- install localized wordpress in subfolder ( http://localhost:8080/wordpress in my case )
- move installation to root of domain
create index.php, redefine siteurl
- 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' );
- ensure everything is working. (It is!)
Try to load editor, (new post/page) - localization is broken.
Expected: geditor-good.jpg
Got: geditor-bad.jpg
Attachments (10)
Change History (55)
This ticket was mentioned in Slack in #core by yui. View the logs.
6 years ago
#3
@
6 years ago
- Description modified (diff)
- Keywords needs-patch needs-unit-tests added
- Milestone changed from Awaiting Review to 5.0.1
- Severity changed from major to normal
- Summary changed from Editor JSON translations fail to load if WP_CONTENT_DIR been redefined to load_script_textdomain() doesn't load translations when WP installed in a subdirectory with custom content dir
#4
@
6 years ago
patch provided for related issue #45488
https://core.trac.wordpress.org/attachment/ticket/45488/45488.diff
did not helped with editor screen translation
#5
@
6 years ago
Possibly related: https://github.com/WordPress/gutenberg/issues/12670
#6
@
6 years ago
- Keywords has-patch added; needs-patch removed
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.
#7
follow-up:
↓ 9
@
6 years ago
https://core.trac.wordpress.org/attachment/ticket/45528/45528.patch
applied patch, but still seeing same problem with translation
This ticket was mentioned in Slack in #polyglots by yui. View the logs.
6 years ago
#9
in reply to:
↑ 7
@
6 years ago
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'] )
#10
follow-up:
↓ 11
@
6 years ago
( ! 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
#11
in reply to:
↑ 10
@
6 years ago
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.
#12
follow-up:
↓ 13
@
6 years ago
<?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)
#13
in reply to:
↑ 12
@
6 years ago
[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.
#14
follow-up:
↓ 15
@
6 years ago
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/
#15
in reply to:
↑ 14
@
6 years ago
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.
#16
@
6 years ago
- Keywords needs-patch added; has-patch removed
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.
#17
follow-up:
↓ 18
@
6 years ago
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.
#18
in reply to:
↑ 17
@
6 years ago
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.
#19
@
6 years ago
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. )
This ticket was mentioned in Slack in #polyglots by yui. View the logs.
6 years ago
#24
@
6 years ago
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.
This ticket was mentioned in Slack in #core-editor by ocean90. View the logs.
6 years ago
This ticket was mentioned in Slack in #polyglots by yui. View the logs.
6 years ago
#29
@
6 years ago
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.
#30
@
6 years ago
- Keywords needs-testing added
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.
#31
@
6 years ago
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?
#32
@
6 years ago
@akirk a somewhat cleaner option might be to temporarily add a filter to site_url
instead and remove it again after the assertion?
#33
@
6 years ago
- Keywords has-unit-tests added; needs-unit-tests removed
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.
#34
@
6 years ago
45528.5.patch
still works,
works in test environment.
works on live server with WP_CONTENT_DIR in /data/ and WordPress in /wp/
#35
@
6 years ago
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?
#38
@
6 years ago
- Owner set to herregroen
- Resolution set to fixed
- Status changed from new to closed
In 44209:
#39
@
6 years ago
- Keywords fixed-5.0 added; needs-testing removed
- Resolution fixed deleted
- Status changed from closed to reopened
#40
@
6 years ago
- Keywords fixed-5.0 removed
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.
Expected (translated) editor screen