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 20 months ago.
25083.diff (2.1 KB) - added by yoavf 13 months ago.

Download all attachments as: .zip

Change History (11)

comment:1 @MikeHansenMe20 months ago

  • Keywords has-patch added

comment:2 @markoheijnen20 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 @SergeyBiryukov20 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: @nacin20 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 @nacin19 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 @nacin17 months ago

  • Keywords 3.9-early added

comment:7 @nacin17 months ago

  • Milestone changed from 3.8 to Future Release

@yoavf13 months ago

comment:8 @yoavf13 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 @nacin13 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.