WordPress.org

Make WordPress Core

Opened 4 years ago

Closed 4 years ago

Last modified 4 years ago

#35681 closed defect (bug) (fixed)

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

Reported by: ramiy Owned by: ocean90
Milestone: 4.6 Priority: normal
Severity: normal Version:
Component: I18N Keywords: has-patch
Focuses: Cc:
PR Number:

Description

See the attached patch.

Attachments (5)

35681.patch (1.7 KB) - added by ramiy 4 years ago.
35681.2.patch (1.7 KB) - added by ramiy 4 years ago.
35681.3.patch (1.7 KB) - added by ramiy 4 years ago.
Translatable URL's
35681.4.patch (1.7 KB) - added by ramiy 4 years ago.
better approach
35681.5.patch (1.8 KB) - added by ramiy 4 years ago.
use sprintf()

Download all attachments as: .zip

Change History (16)

@ramiy
4 years ago

#1 @ramiy
4 years ago

Two strings changed.

The URL was replaced with %s placeholder.

Translator comments added.

The URL is a new string that already been used in other places (dashboard widget), meaning that we don't create a new string.

#2 @ramiy
4 years ago

  • Keywords has-patch added

#3 follow-up: @swissspidy
4 years ago

Shouldn't esc_url() be used here?

@ramiy
4 years ago

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

Replying to swissspidy:

Shouldn't esc_url() be used here?

Thanks for the feedback. Patch updated.

@ramiy
4 years ago

Translatable URL's

#5 @ramiy
4 years ago

  • Summary changed from Use <a href="%s"> in translation strings - remove the URL from the string to Avoid using HTML tags in translation strings (wp-admin/index.php)

@ramiy
4 years ago

better approach

#6 @ramiy
4 years ago

Old strings:

  • <strong>WordPress News</strong> &mdash; Latest news from the official WordPress project, the <a href="https://planet.wordpress.org/">WordPress Planet</a>, and popular and recent plugins.
  • <strong>WordPress News</strong> &mdash; Latest news from the official WordPress project, the <a href="https://planet.wordpress.org/">WordPress Planet</a>.

New strings:

  • <strong>WordPress News</strong> &mdash; Latest news from the official WordPress project, the %s, and popular and recent plugins.
  • <strong>WordPress News</strong> &mdash; Latest news from the official WordPress project, the %s.
  • https://planet.wordpress.org/
  • WordPress Planet

@ramiy
4 years ago

use sprintf()

This ticket was mentioned in Slack in #core-i18n by ocean90. View the logs.


4 years ago

#8 @ocean90
4 years ago

Looking at the strings and noticed that they are bit outdated. The "WordPress News" widget shows only one popular plugin. The second string, when the user can't install a plugin, should get an "and" instead of the comma.

Moving the URL to a separate string makes sense because it's already used for the widget.

So, what about '<strong>WordPress News</strong> &mdash; Latest news from the official WordPress project, the <a href="%s">WordPress Planet</a>, and a popular plugin.'?

#9 @swissspidy
4 years ago

… and a popular plugin. doesn't sound quite right in my ears.

What about removing that part? Or using plural (e.g. and popular plugins) since the plugin recommendations change from time to time anyway.

#10 @ocean90
4 years ago

  • Owner set to ocean90
  • Resolution set to fixed
  • Status changed from new to closed

In 37553:

Dashboard: Improve grammar for WordPress News help text.

Also, move planet URL to a separate string to match the URL which is used for the feed.

Props ramiy for initial patch.
Fixes #35681.

#11 @ocean90
4 years ago

  • Milestone changed from Awaiting Review to 4.6
Note: See TracTickets for help on using tickets.