WordPress.org

Make WordPress Core

Opened 2 months ago

Last modified 32 hours ago

#49060 reviewing defect (bug)

wp_die() to accept text_direction = 'ltr'

Reported by: apedog Owned by: SergeyBiryukov
Milestone: 5.4 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 2 months ago.

Download all attachments as: .zip

Change History (7)

#1 @SergeyBiryukov
2 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
2 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
2 months ago

#3 @apedog
2 months ago

  • Keywords has-patch added

#4 @SergeyBiryukov
2 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.


5 days ago

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


32 hours ago

Note: See TracTickets for help on using tickets.