Make WordPress Core

Opened 8 years ago

Closed 8 years ago

Last modified 7 years ago

#37191 closed feature request (wontfix)

i18n request: Escaping single and plural form translated text

Reported by: henrywright's profile henry.wright Owned by: johnbillion's profile johnbillion
Milestone: Priority: normal
Severity: normal Version: 4.5.3
Component: I18N Keywords: has-patch
Focuses: Cc:

Description

I noticed there's esc_html_e() and esc_attr_e() which can be used in place of _e() to escape the translated text string.

Can esc_attr_n() and esc_attr_n() functions be made to help us escape singular and plural translated text?

Ref: https://codex.wordpress.org/Function_Reference/_n

Attachments (3)

37191.diff (1.9 KB) - added by henry.wright 8 years ago.
37191.2.diff (1.9 KB) - added by henry.wright 8 years ago.
37191.3.diff (1.9 KB) - added by henry.wright 8 years ago.

Download all attachments as: .zip

Change History (12)

@henry.wright
8 years ago

#1 @henry.wright
8 years ago

  • Keywords has-patch added

37191.diff introduces two new functions:

  • esc_html_n()
  • esc_attr_n()

Each just escapes the string returned by _n() for safe output.

@henry.wright
8 years ago

#2 @henry.wright
8 years ago

My apologies, _n() doesn't have 2 underscores. I've amended that in 37191.2.diff.

Last edited 8 years ago by henry.wright (previous) (diff)

#3 @JustinSainton
8 years ago

+1 for this!

Shouldn't we adjust the patch to pass $default, rather than $default = 'default' to the _n() inside the functiona?

Version 0, edited 8 years ago by JustinSainton (next)

#4 @henry.wright
8 years ago

Good catch. I'll do that once I'm home this evening.

@henry.wright
8 years ago

#5 @henry.wright
8 years ago

37191.3.diff passes $default to _n(). Thanks again to @JustinSainton.

Last edited 8 years ago by henry.wright (previous) (diff)

#6 @johnbillion
8 years ago

  • Milestone changed from Awaiting Review to 4.7
  • Owner set to johnbillion
  • Status changed from new to reviewing

#7 @johnbillion
8 years ago

  • Milestone 4.7 deleted
  • Resolution set to wontfix
  • Status changed from reviewing to closed

I was originally in favour of these functions, but after giving it more thought I'm going to close this ticket as wontfix.

The reason is that the return value of the _n family of functions is almost always used as the $format parameter in printf() or sprintf() due to the fact the functions return the singular or plural form of a string, and therefore almost always contain a %s placeholder to represent the number. This means if you want to use the return value of _n() in an HTML attribute, you need to wrap the return value of sprintf() in esc_attr().

This means esc_attr_n() would never be an appropriate function to use, without also wrapping the resulting text in esc_attr() after the placeholder replacement.

Here's an example which demonstrates how these functions could inadvertently mask un-escaped input if they were introduced:

$count = $_GET['count'];
printf( esc_html_n( 'Single: %s', 'Plural: %s', $count ), $count );

In the above, it might look at first glance like this text is safely escaped for output, but it's not. The value of $_GET['count'] is not escaped. The correct usage is:

$count = $_GET['count'];
echo esc_html( sprintf( _n( 'Single: %s', 'Plural: %s', $count ), $count ) );

Granted, the same problem affects esc_html__() too, but that function is often used without placeholders in its text, where it's safe. The _n functions are almost exclusively used with placeholders in their text.

#8 @henry.wright
8 years ago

The reason is that the return value of the _n family of functions is almost always used as the $format parameter in printf() or sprintf()

That makes sense. I can't think of a time when I've used the _n fam without sprintf or printf.

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


7 years ago

Note: See TracTickets for help on using tickets.