WordPress.org

Make WordPress Core

Opened 8 years ago

Closed 8 years ago

#18156 closed enhancement (fixed)

Removing similar strings in Twenty Eleven

Reported by: pavelevap Owned by: westi
Milestone: 3.3 Priority: normal
Severity: normal Version: 3.2.1
Component: Bundled Theme Keywords: has-patch
Focuses: Cc:
PR Number:

Description

Attached is patch with proposed changes:

  • Changed bold to strong - 2 removable strings
  • Other changes - 2 other removable strings "Reply" and "Leave a Reply".
  • Other 2 strings could be removed and arrows repaired in following changes (not part of attached patch because I am not sure how to solve this). Related problem with arrows is http://core.trac.wordpress.org/ticket/17809, arrows were changed in single.php, but not in image.php (so now there are 2 types of arrows).

Current:

<?php previous_image_link( false, __( '&larr; Previous' , 'twentyeleven' ) ); ?>

Expected to be similar as in single.php (the same arrows):

<?php previous_image_link( false, __( '<span class="meta-nav">&larr;</span> Previous' , 'twentyeleven' ) ); ?>

But previous_image_link() function does not allow HTML in text.

Attachments (6)

twenty_eleven_strings.patch (7.9 KB) - added by pavelevap 8 years ago.
Different_arrows.png (3.9 KB) - added by pavelevap 8 years ago.
18156.patch (1007 bytes) - added by SergeyBiryukov 8 years ago.
18156.2.patch (2.5 KB) - added by SergeyBiryukov 8 years ago.
twenty_eleven_strings_2.patch (924 bytes) - added by pavelevap 8 years ago.
18156.3.patch (1.9 KB) - added by SergeyBiryukov 8 years ago.

Download all attachments as: .zip

Change History (19)

#1 @nacin
8 years ago

This will actually change the strings, rather than just merging some based on where HTML tags appear. I don't think we should bother -- it's only a few strings to a pot file that isn't very large.

#2 @pavelevap
8 years ago

It is also inconsistency problem. There are strings Reply, Leave a reply and Leave a Reply used with the same meaning and same place. Also we use bold in some place and strong in other place. It is a little mess for translators... So, 4 strings can be removed (from 132 total) without problems. Last 2 strings are not only problem from localization point of view, but also graphic. See attached screenshot.

Looking at it I also noticed small graphic problem with arrows (arrow is not in the centre of line, it is a little down). Changing #content nav a ( font-size from 12 to 13px ) for example repair this arrow. I did not notice it some weeks ago, it is probably new problem with Twenty Eleven 1.2.

#3 follow-ups: @nacin
8 years ago

"Leave a Reply" looks like it should be standardized to "Leave a reply". Using bold is a deliberate choice - the use of strong there would not be semantic, and the <b> tag is very convenient for the extra bit of styling.

#4 in reply to: ↑ 3 ; follow-up: @johnbillion
8 years ago

Replying to nacin:

Using bold is a deliberate choice - the use of strong there would not be semantic, and the <b> tag is very convenient for the extra bit of styling.

It should really be a span to allow for styling with CSS. The presentational <b> tag was deprecated donkeys years ago.

#5 in reply to: ↑ 4 @nacin
8 years ago

Replying to johnbillion:

It should really be a span to allow for styling with CSS. The presentational <b> tag was deprecated donkeys years ago.

<b> is again semantic in HTML5, for "stylistically offset" text. That certainly seems like it fits the use case here. When possible, divs and spans should be avoided.

#6 in reply to: ↑ 3 @SergeyBiryukov
8 years ago

  • Milestone changed from Awaiting Review to 3.3

Replying to nacin:

"Leave a Reply" looks like it should be standardized to "Leave a reply".

18156.patch does just that.

Replying to pavelevap:

But previous_image_link() function does not allow HTML in text.

Escaping in wp_get_attachment_link() was introduced in [10495] along with the $text parameter itself and changed to esc_attr() in [11204]. Looks like it shouldn't really be there:

  1. $link_text is not an attribute, it's literally a link text.
  2. adjacent_post_link() doesn't escape link text.

So 18156.2.patch addresses the last point of the ticket as well.

#7 @pavelevap
8 years ago

I did not wanted to discuss if <b> or <strong> is better. I only wanted to remove 2 strings which are duplicates because sometimes there is one tag and sometimes there is another tag. I thought that <strong> is better, but from the point of translators view it is no problem. Real problem is that there are duplicated strings due to similar tags. So, see my new patch, now with <b> tag.

#8 @SergeyBiryukov
8 years ago

  • Keywords has-patch added

#9 @ocean90
8 years ago

  • Type changed from defect (bug) to enhancement

#10 @iandstewart
8 years ago

Agreed on standardizing to "Leave a reply" and sticking with <b> -- that <b> was intentional -- along with the /twentyeleven end of 18156.2.patch.

#11 @SergeyBiryukov
8 years ago

18156.3.patch combines 18156.2.patch and twenty_eleven_strings_2.patch, leaving out wp_get_attachment_link() change for now.

#12 @SergeyBiryukov
8 years ago

Opened #19282 for HTML support in wp_get_attachment_link().

#13 @westi
8 years ago

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

In [19375]:

TwentyEleven: Standardise on "Leave a reply". Fixes #18156 props SergeyBiryukov and pavelevap.

Note: See TracTickets for help on using tickets.