Make WordPress Core

Opened 6 years ago

Closed 5 weeks ago

#45473 closed defect (bug) (fixed)

Twenty Nineteen: edit translatable strings with HTML code

Reported by: presskopp's profile Presskopp Owned by: karmatosed's profile karmatosed
Milestone: 6.7 Priority: normal
Severity: normal Version:
Component: Bundled Theme Keywords: has-patch commit
Focuses: Cc:

Description

As always only the translatable part should appear for translators to avoid issues.

I came across this one:

<span class="meta-nav">Published in</span><br><span class="post-title">%title</span>

Attachments (9)

45473.patch (1.4 KB) - added by mukesh27 6 years ago.
Patch.
2019-JPEG-published-in.png (7.6 KB) - added by sabernhardt 17 months ago.
Twenty Nineteen: image attachment template gives a line break before post title
2019-PDF-published-in.png (7.5 KB) - added by sabernhardt 17 months ago.
Twenty Nineteen: non-image attachment page has no line break (and no space either) before post title
2021-JPEG-published-in.png (227.3 KB) - added by sabernhardt 17 months ago.
Twenty Twenty-One: image attachment template only links the post title, with the parent post text in the article's footer
2021-PDF-published-in.png (12.1 KB) - added by sabernhardt 17 months ago.
Twenty Twenty-One: non-image attachment page uses styling similar to the previous post link but without the SVG
45473.1.patch (768 bytes) - added by sabernhardt 17 months ago.
use the string with the line break for all attachment pages in Twenty Nineteen
45473.2.patch (768 bytes) - added by sabernhardt 7 weeks ago.
refreshed
2019-post-parent-links-before.png (33.5 KB) - added by sabernhardt 7 weeks ago.
Twenty Nineteen parent post links before patch: video attachment page does not have a line break (or space) before title
2019-post-parent-links-with-patch.png (33.8 KB) - added by sabernhardt 7 weeks ago.
Twenty Nineteen parent post links with 45473.patch: video and image attachment pages have line break before title, in both English and French

Download all attachments as: .zip

Change History (29)

#1 @SergeyBiryukov
6 years ago

  • Keywords needs-patch added
  • Milestone changed from Awaiting Review to 5.0.1

#3 @pento
6 years ago

  • Milestone changed from 5.0.1 to 5.0.2

@mukesh27
6 years ago

Patch.

#4 @mukesh27
6 years ago

  • Keywords has-patch added; needs-patch removed

This ticket was mentioned in Slack in #core-themes by kjell. View the logs.


6 years ago

#6 @pento
6 years ago

  • Milestone changed from 5.0.2 to 5.0.3

#7 @laurelfulford
6 years ago

  • Keywords 2nd-opinion added

Thanks for the report, @Presskopp!

It looks like there's been quite a bit of discussion about the best approach for this issue, shared in Twenty Nineteen GitHub repo, on top of https://github.com/WordPress/twentynineteen/issues/725 shared above:

@kjellr or @allancole -- I'm definitely not up to date where this ended up in Twenty Nineteen. Is there a particular direction that was decided on for HTML in strings marked for translation? From my read it sounds like it may make sense to keep the HTML in some cases, but I could be wrong!

#8 @laurelfulford
6 years ago

  • Milestone changed from 5.0.3 to 5.1

It seems like this one could be a bit too involved for 5.0.3, so punting to 5.1.

#9 @kjellr
6 years ago

From my read it sounds like it may make sense to keep the HTML in some cases, but I could be wrong!

That's my understanding here too, but I'd defer to the i18n experts.

Regarding @mukesh27's patch above, @grapplerulrich noted that this may not work because some languages may need to have this string rearranged to say "Title published in".

Last edited 6 years ago by kjellr (previous) (diff)

#10 @laurelfulford
6 years ago

  • Milestone changed from 5.1 to 5.2

Punting this issue for now, as we figure out exactly what strings need to be updated.

#11 @SergeyBiryukov
6 years ago

Just noting the string in question is not much different from similar "Published in" strings in Twenty Fourteen, Fifteen, and Sixteen, so the issue is not unique to Twenty Nineteen.

#12 @desrosj
5 years ago

This still needs work and has not been touched since being punted to 5.2. Moving to Future Release until someone is able to work on this.

#13 @desrosj
5 years ago

  • Milestone changed from 5.2 to Future Release

@sabernhardt
17 months ago

Twenty Nineteen: image attachment template gives a line break before post title

@sabernhardt
17 months ago

Twenty Nineteen: non-image attachment page has no line break (and no space either) before post title

@sabernhardt
17 months ago

Twenty Twenty-One: image attachment template only links the post title, with the parent post text in the article's footer

@sabernhardt
17 months ago

Twenty Twenty-One: non-image attachment page uses styling similar to the previous post link but without the SVG

@sabernhardt
17 months ago

use the string with the line break for all attachment pages in Twenty Nineteen

#14 @sabernhardt
17 months ago

HTML in a string is sometimes necessary for more accurate translations; [55655] specifically created two of them. Some languages add the title before "Published in" in at least one of the bundled themes. And Kurdish (Kurmanji) includes text outside of either span tag:
<span class="meta-nav">Di nav</span><br><span class="post-title">%title</span> de hate weşandin

However, I do not like having two different translations for the parent post link, depending on whether the attachment is an image or another type. Both Twenty Nineteen and Twenty Twenty-One have multiple translations, and the styling breaks, too. 45473.1.patch reuses the same string for non-image attachments in Twenty Nineteen, adding the br tag.

Using 'Published in %s' for non-image attachment pages in Twenty Twenty-One might require editing both the single.php template and twenty_twenty_one_entry_meta_footer(). That could be addressed on another ticket.

I do not find much value in changing the other three themes now. Twenty Fourteen is also inconsistent for different attachment types, though without broken styling. Twenty Fifteen and Twenty Sixteen have consistent links for any attachment type, and the strings are even identical between both of those themes.

#15 @karmatosed
8 weeks ago

@sabernhardt I wanted to check if you felt there was still value in changing this now?

@sabernhardt
7 weeks ago

refreshed

#16 @sabernhardt
7 weeks ago

  • Summary changed from Twenty Nineteen: Avoid html code in translatable strings to Twenty Nineteen: edit translatable strings with HTML code

I think it is still worth making Twenty Nineteen's "Published in" links consistent for any attachment page (but probably no other changes this late). The patch only replaces one string with another that already exists.

The %s might have been a better placeholder than %title if it had been used in both templates, yet more languages have translations for the %title string. Also, the theme's design added a line break in those links, and the %title string has that.

I edited the ticket summary because the patch does not follow the original direction. 45473.1.patch applied for me, but I refreshed it anyway.

@sabernhardt
7 weeks ago

Twenty Nineteen parent post links before patch: video attachment page does not have a line break (or space) before title

@sabernhardt
7 weeks ago

Twenty Nineteen parent post links with 45473.patch: video and image attachment pages have line break before title, in both English and French

#17 @karmatosed
6 weeks ago

  • Owner set to karmatosed
  • Status changed from new to assigned

#18 @karmatosed
6 weeks ago

  • Keywords 2nd-opinion removed

#19 @karmatosed
5 weeks ago

  • Keywords commit added
  • Milestone changed from Future Release to 6.7

I can confirm that the latest patch works and I agree with the approach in it. As a result, I am going to push this to commit so we can get progress on this ticket. Thanks everyone.

#20 @karmatosed
5 weeks ago

  • Resolution set to fixed
  • Status changed from assigned to closed

In 58881:

Twenty Nineteen: Fixes translatable strings with HTML code not appearing.

Only the translatable part not HTML, should appear for translators to avoid issues. This resolves one string that was not appearing. This only fixed for one theme although discussion on the ticket was for multiples. Other tickets should be open for those if desireable.

Props Presskopp, SergeyBiryukov, pratikkry, pento, mukesh27, laurelfulford, kjellr, desrosj, sabernhardt.
Fixes #45473.

Note: See TracTickets for help on using tickets.