WordPress.org

Make WordPress Core

Opened 7 weeks ago

Closed 11 days ago

#45441 closed defect (bug) (fixed)

wp_set_script_translations() does not support text domains other than "messages"

Reported by: swissspidy Owned by: ocean90
Milestone: 5.0.3 Priority: normal
Severity: normal Version: 5.0
Component: I18N Keywords: has-patch
Focuses: javascript Cc:

Description

I'm working on some WP-CLI tooling for the new JS I18N stuff we added in #45103.

While doing so I noticed that \WP_Scripts::print_translations() assumes that strings are in locale_data.messages. However, messages is just the default text domain of a Jed translation file.

Since Jed JSON files can contain translations for multiple text domains, one might want to combine all translations into one for easier maintenance. Especially with #45425.

See https://messageformat.github.io/Jed/ for examples of how these JSON files can look like.

Here's an example from my current WP-CLI script:

{
    "translation-revision-date": "2018-10-21 17:21+0200",
    "generator": "WP-CLI\/2.1.0-alpha-c44d019",
    "domain": "foo-plugin",
    "locale_data": {
        "foo-plugin": {
            "": {
                "domain": "foo-plugin",
                "lang": "de_DE",
                "plural-forms": "nplurals=2; plural=(n != 1);"
            },
            "Source": [
                "Quelle"
            ],
            "URL": [
                "URL"
            ],
            "block name\\u0004Recommendation": [
                "Empfehlung"
            ]
        }
    }
}

You'll get the same result when using the po2json npm script with the --domain argument.

IMO the inline JS added by \WP_Scripts::print_translations() should check whether locale_data.$text_domain exists and fall back to locale_data.messages otherwise.

Also can't hurt to esc_js() the variables there.

Attachments (2)

45441.diff (970 bytes) - added by swissspidy 3 weeks ago.
45441.2.diff (11.1 KB) - added by ocean90 13 days ago.

Download all attachments as: .zip

Change History (11)

#1 @swissspidy
6 weeks ago

  • Milestone changed from Awaiting Review to 5.0.1

#2 @pento
5 weeks ago

  • Milestone changed from 5.0.1 to 5.0.2

#3 @pento
5 weeks ago

  • Keywords needs-patch added
  • Milestone changed from 5.0.2 to 5.0.3

#4 @audrasjb
3 weeks ago

  • Milestone changed from 5.0.3 to 5.1

Hello,

5.0.3 is going to be released in a couple of weeks.

It doesn't appear this ticket can be handled in the next couple of weeks. Let's address it in 5.1 which is coming in February. Feel free to ask for changing the milestone if you think this issue can be quickly resolved.

Cheers,

Jb

#5 @swissspidy
3 weeks ago

  • Keywords has-patch added; needs-patch removed
  • Milestone changed from 5.1 to 5.0.3

I think this can land in 5.0.3 just fine. The patch is very simple.

@swissspidy
3 weeks ago

#6 @desrosj
13 days ago

Since this effects the new JS I18N, let's get this into 5.0.3. @swissspidy are you able to commit this before Monday?

@ocean90
13 days ago

#7 @ocean90
13 days ago

45441.2.diff updates the tests.

#8 @ocean90
11 days ago

In 44403:

I18N/Script Loader: Support text domains other than "messages".

The inline JavaScript added by WP_Scripts::print_translations() should check whether locale_data.$text_domain exists and fall back to locale_data.messages otherwise.

Props swissspidy.
See #45441.

#9 @ocean90
11 days ago

  • Owner set to ocean90
  • Resolution set to fixed
  • Status changed from new to closed

In 44404:

I18N/Script Loader: Support text domains other than "messages".

The inline JavaScript added by WP_Scripts::print_translations() should check whether locale_data.$text_domain exists and fall back to locale_data.messages otherwise.

Merge of [44403] to the 5.0 branch.

Props swissspidy.
Fixes #45441.

Note: See TracTickets for help on using tickets.