WordPress.org

Make WordPress Core

Opened 2 years ago

Closed 16 months ago

#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 2 years ago.
25083.diff (2.1 KB) - added by yoavf 17 months ago.

Download all attachments as: .zip

Change History (11)

comment:1 @MikeHansenMe2 years ago

  • Keywords has-patch added

comment:2 @markoheijnen2 years 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 @SergeyBiryukov2 years 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: @nacin2 years 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 @nacin22 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 @nacin20 months ago

  • Keywords 3.9-early added

comment:7 @nacin20 months ago

  • Milestone changed from 3.8 to Future Release

@yoavf17 months ago

comment:8 @yoavf17 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 @nacin16 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.