Make WordPress Core

Opened 9 years ago

Closed 9 years ago

#35668 closed defect (bug) (wontfix)

Avoid using HTML tags in translation strings (wp-admin/credits.php)

Reported by: ramiy's profile ramiy Owned by:
Milestone: Priority: normal
Severity: normal Version:
Component: I18N Keywords: has-patch
Focuses: Cc:

Description

See the attached patch.

Attachments (4)

35668.patch (675 bytes) - added by ramiy 9 years ago.
35668.2.patch (689 bytes) - added by ramiy 9 years ago.
35668.3.patch (711 bytes) - added by ramiy 9 years ago.
35668.4.patch (640 bytes) - added by ramiy 9 years ago.

Download all attachments as: .zip

Change History (14)

@ramiy
9 years ago

#1 @ramiy
9 years ago

  • Keywords has-patch added

@SergeyBiryukov / @ocean90 , I would love to get your feedback on the patch.

#2 @ramiy
9 years ago

I tested several options for this.

Original string:

<p class="clear"><?php
	/* translators: %s: https://make.wordpress.org/ */
	printf( __( 'Want to see your name in lights on this page? <a href="%s">Get involved in WordPress</a>.' ),
		__( 'https://make.wordpress.org/' )
	);
?></p>

Option 1:

<p class="clear"><?php
	printf(
		/* translators: %s: get involved link */
		__( 'Want to see your name in lights on this page? %s' ),
		sprintf(
			/* translators: %s: https://make.wordpress.org/ */
			__( '<a href="%s">Get involved in WordPress</a>.' ),
			__( 'https://make.wordpress.org/' )
		)
	);
?></p>

Option 2:

<p class="clear"><?php
	printf(
		/* translators: %s: get involved link */
		__( 'Want to see your name in lights on this page? %s' ),
		' <a href="' . __( 'https://make.wordpress.org/' ) . '">' . __( 'Get involved in WordPress' ) . '</a>'
		)
	);
?></p>

Option 3:

<p class="clear">
	<?php _e( 'Want to see your name in lights on this page?' ); ?> 
	<a href="<?php echo esc_url( 'https://make.wordpress.org/' ); ?>"><?php _e( 'Get involved in WordPress' ); ?></a>
</p>

I prefer the last one, it has no %s placeholders.

#3 follow-ups: @ocean90
9 years ago

  • Keywords has-patch removed

The link isn't translatable anymore and the period after "Get involved in WordPress" is missing.

I think we should leave the string as it is.

#4 in reply to: ↑ 3 @ramiy
9 years ago

  • Keywords has-patch added

Replying to ocean90:

The link isn't translatable anymore and the period after "Get involved in WordPress" is missing.

Option 4:

<p class="clear">
	<?php _e( 'Want to see your name in lights on this page?' ); ?> 
	<a href="<?php echo esc_url( __( 'https://make.wordpress.org/' ) ); ?>"><?php _e( 'Get involved in WordPress' ); ?></a>.
</p>

Option 5:

<p class="clear">
	<?php _e( 'Want to see your name in lights on this page?' ); ?> 
	<a href="<?php echo esc_url( __( 'https://make.wordpress.org/' ) ); ?>"><?php _e( 'Get involved in WordPress' ); ?></a><?php _e( '.' ); ?>
</p>

#5 in reply to: ↑ 3 @ramiy
9 years ago

Replying to ocean90:

I think we should leave the string as it is.

If we want more locals to finish translating wordpress, we have to provide shorter and simpler translation string.

As a the Hebrew (he_IL) GTE, I see that most contributors tend to translate the simple strings, and I have to finish all the long strings by myself.

I also noticed that contributors avoid translating strings with HTML tags because in hebrew letters are RTL and the HTML tags are LRT. It's very confusing (even for me).

This is why I made it my mission to fix as many core translation strings as I can.

Unfortunately, core is only the first step - plugins and theme is the next step.

#6 @ramiy
9 years ago

Option 6:

<p class="clear"><?php
	printf(
		/* translators: %s: link to make.wordpress.org */
		__( 'Want to see your name in lights on this page? %s.' ),
		'<a href="' . esc_url( __( 'https://make.wordpress.org/' ) ) . '">' . __( 'Get involved in WordPress' ) . '</a>'
	);
?></p>

I think that this is the best approach.

Last edited 9 years ago by ramiy (previous) (diff)

@ramiy
9 years ago

#7 @ramiy
9 years ago

Option 7:

<p class="clear"><?php
	printf(
		/* translators: %s: link to make.wordpress.org */
		__( 'Want to see your name in lights on this page? %s.' ),
		sprintf(
			'<a href="%1$s">%2$s</a>',
			esc_url( __( 'https://make.wordpress.org/' ) ),
			__( 'Get involved in WordPress' )
		)
	);
?></p>

@ramiy
9 years ago

#8 @SergeyBiryukov
9 years ago

Like in #35743, it's easy to forget or accidentally remove %s at the end.

I understand the aspiration to make all the strings as simple as possible, but the original string seems simple enough. Removing all the tags from all the strings would not be feasible.

#9 @ramiy
9 years ago

Ok, last one. Option 8:

<p class="clear"><?php
	printf(
		'%1$s <a href="%2$s">%3$s</a>.',
		__( 'Want to see your name in lights on this page?' ),
		esc_url( __( 'https://make.wordpress.org/' ) ),
		__( 'Get involved in WordPress' )
	);
?></p>

This way you can't accidentally forget the placeholder, because the string has no placeholders.

@ramiy
9 years ago

#10 @ocean90
9 years ago

  • Milestone Awaiting Review deleted
  • Resolution set to wontfix
  • Status changed from new to closed

35668.4.patch isn't an option because you can't translate/move the period anymore.

That strings looks good as is.

Note: See TracTickets for help on using tickets.