Make WordPress Core

Opened 4 years ago

Closed 6 months ago

Last modified 6 months ago

#52438 closed defect (bug) (worksforme)

Theme translations in WP_LANG_DIR are loaded twice, no (logical) way to override from a (child) theme.

Reported by: captaincrash's profile captain.crash Owned by:
Milestone: Priority: normal
Severity: minor Version: 5.6
Component: I18N Keywords: needs-patch needs-testing
Focuses: Cc:

Description

I was trying to override some translation strings from within a child theme for a (parent) theme that has online translations (from translate.wordpress.org) that WordPress downloaded into WP_LANG_DIR/themes/domain-locale.mo.

There are a couple of problems getting this to work, at least one of which (2)) I believe is a bug.

1) Both load_theme_textdomain() and consequently load_child_theme_textdomain() will give priority to .mo files found in WP_LANG_DIR and will completely ignore theme based .mo files when the former exist. I tried to circumvent this with load_textdomain() directly, see here and made it load before the parent theme's call to load_theme_textdomain() in order to make my translations take precedence in the merge. I did like proposed in the docs, i.e. in the child theme's functions file (that loads before the parent's) via the after_setup_theme hook.
The question here is should there be a better (more intuitive) way? Maybe calling load_theme_textdomain() with the parent's text domain again (and make it work, of course)?

2) It did not seem to work, so I tried to log which .mo files WordPress tries loading with this code. This showed that the downloaded translation in WP_LANG_DIR gets called twice. The 2nd call is from the parent theme's load_theme_textdomain() and my effort from 1) successfully put my translaions before that 2nd call. But the 1st .mo file loading... I traced this down to this code within l10n.php: the check with _load_textdomain_just_in_time() actually loads the .mo file via load_textdomain() and way to early. And anyway, does this check make any sense:

if ( isset( $l10n[ $domain ] ) || ( _load_textdomain_just_in_time( $domain ) && isset( $l10n[ $domain ] ) ) )

If isset( $l10n[ $domain ] ) is false, how could the condition after || ever be true?
Removing the OR condition with the call to _load_textdomain_just_in_time() also removes the 1st logged .mo file. Then, my translations from the child theme were the first to load and worked like intended, i.e. merge with the parent's translations that come afterwards.

It also works when calling load_textdomain() in the (child) themes functions file directly (not bound to a hook), but that's not how I understand that this should be done?!

I reproduced the double loading .mo files with a fresh WordPress 5.6 install and the default Twenty Twenty-One theme in formal German language.

I hope, I did not miss anything crucial and this report is of any help.

Change History (13)

#1 @captain.crash
4 years ago

Gently bumping this after three months... Anyone?

#2 @SergeyBiryukov
4 years ago

  • Milestone changed from Awaiting Review to 5.8

#3 @SergeyBiryukov
4 years ago

Hi there, welcome back to WordPress Trac!

Thanks for the report, moving to the 5.8 milestone for investigation.

#4 @desrosj
4 years ago

  • Keywords needs-patch needs-testing added
  • Milestone changed from 5.8 to Future Release

Unfortunately, a contributor has not been able to dive into this one to investigate. The 5.8 deadline is today, so this will need to be punted. I'm punting to Future Release, but once this has more information and a patch, it can be moved back to a numbered milestone.

@ocean90 @swissspidy Do you happen to have any insight here?

#5 @ilovecats7
4 years ago

theme that has online translations (from translate.wordpress.org) that WordPress downloaded into WP_LANG_DIR/themes/domain-locale.mo.

I can't seem to reproduce this. I'm using a fresh install of WordPress 5.8, the Twenty Twenty-One theme, and when I switch the language (in Settings > General > Site Language), the theme strings don't change. Is there something else that I need to do in order for WordPress to download the translations from translate.wordpress.org?

#6 follow-up: @swissspidy
6 months ago

  • Milestone Future Release deleted
  • Resolution set to worksforme
  • Status changed from new to closed

Can't reproduce this at first glance. Code to reproduce would be helpful.

#7 in reply to: ↑ 6 @captain.crash
6 months ago

  • Resolution worksforme deleted
  • Status changed from closed to reopened

Replying to swissspidy:

Can't reproduce this at first glance. Code to reproduce would be helpful.

A first glance after almost four years? Nice.
I understand that this is a minor thing, but all the code to reproduce was there or linked to in my original report.

Changes to _load_textdomain_just_in_time() seem to have fixed the double loading issue, but the underlying problems remain.

1) What is the condition, now found here, supposed to be doing? If isset( $l10n[ $domain ] ) is false, how could the condition after || ever be true? But, if that part gets evaluated, then the call to _load_textdomain_just_in_time() might use return load_textdomain( $domain, $mofile, $locale ); as a return value and that could already load a .mo file (see here) which is way too early and not desired in this place.

2) I'm trying to override some translation strings from within a child theme for a (parent) theme that has online translations stored into WP_LANG_DIR/themes/parentdomain-locale.mo. I put my divergent translations for the parent theme into my child theme CHILD/languages/parentdomain-locale.mo, so I do not lose them during updates. Obviously, I want these to take precedence. How am I supposed to make these load before the ones in WP_LANG_DIR/themes/domain-locale.mo?

#8 @swissspidy
6 months ago

  • Resolution set to worksforme
  • Status changed from reopened to closed
What is the condition, now found here, supposed to be doing? If isset( $l10n[ $domain ] ) is false, how could the condition after
ever be true?

_load_textdomain_just_in_time() calls load_textdomain, which modifies the global $l10n[ $domain ] variable. So if the loading is successful, that next condition will be true.

I'm trying to override some translation strings from within a child theme for a (parent) theme that has online translations stored into WP_LANG_DIR/themes/parentdomain-locale.mo. I put my divergent translations for the parent theme into my child theme CHILD/languages/parentdomain-locale.mo, so I do not lose them during updates. Obviously, I want these to take precedence. How am I supposed to make these load before the ones in WP_LANG_DIR/themes/domain-locale.mo?

Let's say you are building a child theme for twentyten.

In your child theme's functions.php you will use code like this:

<?php
function twentyten_child_override_parent_translations() {
        $locale = determine_locale();
        load_textdomain( 'twentyten', get_stylesheet_directory() . "/languages/twentyten-$locale.mo" );
}

add_action( 'after_setup_theme', 'twentyten_child_override_parent_translations' );

Then, create languages/twentyten-de_DE.l10n.php (yes, a PHP file, but MO would work too) in your child theme's directory to override the de_DE translation (to give an example). The file could look like this:

<?php
return array(
        'x-generator'               => 'GlotPress/4.0.1',
        'translation-revision-date' => '2024-08-02 14:38:38+0000',
        'plural-forms'              => 'nplurals=2; plural=n != 1;',
        'project-id-version'        => 'Themes - Twenty Ten',
        'language'                  => 'de',
        'messages'                  => array(
                'Apologies, but the page you requested could not be found. Perhaps searching will help.' => 'My new not found string',
                'Not Found' => 'My new not found string',
        ),
);

This will override the "Not Found" string on the 404 page.

Nothing will be loaded twice.


Note that I am closing this again, but conversation can continue on closed tickets as well.

#9 @captain.crash
6 months ago

Note that I am closing this again, but conversation can continue on closed tickets as well.

Sorry about that. Will leave it closed now.


So if the loading is successful, that next condition will be true.

Right, ok, that makes sense, although that condition still looks very confusing.


Finally, about the code for my child theme's functions.php:

The code you provided is pretty much exactly the code I was using and what I described under 1) of my original post.

This loads the desired MO file, but it loads instead the parent theme's translations which means nothing else (!) but the few strings I'm trying to override are translated.

Back when I opened the ticket (WP 5.6) the child theme's MO file loaded additionally to the parent theme's MO files causing all strings of one text domain to truely merge.

Furthermore, back then, downloaded parent theme translations within WP_LANG_DIR were loaded twice (once always before my child theme MO file - what I described under 2) of my original post).

Now, the parent theme MO file doesn't load at all with the code you provided. The parent theme translations inside the child theme override nothing, they are the only translations loaded for the text domain.

So, my questions remain (they only need to adapt as this seems to work differently now):

A) What I initially meant with "a (logical) way to override from a (child) theme" is this. Given functions like load_theme_textdomain or even load_child_theme_textdomain, there should be a clearer way to achieve what I'm trying to do than using the code you provided, maybe load_parent_theme_textdomain?

B) How can I override some translation strings of a parent theme from within a child theme without losing all the other already translated strings from the parent theme?

I'm using this code logging the loaded MO files:

add_action( 'load_textdomain', function( $domain, $mofile ) {
    echo 'loading file "' . $mofile . '" on domain "' . $domain . '"<br>';
}, 10, 2);

#10 @swissspidy
6 months ago

I think I used an incorrect hook priority in that example, while I was working on some other i18n stuff. There are some new i18n changes in 6.8 compared to 6.7, which already had some big changes.

Here's what I just successfully tested:

<?php

// languages/de_DE.l10n.php: Child theme translations.
// languages/twentyten-de_DE.l10n.php: Overrides for the parent theme translations.

function twentyten_child_setup() {
        // First, load our translation overrides for the parent theme, then load the its original translation.
        $locale = determine_locale();
        load_textdomain( 'twentyten', get_stylesheet_directory() . "/languages/twentyten-$locale.mo" );
        load_textdomain( 'twentyten', WP_LANG_DIR . "/themes/twentyten-$locale.mo" );

        // Load this child theme's translations.
        // Note: Calling load_theme_textdomain() is no longer necessary with 6.8-alpha (trunk), see https://core.trac.wordpress.org/ticket/62244.
        if ( version_compare( $GLOBALS['wp_version'], '6.8-alpha', '<' ) ) {
                load_theme_textdomain( 'twentyten-child', get_stylesheet_directory() . '/languages' );
        }

        // To test, translate a string from the child theme, and one from the parent theme.
        echo __( 'Not Found', 'twentyten-child' );
        echo __( 'Not Found', 'twentyten' );
}

add_action( 'after_setup_theme', 'twentyten_child_setup' );

// Just to make stylesheets working.
function twentyten_child_enqueue_styles() {
        $parent_style = 'twentyten-style';
        wp_enqueue_style( $parent_style, get_template_directory_uri() . '/style.css' );
        wp_enqueue_style(
                'twentyten-child-style',
                get_stylesheet_directory_uri() . '/style.css',
                array( $parent_style ),
                wp_get_theme()->get( 'Version' )
        );
}

add_action( 'wp_enqueue_scripts', 'twentyten_child_enqueue_styles' );

This causes the following translation files to be loaded:

  • wp-content/themes/twentyten-child/languages/twentyten-de_DE.l10n.php - Your overrides for the parent theme translations
  • wp-content/languages/themes/twentyten-de_DE.l10n.php - Parent theme translations
  • wp-content/themes/twentyten-child/languages/de_DE.l10n.php - Child theme translations

#11 @captain.crash
6 months ago

Thanks for the code. This would work of course, but again...
I have a fully translated theme either by the author or the community and only need to change some wording to better fit a project's voice and tone.
I can not just load my divergent parent theme translations anyhow?
(Or rather let that happen automatically?)

The correct way would be - because my parent theme translation overrides would wipe out all parent theme translations - to make sure I load these as well, really?

If load_theme_textdomain() and the likes are becoming obsolete and loading happens automatically, why is it not looking up and loading translations in this order (only MO files for brevity):

  • wp-content/themes/child/languages/locale.mo
  • wp-content/languages/themes/child-locale.mo
  • wp-content/themes/child/languages/parent-locale.mo
  • wp-content/themes/parent/languages/locale.mo
  • wp-content/languages/themes/parent-locale.mo
Last edited 6 months ago by captain.crash (previous) (diff)

#12 @swissspidy
6 months ago

I can not just load my divergent parent theme translations anyhow?

I literally shared the code to do this.

The correct way would be - because my parent theme translation overrides would wipe out all parent theme translations - to make sure I load these as well, really?

The reason is that otherwise your file would be loaded afterwards, and then would not override the translation. Files that are loaded later are only used to "fill in the blanks".

why is it not looking up and loading translations in this order (only MO files for brevity):

Because that's not how the i18n system or the child theme functionality were designed.


This has now diverged quite a bit from the original bug report.

If you want to suggest an enhancement for easier translation overriding (which would basically be the opposite of the existing "fill in the blanks" order, that is now best done in its own enhancement ticket.

#13 @captain.crash
6 months ago

I literally shared the code to do this. [...] The reason is that otherwise your file would be loaded afterwards

No, you shared code where I have to load the parent theme translations as well, although they had been working perfectly fine before I tried to override some of them from my child theme.

And no, as we came to know in this very thread, loading just my overrides would not make them load afterwards, it would make the original parent theme translations not load at all.

Because that's not how [...] child theme functionality were designed.

I'm sorry, but it is exactly how child themes function. Take template hierarchy or add_editor_style() as just two examples: they look up the relevant files in a child theme first. The concept of "filling in the blanks" has nothing to do with it. Blanks would be filled regardless if the file loads first or last.

It was and stays unintuitive to override translations, so yes, naturally, I am inclined to suggest an enhancement (which I thought I've done in a way).

This has now diverged quite a bit from the original bug report.

Indeed, which is mainly because it took about four years and I'm told things work like they work.
It's neither motivational nor rewarding to put all this in its own enhancement ticket, sorry.

Genuinely, thank you for your time nonetheless!

Note: See TracTickets for help on using tickets.