Issue #3620615: Strings that cannot be translated, and the check that misses them

Fixes the eight findings of #3620615, one commit each.

The check first

The high-severity finding is the tooling, so it is the first commit and it is what found the rest. scripts/check-translations.php reported two directions, and its misfiled half — "an entry in the catalog whose text appears in none of the project's own files" — was a str_contains() over every one of a project's files concatenated and whitespace-flattened. So a short entry matched an identifier or a fragment of a longer sentence and passed: Script inside "JavaScript", Palette inside getPaletteLabel(), Default inside DefaultPluginManager, Shared inside a docblock, and Hold and "Access the Orchestra HTTP API" inside the longer strings that had replaced them. Thirty entries were hiding behind it, not five.

The haystack is now whole strings, from the two places a project's strings come from: what potx extracted, and the labels its shipped config carries, read through Drupal's own YAML decoder. That also settles the apostrophe-doubling the text search had to guess at, and reaches a config value that is itself a YAML document, which a webform's elements is.

Two more readers were wrong the same quiet way, and both are fixed: the catalog reader took only the first line of a msgid, so a catalog re-wrapped by msgcat, Poedit or a download from localize.drupal.org would have read as entirely missing and entirely exempt at once; and a translation call whose whole content is a placeholder is now a finding of its own rather than an allow-list hit.

The reading moves to scripts/check-translations.inc, because it is the half that can be wrong without saying so, and a unit test asserts each of the three questions it answers, including the two shapes that made a wrong answer look right.

The rest

  • The status palette's five names could not be translated at all. Plain strings in a class constant, so no catalog could carry them; the select had reached for t('@name', ...), which translates nothing. Now Status::getPaletteOptions() returning translatable markup, a method because PHP admits new in an initializer everywhere except a class constant. Info was missing from the catalog and is added.
  • A work item's unknown state was wrapped in t('@state'). It now reads as its machine value, which is how ProcessInstance::getStateLabels() has always been read by its callers.
  • Twenty-six catalog entries no file of their project carries. Nine are entries whose string is nowhere in the repository, five of those the stale prefixes of the sentences that replaced them. The rest were filed under a project that cannot use them, since Drupal imports a module's catalog only when that module is installed. Cancel moves rather than goes: the payment example ships it with no French of its own.
  • Five count messages said "instance(s)". Now formatPlural, which the eight sibling messages in these same three classes already used. Two can be reached with a count of zero, so their French singular carries @count: French uses the singular for zero.
  • An account-less recipient was always mailed in English. The channel asked each recipient for its language with the literal 'en', and an account-less one has none to give, so every external partner address and every address held in a process variable was written to in English whatever language the site runs in — against what the README promises. The fallback is now the site's default language, which is what an account-backed recipient already gets.
  • The example workflows were only half translated. example_review, alone among eleven, had no French file; no example translated an action label or an outcome label anywhere; and seven flow labels had no French by any route, including two that appear nowhere else. example_approval's French name had also drifted from the English.
  • The Easy Email templates had French subjects over English bodies. The bodies ship as language overrides carrying only bodyHtml and bodyPlain; a subject belongs in the catalog, a twenty-five line HTML document does not.

Tests

Every behaviour change has one, and each was seen to fail on the tree before its commit:

  • TranslationCheckTest (unit): the three questions the check asks.
  • StatusPaletteTest (kernel): the five names are markup, a theme's class is not.
  • WorkItemPresentationTest: a state Orchestra does not name.
  • NotificationMailChannelTest: the language an account-less recipient is written to in, and that a langcode supplied with the address still wins.
  • ExampleTranslationsTest (unit): every shipped example label has French by one of the two routes; it named all thirty-one gaps on the old tree.
  • TemplateTranslationsTest (unit): both halves of a template, and that a French body differs from the English and keeps its markup.
  • OrchestraUiTest: the plural form of the delete refusal.

Left for a follow-up

The missing direction still does not cover shipped config: potx is not given config/install or config/optional, so nothing requires French for a label only a config entity carries. That is why the Easy Email bodies and example_review went unreported for four audits, and it still holds for orchestra_views, orchestra_inbox_views, orchestra_payment_example and orchestra_interaction_webform_examples, which ship view and workflow labels with no French. potx CAN extract them (potx_finish_processing() plus the config directories), but it needs module discovery to resolve a dependency's schema, which depends on where the checkout sits relative to a Drupal root, and it would also demand entries for the view chrome core already translates (Reset, Sort by, Asc). Deciding that is its own issue rather than a side effect of this one.

Also noticed, unfixed here: Completed and Canceled have four different French strings across four catalogs, and locale merges every imported catalog into one table, so the last import wins.

What a reviewer should look at first

WorkItemInterface::getStateLabel() is an @api signature change: TranslatableMarkup becomes string|TranslatableMarkup, and docs/extending.md lists that interface as public. An implementor that declared the narrower type stays valid (a child may narrow), and a caller that type-hinted the result does not; nothing in the docroot's contrib tree calls it. StatusInterface::getPaletteLabel() widens the same way but carries no @api, and Status::PALETTE, a public constant, is gone in favour of Status::getPaletteOptions(). Pre-1.0, so the better shape wins, but they are the two lines here that break a signature.

Audit of this branch

Read back before merging, and the last commit is what that found: seven things, including the one that matters most — the language finding's root cause was an undocumented @api parameter, which is why both shipped channels passed 'en' independently. RecipientInterface::getLangcode() now says what to pass.

Checked and clean:

  • Duplication. scripts/claude/find-duplicate-bodies.php reports the same 53 groups on this branch as on 1.x, so no method body is copied here. By hand: one .po reader in the repository (orchestra_catalog(), reused by the three tests that need one rather than reimplemented), one whitespace-flatten, no pre-existing helper that either new test should have called. The two shipped-config tests share two lines and nothing else worth a seam; a common trait would take a config bag and read worse at both call sites.
  • Performance. The check is 0.10s slower and 6MB lighter than the substring version (0.58s to 0.68s, five runs each), because it parses 61 shipped config files instead of concatenating every file of every project and running four str_contains() variants of every catalog entry over the result. Status::getPaletteOptions() builds five TranslatableMarkup per call, and getPaletteLabel() calls it per list row — which is exactly what ProcessInstance::getStateLabels() already does per row in two Views fields, so it is the shape already in use on a busier path, not a new cost.
  • Vacuity. Both shipped-config tests could have passed by finding nothing to check; each now asserts what it covered.
  • No hard-coded langcode is left in any non-test file.
  • Every catalog passes msgfmt --check; no fuzzy, obsolete or empty-msgstr entry anywhere; no dead entry in translations/untranslated.txt.

Two limitations of the check worth knowing, neither reached today: it counts a catalog entry as present without looking at its msgstr, so an empty or fuzzy translation would read as translated; and reading a .po for msgids only is what keeps that reader simple enough to be worth trusting.

Second audit: two commits did not work, and now do

The first audit was static. The second built a real French site (a copy-on-write clone of a docroot, host MySQL, drush si minimal, French added before the modules) and read the configuration back. Two commits were delivering nothing.

locale owns every config key the schema marks translatable. LocaleConfigManager::filterOverride() drops those keys from whatever a module ships under config/install/language/<langcode>/ and merges back only what the interface translation supplies. So a shipped override for a translatable key is discarded on any site with locale installed, which is any site that wants French. Measured on that site: 47 of the 90 strings those files carry never reached the configuration, every example workflow's own label among them.

The four Easy Email French bodies were discarded entirely — the stored language.fr override held only label, subject, inboxPreview, and a French site read the English body. Exactly the defect that commit claimed to fix.

The fix is the catalog, which is the route that arrives: the 51 example strings that had no entry now have one, their French carried across from the override files rather than translated anew. Measured again on the same site: 101 of the 106 shipped labels read French. Four of the five left are the ECA models' own label, which eca's eca.eca.* schema does not mark translatable, so no module can translate it — worth an eca issue — and the fifth is "SMS", whose French is "SMS".

The HTML email body cannot be translated by either route, so it now ships English and says so. The override route is discarded as above; the catalog route refuses it, because every imported translation passes through locale_string_is_safe(), which rejects anything Xss::filter() would change. Drupal logged all four bodies itself: "skipped because of disallowed or malformed HTML". A test pins the absence so nobody ships one again believing it works, and the README states the constraint and what a site does instead.

Both tests had asserted the wrong contract — they accepted a label as translated when an override file carried it. They require the catalog now, scoped the way locale reads it: one string table per site, so a module may rely on its own catalog plus its dependencies', which is how every example reads "Start" from the base module.

The override files themselves stay: a site running language without locale has no interface translation for locale to prefer, and that shape was not measured.

Also verified in this pass, on the branch rather than by argument: the old check is silent on an injected stale-prefix orphan and on an entry that only matches an identifier, while the new one names both; against a real msgcat-rewrapped catalog the old check reports 20 false missing strings and the new one is clean; a placeholder-only call is reported even when explicitly allow-listed; and the extracted-string set differs from 1.x by exactly the intended ten additions and six removals, so nothing stopped being extracted by accident.

Third audit: a French plural stated a count of one at a count of zero

Rendered rather than reasoned about, at 0, 1, 2 and 3 on a French-default site. French takes the singular form for zero, so the French singular is what a count of zero reads, and two of the five converted messages said "1" there: "Migrated 0 instances" came out as "1 instance migrée vers la version 3."

Zero is reachable for that one: the migrate form is only offered when the preview finds work, but migrate() re-queries the migratable instances at submit time, so an instance that finished in between leaves an empty plan and returns 0. Both messages now carry @count in the singular and are true at every count: "0 instance migrée vers la version 3." The other three were already written that way and rendered correctly.

Also verified in this pass, on a real site or by measurement:

  • The status palette renders in French — Neutre, Info, Succès, Avertissement, Danger — which is what the second commit is for.
  • No escaping regression from returning an unknown work-item state as a plain string instead of markup. Rendered through the shared status tag with a hostile value: &lt;script&gt;alert(1)&lt;/script&gt;. The template prints {{ label }} with no |raw, so Twig escapes a plain string exactly as the placeholder used to.
  • No new conflicting translations. Msgids carried by two or more catalogs with disagreeing French: 24 on 1.x, 22 on this branch, none of them new — the catalog cleanup removed the Process and Running conflicts. The 22 that remain predate this issue (Completed has four different French strings in four catalogs) and are worth their own issue, since locale keeps one string table and the last import wins.
  • Every catalog rewrapped, not just one. With all 34 in gettext's normal wrapped form (411 wrapped entries), the old check reports 373 false missing strings and the new one is clean. The project could not have accepted a catalog downloaded from localize.drupal.org.

Fourth audit: nothing new

The first three each found something. This one exercised the rendered output and the delivered mail, and found no defect.

  • The palette select, rendered as HTML in French, which is the headline fix's actual code path and had only ever been checked through the API: six options, - Par défaut -, Neutre, Info, Succès, Avertissement, Danger, the current value marked selected, and no array-to-string anywhere.
  • The status list, rendered in French, covering all three branches of getPaletteLabel() in one table: a palette class shows its French name (Info), a class a theme added shows the raw token (is-brand), the neutral default shows Par défaut.
  • The account-less recipient fix, with a real send, before and after on the same French-default site. The 1.x subscriber: langcode en, "A task is waiting for you". This branch: langcode fr, "Une tâche vous attend". Same site, same recipient, only the subscriber swapped.
  • Easy Email in its final state, with the language directory deleted, which no earlier pass had installed: the module installs clean and a French site reads a French name, subject and plain-text body with the HTML body in English, which is the documented constraint.
  • The script's own promise that it runs "from the module root, or from anywhere": verified from an unrelated directory and from /.

One mechanism nuance worth recording, since it decides which measurement applies to which site. On a site whose default language is French, locale writes the translation into the active configuration (saveTranslationActive()) and there is no override at all; on an English-default site with French added it writes a language.fr override. The 47-of-90 loss measured in the second audit was the override shape, which is the one where a shipped override file would have mattered. Both shapes read French now.

This merge request was prepared with the assistance of an AI agent (Claude). The analysis and the wording were reviewed by me before posting, and accountability for the content is mine.

Edited by Frank Mably

Merge request reports

Loading
Loading