Make WordPress Core

Opened 5 years ago

Closed 4 years ago

#49058 closed defect (bug) (fixed)

Twenty Twenty: Comments report

Reported by: parvand's profile Parvand Owned by: ianbelanger's profile ianbelanger
Milestone: 5.5 Priority: normal
Severity: normal Version: 5.3.2
Component: Bundled Theme Keywords: good-first-bug has-patch has-screenshots
Focuses: template Cc:

Description

problem in comments.php
at line 35 where it starts:

} elseif ( '1' === $comments_number ) {

its not work for translate. I think it should be as follows:

} elseif ( '1' == $comments_number ) {

i change and it work now.

in twentynineteen its :

if ( '1' == $discussion->responses ) {

is that problem or not?

Attachments (9)

49058.diff (623 bytes) - added by benjamingosset 4 years ago.
Apply patch
before-patch.png (41.8 KB) - added by benjamingosset 4 years ago.
Before patch
after-patch.png (43.5 KB) - added by benjamingosset 4 years ago.
After patch
1-comment-2015.png (30.3 KB) - added by audrasjb 4 years ago.
Twenty Fifteen: translation correctly applied.
1-comment-2016.png (26.3 KB) - added by audrasjb 4 years ago.
Twenty Sixteen: translation correctly applied.
1-comment-2017.png (21.1 KB) - added by audrasjb 4 years ago.
Twenty Seventeen: translation correctly applied.
1-comment-2019.png (28.8 KB) - added by audrasjb 4 years ago.
Twenty Nineteen: translation correctly applied.
1-comment-2020.png (41.8 KB) - added by audrasjb 4 years ago.
Twenty Twenty: translation will never be applied.
49058.1.diff (508 bytes) - added by opurockey 4 years ago.
Apply patch

Download all attachments as: .zip

Change History (23)

#1 follow-up: @SergeyBiryukov
5 years ago

  • Keywords reporter-feedback added; needs-design-feedback removed

Hi there, welcome to WordPress Trac! Thanks for the report.

That condition is correct as is, since get_comments_number() returns a numeric string, not an integer. It's also the same in other bundled themes (except for Twenty Nineteen). See #38369 for more details.

Could you clarify what exactly is the problem with translations?

Last edited 5 years ago by SergeyBiryukov (previous) (diff)

#2 @desrosj
5 years ago

  • Milestone Awaiting Review deleted
  • Resolution set to worksforme
  • Status changed from new to closed

@Parvand I am going to close this out because the lines you mentioned are correct from a code standpoint.

If you are still experiencing issues, please reopen the ticket and add more details about the problem with translations so it can be troubleshooted further.

#3 @juliobox
4 years ago

  • Keywords needs-patch added
  • Resolution worksforme deleted
  • Status changed from closed to reopened

Hey there
The issue is still there, but here is the real bug :

Line 26 : $comments_number = absint( get_comments_number() );
Line 36 : } elseif ( '1' === $comments_number ) {

This is why this if can't be triggered.
The patch is : } elseif ( 1 === $comments_number ) {

Have a good day

#4 @Mista-Flo
4 years ago

  • Keywords good-first-bug added

Good catch, I agree with Julio.

@benjamingosset
4 years ago

Apply patch

@benjamingosset
4 years ago

Before patch

@benjamingosset
4 years ago

After patch

#5 @benjamingosset
4 years ago

Hey,

This is my first contribution, i applied the patch proposed by Julio.
I added screenshots before/after applying patch, it seems ok.

Could someone check if everything is ok ?

Thanks

#6 @SergeyBiryukov
4 years ago

  • Keywords has-patch added; reporter-feedback needs-patch removed
  • Milestone set to 5.5

#7 in reply to: ↑ 1 @elimuhub
4 years ago

Replying to SergeyBiryukov:

Hi there, welcome to WordPress Trac! Thanks for the report.

That condition is correct as is, since get_comments_number() returns a numeric string, not an integer. It's also the same in other bundled themes (except for Twenty Nineteen). See #38369 for more details.

Could you clarify what exactly is the problem with translations?

#8 @SergeyBiryukov
4 years ago

#50221 was marked as a duplicate.

@audrasjb
4 years ago

Twenty Fifteen: translation correctly applied.

@audrasjb
4 years ago

Twenty Sixteen: translation correctly applied.

@audrasjb
4 years ago

Twenty Seventeen: translation correctly applied.

@audrasjb
4 years ago

Twenty Nineteen: translation correctly applied.

@audrasjb
4 years ago

Twenty Twenty: translation will never be applied.

#9 @audrasjb
4 years ago

  • Keywords has-screenshots commit added
  • Owner set to audrasjb
  • Status changed from reopened to accepted

@SergeyBiryukov I think the previous commenters are right, Twenty Twenty will never apply this translation.
The current patch fixes the issue on my side.

Marking this ticket for commit.

#10 @audrasjb
4 years ago

PS: I tested all the other themes and they are all displaying the correct translation.

@opurockey
4 years ago

Apply patch

#11 @ianbelanger
4 years ago

  • Focuses coding-standards removed
  • Owner changed from audrasjb to ianbelanger
  • Status changed from accepted to assigned

Reviewing for commit.

#12 @ianbelanger
4 years ago

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

In 47941:

Bundled Themes: Twenty Twenty number of replies not translatable.

Fixes the issue by removing the single quote from around the 1 in the elseif conditional of the comments template.

Props Parvand, SergeyBiryukov, desrosj, juliobox, Mista-Flo, benjamingosset, audrasjb, opurockey.
Fixes #49058.

#13 @ianbelanger
4 years ago

  • Keywords fixed-major added; commit removed
  • Milestone changed from 5.5 to 5.4.3
  • Resolution fixed deleted
  • Status changed from closed to reopened

Reopening for backport.

#14 @desrosj
4 years ago

  • Keywords fixed-major removed
  • Milestone changed from 5.4.3 to 5.5
  • Resolution set to fixed
  • Status changed from reopened to closed

With 5.5 just under 5 weeks away, there is no 5.4.3 planned. Moving back to the 5.5 milestone.

Note: See TracTickets for help on using tickets.