Make WordPress Core

Opened 2 years ago

Closed 21 months ago

Last modified 21 months ago

#55941 closed defect (bug) (fixed)

Empty string wrongly translated to '0'

Reported by: chouby's profile Chouby Owned by: audrasjb's profile audrasjb
Milestone: 6.1 Priority: normal
Severity: normal Version: 2.8
Component: I18N Keywords: has-patch has-unit-tests
Focuses: Cc:

Description (last modified by sabernhardt)

I encountered a case where an empty string is translated to '0', when I expected it to be ''. This edge case is due to the translation file having the string '0', translated to '0'.

According to the PHP documentation https://www.php.net/manual/en/language.types.array.php, in array keys,

  • null is the same as ''
  • false and '0' are the same as 0

In Translation_Entry::key() we can find:

<?php
if ( null === $this->singular || '' === $this->singular ) {
        return false;
}

The singular string being the key of the entries array, that means that null, '', false, 0 and '0' all have the same translation.
I don't expect non-string keys to be used, but '' and '0' are two valid strings which should accept different translations.

Attachments (1)

Capture d’écran 2022-09-26 à 23.00.31.png (274.5 KB) - added by audrasjb 21 months ago.
Mistake solved :)

Download all attachments as: .zip

Change History (14)

#1 in reply to: ↑ description @Chouby
2 years ago

  • null is the same as
  • false and '0' are the same as 0

Sorry, I messed with formatting and don't know how to edit the ticket. This must be read:

  • null is the same as ''
  • false and '0' are the same as 0

#2 @audrasjb
2 years ago

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

Thanks chouby, the reported issue looks legitimate to me.
Definitely moving this for 6.1 consideration.

#3 follow-up: @audrasjb
2 years ago

@Chouby you can edit the ticket at the bottom of this page :)

Version 0, edited 2 years ago by audrasjb (next)

#4 @sabernhardt
2 years ago

  • Description modified (diff)

#5 in reply to: ↑ 3 @manooweb
2 years ago

Replying to audrasjb:

@Chouby you can edit the ticket at the bottom of this page, just use the "description" field :)<

Are you sure? I also didn't see this field on one of my tickets.

#6 @rafiahmedd
2 years ago

I think we can return just an empty string instead of false because false return a value 0 and that's why an empty string turns into 0 instead of ''.

This ticket was mentioned in PR #2939 on WordPress/wordpress-develop by Hug0-Drelon.


2 years ago
#7

  • Keywords has-patch has-unit-tests added; needs-patch removed

Trac ticket: https://core.trac.wordpress.org/ticket/55941

  • Do not return false when calling key() on an empty string translation entry.
  • Add test accordingly.

#8 @lopo
21 months ago

I'm reviewing this one as part of the Yoast Contributor Day activities.
The patch looks good to me and appears to be working as expected.

To test the fix I:

  • changed a string in the Settings page to '' (empty string)
  • switched my env to Italian
  • added the following to admin-it_IT.po and updated the .mo with msgfmt on the command line
    msgid "0"
    msgstr "0"
    
  • I could see the empty string was wrongly translated as 0
  • applying the patch the string is correctly translated to an empty string.

#9 @audrasjb
21 months ago

  • Owner set to audrasjb
  • Status changed from new to accepted

Thanks for the patch @lopo, it works fine on my side using your test scenario. Thanks for adding some unit tests, too.

Self-assigning for commit.

#10 @audrasjb
21 months ago

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

In 54315:

i18n: Ensure empty strings are consistently translated to ''.

This changeset fixes an edge case where empty strings were wrongly translated to '0' (falsey value) instead of '' (empty string).

Props Chouby, manooweb, rafiahmedd, lopo.
Fixes #55941.

#12 @audrasjb
21 months ago

I forgot @hugod in the props list, so I'm adding them with the manual Props tool located in the Make/Core website. Sorry for the inconvenience.

This ticket was mentioned in Slack in #core by sergey. View the logs.


21 months ago

Note: See TracTickets for help on using tickets.