Problem/Motivation

Core outputs JS translations for strings that LocaleJs::parseJsFile() finds in the JavaScript files of the pages, with their contexts. While porting potx to a modern architecture two bugs were found in the matching code there, so transporting here too.

Two forms of Drupal.t() and Drupal.formatPlural() calls, both in core's own JavaScript, are not found, or are found without their context, so the browser does not get their translations, even when the translations exist:

  1. Template literals without substitutions. The detected expression of the strings has single and double quotes only currently. Calls with string literals are not found, and their strings are not in the JavaScript translations. These are direct examples from core that are not found:
    • Drupal.t(`The following files could not be loaded: @dependencies`, { '@dependencies': … }) in misc/ajax.js and misc/htmx/htmx-utils.js
    • Drupal.t(`@listType button @label`, { … }) in modules/ckeditor5/js/ckeditor5.admin.js
  2. null in place of the arguments leads to lack of context recorded. An object is expected for the arguments, so the match ends at the comma after the string, and the string is registered without the context of the options. Later Drupal.t() looks up the translation in the context of the call, which is not in the JavaScript translations. Both calls are in modules/ckeditor5/js/ckeditor5.admin.js:
    Drupal.t(
      "The toolbar buttons that don't fit the user's browser window width will be grouped in a dropdown. …",
      null,
      {
        context:
          'CKEditor 5 toolbar help text, default, no explicit wrapping breakpoint',
      },
    )

    and the same with the context "CKEditor 5 toolbar help text, with explicit wrapping breakpoint".

The extractor of localize.drupal.org, potx, has the same fixes in #3630552: JavaScript strings potx does not find: trailing commas, null arguments, template literals (which includes a trailing comma fix core already has). But without this issue, the translations are not used by actual Drupal sites.

Proposed resolution

  • The updated expression of the strings should match template literals without ${, and stripQuotes() removes their backticks, like the quotes of the other strings. A template literal with substitutions is not a literal string, and it is not matched.
  • Make it possible to have the arguments of Drupal.t() and Drupal.formatPlural() be null.
  • Update LocaleJavascriptTranslationTest: add calls to locale_test.js with null arguments and template literals, and a template literal with a substitution, which should still not be found.

Remaining tasks

Review.

User interface changes

None.

Introduced terminology

None.

API changes

None.

Data model changes

None.

Release notes snippet

JavaScript translations now include strings in backticks (in case they do not have ${…} placeholders), like Drupal.t(`Text`). They also include the context of calls with null in place of the arguments, like Drupal.t('Text', null, {context: 'Context'}). Before, these strings were not translated in the browser, even when their translations existed. After a core update, as the caches are cleared automatically, Drupal parses the JavaScript files again and adds these strings.

LLM disclosure

LLM was used to find, diagnose, explain and fix this issue. With human review.

Issue fork drupal-3630560

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

gábor hojtsy created an issue. See original summary.

gábor hojtsy’s picture

Status: Active » Needs review

nicxvan’s picture

Issue tags: +Needs security review

I think any changes to the regex need careful review.

nod_’s picture

We might be at the point where regex are not the right solution anymore. We have a real js parser in core with mck89/peast, there are performance considerations on first load since the parser is way heavier than a regex but at least we woudln't need to worry about JS syntax at all.
Currently core and this regex catch a code example in a comment (announce.js, line 15-17) not a big deal but a parser-based solution wouldn't have that problem.

It needs code around it but the core of the parser would be something like this

Query($ast))->find('CallExpression[callee.computed=false][callee.object.name="Drupal"][callee.property.name=/^(t|formatPlural)$/]');