WordPress.org

Make WordPress Core

#25083 closed defect (bug) (fixed)

Merge strings for available updates

Reported by: pavelevap Owned by: nacin
Milestone: 3.9 Priority: normal
Severity: normal Version: 3.6
Component: I18N Keywords: has-patch 3.9-early commit
Focuses: Cc:

Description

There are several similar (and long) strings in theme.php and update.php. Not good for translators... I tried to merge them by removing not needed esc_attr() calls (names are escaped), etc. But maybe I am missing something... I also could not find any solution how to better integrate %5$s variable (onclick code) and remove another string. There is a patch to illustrate the issue...

Attachments (2)

merge_update_strings.patch (6.6 KB) - added by pavelevap 22 months ago.
25083.diff (2.1 KB) - added by yoavf 14 months ago.

Download all attachments as: .zip

Change History (11)

comment:1 @MikeHansenMe22 months ago

  • Keywords has-patch added

comment:2 @markoheijnen22 months ago

the escaping there is more for when names has quotes in them. I guess going the other way around is maybe better.

comment:3 @SergeyBiryukov21 months ago

  • Milestone changed from Awaiting Review to 3.7

We should probably remove the title attributes altogether, they just duplicate the plugin/theme name and appear to be useless here. Related: #24766.

comment:4 follow-up: @nacin21 months ago

One option for the onclick is instead of <a href="%4$s" %5$s>, just doing <a %4$s> then sprintf in the attributes. Not sure how nice that is, though — probably not used much elsewhere.

Simply combining many of these similar strings would be a step in the right direction.

I agree about removing duplicative all title attributes.

comment:5 in reply to: ↑ 4 @nacin20 months ago

  • Milestone changed from 3.7 to 3.8

Replying to nacin:

One option for the onclick is instead of <a href="%4$s" %5$s>, just doing <a %4$s> then sprintf in the attributes. Not sure how nice that is, though — probably not used much elsewhere.

Never mind this — more difficult to ensure security and safety.

Let's do this, but in 3.8. Right now 3.7 is in its final few weeks, and there's no need to merge these strings, as we actually end up with a few new and long ones out of it, and every translation team already long ago translated these strings.

comment:6 @nacin18 months ago

  • Keywords 3.9-early added

comment:7 @nacin18 months ago

  • Milestone changed from 3.8 to Future Release

@yoavf14 months ago

comment:8 @yoavf14 months ago

  • Keywords commit added
  • Milestone changed from Future Release to 3.9

Almost opened a new ticket for this :)
25083.diff combines strings without changing the existing ones. I opted to use the safer version, which is actually used twice in update.php. So the patch only updates theme.php.

comment:9 @nacin14 months ago

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

In 27748:

Merge some theme update strings.

props pavelevap, yoavf.
fixes #25083.

Note: See TracTickets for help on using tickets.