WordPress.org

Make WordPress Core

Opened 5 months ago

Last modified 3 months ago

#49060 reviewing defect (bug)

wp_die() to accept text_direction = 'ltr'

Reported by: apedog Owned by: SergeyBiryukov
Milestone: 5.5 Priority: normal
Severity: normal Version:
Component: I18N Keywords: has-patch
Focuses: Cc:

Description

wp_die() only accepts text_direction='rtl'. It should accept 'ltr' as well.

If site language/locale is set to an rtl language, but a plugin produces a message that does not have a translation text to that rtl language, there is no way to make wp_die() style that message correctly.

Attachments (1)

49060.diff (752 bytes) - added by apedog 5 months ago.

Download all attachments as: .zip

Change History (9)

#1 @SergeyBiryukov
5 months ago

Hi there, thanks for the ticket.

This appears to be a bit trickier than I thought.

_wp_die_process_input() actually supports both `rtl` and `ltr`, however _default_wp_die_handler() only uses that value if `language_attributes()` and `is_rtl()` are not available yet, otherwise it just uses get_language_attributes().

Introduced in [16392].

The passed text_direction argument would need to overwrite the one from get_language_attributes().

Some potential solutions:

  • Use str_replace() to overwrite dir="rtl" with dir="ltr" if $parsed_args['text_direction'] is ltr.
  • Add a new parameter to get_language_attributes() with an array of default arguments.

A workaround would be to use the language_attributes filter before calling wp_die():

add_filter(
	'language_attributes',
	function( $output ) {
		return str_replace( 'dir="rtl"', 'dir="ltr"', $output );
	}
);
wp_die( 'Message', 'Title' );

#2 @apedog
5 months ago

I see.
The way I read this the problem is two-fold

  1. _default_wp_die_handler() prefers language_attributes over explicitly passed text_direction. ie. the argument is only a fallback. This is a design issue and is the main problem IMO.
  1. _default_wp_die_handler() cannot even know if text_direction was explicitly set in $args or was implicitly set by _wp_die_process_input() in $parsed_args. This is a minor technical issue.

Filtering language_attributes seems like overkill to me.

The way I see it, explicitly passed argument should be paramount. Followed by the current handling.
in _default_wp_die_handler() it should look something like this:

$text_direction = $parsed_args['text_direction'];
if ( ! empty( $args['text_direction'] ) && in_array( $args['text_direction'], array( 'ltr', 'rtl' ), true ) ){
    $dir_attr = "dir='$text_direction'";
} else if ( function_exists( 'language_attributes' ) && function_exists( 'is_rtl' ) ) {
    $dir_attr = get_language_attributes();
} else {
    $dir_attr = "dir='$text_direction'";
}

Note: the if condition is tested against $args - not $parsed_args

@apedog
5 months ago

#3 @apedog
5 months ago

  • Keywords has-patch added

#4 @SergeyBiryukov
5 months ago

  • Milestone changed from Awaiting Review to 5.4
  • Owner set to SergeyBiryukov
  • Status changed from new to reviewing

This ticket was mentioned in Slack in #core by david.baumwald. View the logs.


3 months ago

This ticket was mentioned in Slack in #core by david.baumwald. View the logs.


3 months ago

This ticket was mentioned in Slack in #core by david.baumwald. View the logs.


3 months ago

#8 @davidbaumwald
3 months ago

  • Milestone changed from 5.4 to 5.5

With 5.4 RC1 landing tomorrow, this is being moved to 5.5. If any maintainer or committer feels this can be resolved in time or wishes to assume ownership during a specific cycle, feel free to update the milestone.

Note: See TracTickets for help on using tickets.