WordPress.org

Make WordPress Core

Opened 3 years ago

Closed 2 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:

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 3 years ago.
Different_arrows.png (3.9 KB) - added by pavelevap 3 years ago.
18156.patch (1007 bytes) - added by SergeyBiryukov 3 years ago.
18156.2.patch (2.5 KB) - added by SergeyBiryukov 3 years ago.
twenty_eleven_strings_2.patch (924 bytes) - added by pavelevap 3 years ago.
18156.3.patch (1.9 KB) - added by SergeyBiryukov 2 years ago.

Download all attachments as: .zip

Change History (19)

comment:1 nacin3 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.

comment:2 pavelevap3 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.

pavelevap3 years ago

comment:3 follow-ups: nacin3 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.

comment:4 in reply to: ↑ 3 ; follow-up: johnbillion3 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.

comment:5 in reply to: ↑ 4 nacin3 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.

SergeyBiryukov3 years ago

SergeyBiryukov3 years ago

comment:6 in reply to: ↑ 3 SergeyBiryukov3 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.

comment:7 pavelevap3 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.

comment:8 SergeyBiryukov3 years ago

  • Keywords has-patch added

comment:9 ocean902 years ago

  • Type changed from defect (bug) to enhancement

comment:10 iandstewart2 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.

SergeyBiryukov2 years ago

comment:11 SergeyBiryukov2 years ago

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

comment:12 SergeyBiryukov2 years ago

Opened #19282 for HTML support in wp_get_attachment_link().

comment:13 westi2 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.