Make WordPress Core

Opened 7 weeks ago

Last modified 7 weeks ago

#62519 reopened enhancement

Trigger _doing_it_wrong in _load_textdomain_just_in_time() even when there are no translations

Reported by: babelhalsupport's profile babelhalsupport Owned by:
Milestone: Future Release Priority: normal
Severity: normal Version: 6.7
Component: I18N Keywords: dev-feedback
Focuses: Cc:

Description

In the file l10n.php we have the function _load_textdomain_just_in_time().
The _doing_it_wrong() call is after the $wp_textdomain_registry->get( … ) call,
will fail if a plugin uses a language file in a custom location.
This way _doing_it_wrong() is not called, since the code returns beforehand.

Solution:
Change the order of the above calls.

Attachments (1)

62519.patch (894 bytes) - added by karthickmurugan 7 weeks ago.
changing the order of function call

Download all attachments as: .zip

Change History (10)

@karthickmurugan
7 weeks ago

changing the order of function call

#1 @karthickmurugan
7 weeks ago

Checked the above issue and created a patch for the same by changing the order by calling _doing_it_wrong function before $wp_textdomain_registry->get( … ) call

#2 @karthickmurugan
7 weeks ago

  • Keywords has-patch added

#3 @swissspidy
7 weeks ago

  • Milestone Awaiting Review deleted
  • Resolution set to worksforme
  • Status changed from new to closed
  • Version changed from 6.7.1 to 6.7

Hi there and welcome to Trac.

The current location is correct. If there is no $path, there is no translation file to load, so there is no reason for warning about loading translations too early.

#4 @babelhalsupport
7 weeks ago

The erros is there.
The plugins calls a translation (eg. _ _() ) and WordPress fails to load a language file, for the translation.
This is definitely doing something wrong.
This happens especially when calling _ _() before load_plugin_textdomain().
Please reconsider, your statement.

Last edited 7 weeks ago by babelhalsupport (previous) (diff)

#5 @swissspidy
7 weeks ago

Do you perhaps have a code/plugin to explain the scenario you are describing? Or even a unit test.

#6 @babelhalsupport
7 weeks ago

Yes, I have a plugin which had this error.
I can post the relevant code parts next week.

#7 @babelhalsupport
7 weeks ago

bh-meeting.php

<?php
/**
 * Plugin Name: BH - Ülések modul
 * Description: Admin felület az ülések kezelésére
 * Version: 2023.12.04
 * Author: Bábelhal Webstudio Kft.
 * Author URI: https://babelhal.hu
 * Text Domain: bh_meeting
 */

[...]

define( 'BH_CPT_MEETING', 'meeting' );

$cpt_args =
[
        BH_CPT_MEETING =>
        [
                'public'             => true,
                'publicly_queryable' => true,
                'show_ui'            => true,
                'show_in_menu'       => true,
                'query_var'          => true,
                'capabilities'       => array(
[...]
                ),
                'has_archive'        => false,
                'hierarchical'       => false,
                'menu_position'      => 30,
                'supports'           => [ 'title', 'editor' ],
                'menu_icon'          => 'dashicons-groups',
                'labels'             =>
                [
                        'name'               => __( 'Seats', 'bh_meeting' ),
                        'singular_name'      => __( 'Seat', 'bh_meeting' ),
                        'menu_name'          => __( 'Seats', 'bh_meeting' ),
[...]
                ]
        ]
];

$params =
[
        'plugin_name'      => plugin_basename( dirname( __FILE__, 1 ) ),
        'plugin_path'      => plugin_dir_path( __FILE__ ),
        'plugin_url'       => plugin_dir_url( __FILE__ ),
        'post_type'        => BH_CPT_MEETING,
        'cpt_args'         => $cpt_args,
];
$instance = new Babelhal\Plugins\BhMeeting\BhMeeting( $params );

BhMeeting.php

<?php
class BhMeeting extends BhPluginBase
{
        public function __construct( $params ) {
                parent::__construct( $params );
[...]
        }
[...]
}

BhPluginBase.php

<?php
abstract class BhPluginBase
{
        public function __construct( $params ) {
                $this->plugin_name      = $params['plugin_name'];
                $this->plugin_path      = $params['plugin_path'];
                $this->plugin_url       = $params['plugin_url'];
                $this->post_type        = $params['post_type'];
                $this->cpt_args         = ( isset( $params['cpt_args'] ) ? $params['cpt_args'] : [] );
[...]
                add_action( 'init', array( $this, 'load_plugin_textdomain' ) );
[...]
                add_action( 'init', array( $this, 'register_custom_post_types' ) );
[...]
        }
[...]
}

Some explanation
We tried to translate the name of the custom post, the menu and related text.
But we did this before calling load_plugin_textdomain().
I think this should trigger a doing_it_wrong.

#8 follow-up: @swissspidy
7 weeks ago

  • Keywords dev-feedback added; has-patch removed
  • Milestone set to Future Release
  • Resolution worksforme deleted
  • Status changed from closed to reopened
  • Summary changed from _doing_it_wrong misplaced in the _load_textdomain_just_in_time() function to Trigger _doing_it_wrong in _load_textdomain_just_in_time() even when there are no translations
  • Type changed from defect (bug) to enhancement

Thanks for the clarification. I see where you're coming from. In this scenario you might indeed be wondering why a string wasn't translated.

The problem there is that this would also cause a warning if there are no translations installed yet, for example if the locale is set to en_US, or if the domain is spelled incorrectly.

Adding a notice even in such cases would be very noisy.

I think one way to mitigate this is by landing #62244 first, which removes the need to call load_plugin_textdomain() at all. This reduces the likelihood of this warning being triggered for custom plugins.

Second, we might want to give this a little bit more time before landing, maybe doing it only in a subsequent release.

#9 in reply to: ↑ 8 @babelhalsupport
7 weeks ago

Replying to swissspidy:

The problem there is that this would also cause a warning if there are no translations installed yet, for example if the locale is set to en_US, or if the domain is spelled incorrectly.

Actually when the domain is spelled incorrectly is also a doing_it_wrong scenario, even if another one.

I think one way to mitigate this is by landing #62244 first, which removes the need to call load_plugin_textdomain() at all. This reduces the likelihood of this warning being triggered for custom plugins.

Second, we might want to give this a little bit more time before landing, maybe doing it only in a subsequent release.

Agreed.

Note: See TracTickets for help on using tickets.