Remove the requirement to call load_plugin_textdomain() or load_theme_textdomain()

[johnbillion]

In the near future we'll start to see some plugins and themes which do not bundle any translations, instead relying on all translations being powered by language packs from translate.wordpress.org. In this situation, the load_[plugin|theme]_textdomain() call falls back to the WP_LANG_DIR/[plugins|themes] directory.

It makes sense then that the requirement for such a plugin or theme to call load_[plugin|theme]_textdomain() could be removed, because the translation now lives in one of two known locations, and as long as MO files were loaded JIT.

The first time core encounters a localised string with a textdomain that has not already been loaded, it should look for a corresponding MO file first in the WP_LANG_DIR/plugins directory and then in the WP_LANG_DIR/themes directory, and load it if it's found.

[dd32]

This is very similar to something I've been working on - allowing JIT translation loading from the "loaded" textdomains, extending it to auto-load textdomains is something I hadn't thought of, but something we could do easily.

One of the reasons I've been looking into JIT is that currently our POMO classes load all translations upon load_textdomain() call, which if you have many plugins which only have strings on a small percentage of pages (or admin only) causes a bunch of extra memory usage (plus loading MO files is slow)

[swissspidy]

I thought I give this a shot.

What 34114.diff​ does:

• In get_translations_for_domain(), tries to load plugin or theme text domain if the domain is unknown. Maybe performance can be improved a bit here, not sure.
• Creates a new plugin and a new theme with dummy pomo files, calling __() in the tests.

I wasn't sure about the best approach, so now I just copied the theme / plugin files to the appropriate directories. I'm not that familiar with i18n stuff :-)

[nacin]

The first time core encounters a localised string with a textdomain that has not already been loaded, it should look for a corresponding MO file first in the WP_LANG_DIR/plugins directory and then in the WP_LANG_DIR/themes directory, and load it if it's found.

Very smart approach, @johnbillion. And @swissspidy, a very elegant patch with some solid tests.

The main problem here is we'll double the number of file_exists() checks, as at the time of call, we don't know whether to look in plugins or themes. I think there's a decent way out of this one.

There's only ever going to be one or two themes, so maybe we can do that one a bit differently — perhaps by always calling it and eating the 2 or 4 file_exists() checks — and then using load_plugin_textdomain() inside get_translations_for_domain(). Thus 2p + 2t + 2c (where c is child theme and can be 0), rather than 4(p + t + c). That's still a change from what we have now, which is 2( p + t + c).

That's still not great, as there's a good chance a plugin is internationalized but not translated, which means we're burning file_exists() checks on those sites. This includes en_US sites, by the way.

The underlying stat() call isn't going to be terribly expensive in most situations, but we should still seek to avoid unnecessary calls with whatever is reasonable.

[dd32]

The main problem here is we'll double the number of file_exists() checks, as at the time of call, we don't know whether to look in plugins or themes. I think there's a decent way out of this one.

My thoughts here is that we should pre-cache the files which we know about for plugins:

• Plugins: glob() languages/plugins/ to find out which files exist once per pageload, never file_exists() just load directly
• Themes: Can only be one of two, if the textdomain matches STYLESHEET/TEMPLATE then just perform the needed file_exists() checks it's not really worth optimizing this. It could be optimized to not performing file_exists() and just attempting to read the file (which should fail just as fast).

[swissspidy]

Thanks for the quick feedback! It led me to another round of testing.

It's clear that using load_plugin_textdomain() and load_theme_textdomain() isn't the solution. I like the idea of caching the list of translation files, so in 34114.2.diff​ I added a load_textdomain_just_in_time() function that does this the first time it's called (using get_available_languages()).

I haven't done any in-depth testing, but this method is definitely faster. Presumably even better than having every plugin call load_plugin_textdomain().

Some notes:

• Defining WP_LANG_DIR as DIR_TESTDATA . '/languages' would help simplifying and extending the tests. Copying files just feels so wrong. See ​https://wordpress.slack.com/archives/core/p1435764072001069 (ping @ocean90)
• Should #34213 be a prerequisite?
• What happens when a theme and a plugin have the same textdomain?

[swissspidy]

34114.3.diff​ is a refreshed version of 34114.2.diff​ leveraging the fact that WP_LANG_DIR is now defined in the test suite, making the code a bit simpler.

[giuseppe.mazzapica]

@swissspidy So a theme can load a translation from WP_LANG_DIR . '/plugins' and a plugin from WP_LANG_DIR . '/themes'?

It doesn't feel really right...

[swissspidy]

Replying to giuseppe.mazzapica:

@swissspidy So a theme can load a translation from WP_LANG_DIR . '/plugins' and a plugin from WP_LANG_DIR . '/themes'?

It doesn't feel really right...

Pretty sure that's already possible, i.e. when you use __( 'Primary Menu', 'twentyfifteen' ) in your plugin, and the twentyfifteen text domain is loaded, it will translate that text.

[giuseppe.mazzapica]

Replying to swissspidy:

Replying to giuseppe.mazzapica:

@swissspidy So a theme can load a translation from WP_LANG_DIR . '/plugins' and a plugin from WP_LANG_DIR . '/themes'?

It doesn't feel really right...

Pretty sure that's already possible, i.e. when you use __( 'Primary Menu', 'twentyfifteen' ) in your plugin, and the twentyfifteen text domain is loaded, it will translate that text.

If I explicitly load a language file in my plugin folder, as of now, that language file is loaded. Seems I can't do that anymore.

Am I wrong or with your patch there will be no way to load a language file from plugin / theme folder if there's a file for that domain in either plugins or themes language folder?

[ocean90]

Replying to giuseppe.mazzapica:

Am I wrong or with your patch there will be no way to load a language file from plugin / theme folder if there's a file for that domain in either plugins or themes language folder?

If you want to load the files from another folder you still have to use load_plugin_textdomain() or load_theme_textdomain(). This ticket is about an enhancement for plugins/themes which are using language packs.

[giuseppe.mazzapica]

Replying to ocean90:

Replying to giuseppe.mazzapica:

Am I wrong or with your patch there will be no way to load a language file from plugin / theme folder if there's a file for that domain in either plugins or themes language folder?

If you want to load the files from another folder you still have to use load_plugin_textdomain() or load_theme_textdomain(). This ticket is about an enhancement for plugins/themes which are using language packs.

Seems I misread the patch.. For some reason I was convinced that the line

if ( load_textdomain_just_in_time( $domain ) || isset( $l10n[ $domain ] ) ) {

was part of load_plugin_textdomain... but actually is for get_translations_for_domain...

facepalm

[ocean90]

Some feedback on 34114.3.diff​:

• The tests are failing for me:

) Tests_L10n::test_load_plugin_textdomain_just_in_time
Failed asserting that two strings are identical.
--- Expected
+++ Actual
@@ @@
-Das ist ein Dummy Plugin
+This is a dummy plugin

/tests/phpunit/tests/l10n.php:70

2) Tests_L10n::test_load_theme_textdomain_just_in_time
Failed asserting that two strings are identical.
--- Expected
+++ Actual
@@ @@
-Das ist ein Dummy Theme
+This is a dummy theme

/tests/phpunit/tests/l10n.php:99

• In load_textdomain_just_in_time() the default value of the static variable should be null and if ( empty( $cached_mofiles ) ) should be replace with if ( null === $cached_mofiles ). This prevents recurring lookups for installs which don't have any translations installed.
• The tests should remove the test plugin/theme from the plugin/theme directory
  • Do we really have to copy them? Tests_Theme_ThemeDir is changing the global $wp_theme_directories variable, for the plugin we could just require the file like Tests_Import_Import does.

[swissspidy]

@ocean90 I forgot to mention that you need to create the .mo files yourself when applying the patch, as these binary files are not part of it. Can upload them to this ticket though.

Thanks for the heads-up regarding the copying. I don't like that method, but didn't see how else I could do it. Will look at Tests_Theme_ThemeDir and Tests_Import_Import

[swissspidy]

.mo file used in the tests

[swissspidy]

.mo file used in the tests

[swissspidy]

@ocean90 Check out 34114.4.diff​, where I took your suggestions to heart.

[ocean90]

@swissspidy Thanks. The array needs to be initialized, otherwise you'll get a PHP warning: Warning: in_array() expects parameter 2 to be array, null given in /src/wp-includes/l10n.php on line 819.

In 34114.5.diff​:

• Initialize $cached_mofiles as an array if it's null.
• Short-circuit if domain is 'default' which is reserved for core.
• Move tests to a new file l10n/loadTextdomainJustInTime.php.
• In get_translations_for_domain() swap the order of the isset() check and load_textdomain_just_in_time(). No need to try loading the text domain again if it's already there.

[swissspidy]

34114.5.diff​ looks great!

Would be nice if we could do some testing to see how this improves performance.

[swissspidy]

Just did some very basic profiling on a medium-sized site with about a dozen plugins. There were 4 scenarios:

• No patch, en_US
• No patch, de_DE
• Patch applied, en_US
• Patch applied, de_DE

All plugins had de_DE translation files, so for the tests with the patch applied I removed all the load_plugin_textdomain() calls. What I noticed

• When using de_DE (no patch applied), the site was 30ms slower in average, using about 10MB more memory.
• When using en_US (patch applied), load time and memory consumption was identical, not real changes there
• When using de_DE (patch applied), load time was identical as without the patch, memory consumption went down by 1MB.

So, to sum this up, nothing really changes.

[ocean90]

Thanks for the test results @swissspidy!

@DrewAPicture Do you have any feedback on the docs for load_textdomain_just_in_time()?

[DrewAPicture]

@ocean90 The docs look good, though I'd probable mark the function private, e.g. @access private to signal that this is intended for core use.

[swissspidy]

34114.6.diff​ marks _load_textdomain_just_in_time() as private.

[swissspidy]

In 37415:

I18N: Remove the requirement to call load_plugin_textdomain() / load_theme_textdomain().

By initially scanning the wp-content/languages directory and loading available MO files just-in-time, plugins and themes do not need to manually load text domains anymore.

Props swissspidy, ocean90.
Fixes #34114

[swissspidy]

In 37416:

I18N: Add changes missed in [37415].

See #34114.

[jrf]

By initially scanning the wp-content/languages directory and loading available MO files just-in-time, plugins and themes do not need to manually load text domains anymore.

Please make sure that in any communications around this it will say: "plugins and themes for which translations are provided through translate.wordpress.org do not need to manually load text domains anymore"

AFAICS for plugins/themes hosted elsewhere and therefore without access to translate.wordpress.org, this will not work and could actually be problematic. Also see my comment regarding this in #34213#comment:26

[danielhuesken]

There is a notice wenn the filter 'override_load_textdomain' is true. The notice is a 'Undefined index: ...' message for the $l10n Array.

[danielhuesken]

Fix notice when filter override_load_textdomain is ture

[ocean90]

In 37440:

I18N: In get_translations_for_domain() check if the global $l10n was set by _load_textdomain_just_in_time() before accessing it.

Props danielhuesken.
Fixes #34114.

[swissspidy]

In 37855:

I18N: Enable unloading of text domains that have been loaded just in time.

[37415] removed the requirement to call load_plugin_textdomain() / load_theme_textdomain(). This caused unload_textdomain() to not work properly anymore in these cases.

With this change, unloaded text domains need to be explicitly loaded manually if you want to use them again.

Props swissspidy, ocean90.
Fixes #37113. See #34114.

[leemon]

I'm testing 4.6RC in a dev site. A private plugin I developed is no longer loading its .mo files from the plugin's language folder using the following code:

add_action( 'plugins_loaded', 'my_plugins_loaded' );
function my_plugins_loaded() {
    load_plugin_textdomain( 'domain', false, dirname( plugin_basename( __FILE__ ) ) .  '/languages' );
}

I had no issues in 4.5.3. Do I have to change the way I load the plugin's .mo files, now? Any help would be appreciated.

[ocean90]

Replying to leemon:

I'm testing 4.6RC in a dev site.

Can you post ​a new topic in the support forums please? Feel free to mention me.