Make WordPress Core

Opened 10 months ago

Closed 9 months ago

Last modified 9 months ago

#63269 closed defect (bug) (fixed)

Duplicate array key `Code` in `_WP_Editors::get_translation()`

Reported by: justlevine's profile justlevine Owned by: joedolson's profile joedolson
Milestone: 6.8.1 Priority: normal
Severity: normal Version:
Component: Editor Keywords: has-patch dev-reviewed commit
Focuses: Cc:

Description

This is a bug introduced in #38061, where the new Code key added to _WP_Editors::get_translation() on Line 1399 overwrites the existing Code registered on Line 1168.

 Line    wp-includes/class-wp-editor.php                                 
 ------- ---------------------------------------------------------------- 
  :1168   Array has 2 duplicate keys with value 'Code' ('Code', 'Code').  
          🪪  array.duplicateKey       

---

Caught as part of #61175 (PHPStan baseline)

Attachments (4)

63269.diff (1.6 KB) - added by sabernhardt 10 months ago.
restoring 'Text' array key
63269.2.diff (1.6 KB) - added by joedolson 9 months ago.
Alternate with Code|tab
TinyMCE-Code-button.png (33.9 KB) - added by sabernhardt 9 months ago.
Before patch: TinyMCE Code button with untranslated "Code" tooltip
advanced-editor-trunk-vi.png (15.9 KB) - added by sabernhardt 9 months ago.
in trunk, the two different Vietnamese strings display correctly: "Mã" for the Code tab, and "Mã code" plus the keyboard shortcut for the TinyMCE Code button tooltip

Download all attachments as: .zip

Change History (24)

#1 @joedolson
10 months ago

  • Milestone changed from Awaiting Review to 6.8
  • Owner set to joedolson
  • Status changed from new to accepted

#2 @sabernhardt
10 months ago

  • Component changed from General to Editor
  • Keywords needs-patch added

The simplest fix might be to restore 'Text' as the array key (but leaving the translation).

  • base.js : text( window.tinymce.translate( 'Text' ) )
  • class-wp-editor.php : 'Text' => _x( 'Code', 'Name for the Code editor tab (formerly Text)' ),

One way to test the script is with a Text widget (using the Classic Widgets plugin).

@sabernhardt
10 months ago

restoring 'Text' array key

#3 @sabernhardt
10 months ago

  • Keywords has-patch added; needs-patch removed

#4 @joedolson
9 months ago

I think we should follow the same model used elsewhere in the array for distinguishing between common keys, e.g. Edit vs. Edit|button. Perhaps Code|editor?

@joedolson
9 months ago

Alternate with Code|tab

#5 @joedolson
9 months ago

Added an alternate patch that uses Code|tab, which better describes the use case.

#6 @audrasjb
9 months ago

I think the alternate patch looks good to me, but I'm wondering wether we should fix this in 6.8 or in 6.8.1. What is the potential issue for end users: user facing strings conflicts?

This ticket was mentioned in Slack in #core-i18n by justlevine. View the logs.


9 months ago

#8 @swissspidy
9 months ago

From what I can see, most if not all locales translate the two strings the same anyway, so it doesn't really matter in that regard. This can be fixed in 6.8.1 or even 6.9

#9 @audrasjb
9 months ago

  • Milestone changed from 6.8 to 6.8.1

Let's move this ticket to milestone 6.8.1 then.

@sabernhardt
9 months ago

Before patch: TinyMCE Code button with untranslated "Code" tooltip

#10 follow-up: @sabernhardt
9 months ago

I found the first 'Code' string in the TinyMCE Code button. Core does not activate that in a Classic block or with Classic Editor, but it can be added with a plugin such as Advanced Editor Tools.

  • If the newer string is not translated in the user's language, the tooltip shows "Code" (in English).
  • Now the tooltip does not give the keyboard shortcut in any language.

5 out of 38 translations do not match the previous "Code" string perfectly, but the differences seem minor (to someone who cannot read them).

The "Text" patch could have one small advantage: if someone replaced the old string using the wp_mce_translation filter, the replacement would continue to work. However, I did not find any directory plugins that edited the "Text" tab, and most plugins in the search results just add strings for their own elements (very few replace a string).

#11 in reply to: ↑ 10 @sabernhardt
9 months ago

The "Text" patch could have one small advantage...

If a site replaced "Text" with the wp_mce_translation filter, updating to 6.8 would already change the key and string. That decreases any value that restoring "Text" might have had.

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


9 months ago

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


9 months ago

#15 @jorbin
9 months ago

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

In 60182:

Editor: Use different keys in array of translatable strings.

[59696] changed the 'Text' tab of the classic editor to 'Code' but Code was already used as a key in the array of translatable text. Since arrays keys need to be unique, this meant that it is possible for the wrong translation to appear in a locale. Using different keys fixes that.

Props joedolson, sabernhardt, justlevine, swissspidy, audrasjb.
Fixes #63269. See #38061.

#16 @jorbin
9 months ago

  • Keywords dev-feedback fixed-major added; commit removed
  • Resolution fixed deleted
  • Status changed from closed to reopened

Reopening for 2nd committer review and backport consideration

@sabernhardt
9 months ago

in trunk, the two different Vietnamese strings display correctly: "Mã" for the Code tab, and "Mã code" plus the keyboard shortcut for the TinyMCE Code button tooltip

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


9 months ago

#18 @joedolson
9 months ago

  • Keywords dev-reviewed commit added; dev-feedback removed

Looks good for backport.

#19 @joedolson
9 months ago

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

In 60187:

Editor: Use different keys in array of translatable strings.

[59696] changed the 'Text' tab of the classic editor to 'Code' but Code was already used as a key in the array of translatable text. Since arrays keys need to be unique, this meant that it is possible for the wrong translation to appear in a locale. Using different keys fixes that.

Reviewed by joedolson.
Merges [60182] to the 6.8 branch.

Props joedolson, sabernhardt, justlevine, swissspidy, audrasjb.
Fixes #63269. See #38061.

#20 @joedolson
9 months ago

  • Keywords fixed-major removed
Note: See TracTickets for help on using tickets.