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

Download all attachments as: .zip

Change History (11)

comment:1 @MikeHansenMe19 months ago

  • Keywords has-patch added

comment:2 @markoheijnen19 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 @SergeyBiryukov19 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: @nacin18 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 @nacin17 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 @nacin15 months ago

  • Keywords 3.9-early added

comment:7 @nacin15 months ago

  • Milestone changed from 3.8 to Future Release

@yoavf12 months ago

comment:8 @yoavf12 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 @nacin11 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.