Skip to content

[2.x] fix: resolve translation references in implicitly-loaded fallback catalogues - #5023

Merged
imorland merged 1 commit into
flarum:2.xfrom
gianniguida:gg/fix-translator-fallback-catalogue-parse
Sep 2, 2026
Merged

[2.x] fix: resolve translation references in implicitly-loaded fallback catalogues#5023
imorland merged 1 commit into
flarum:2.xfrom
gianniguida:gg/fix-translator-fallback-catalogue-parse

Conversation

@gianniguida

Copy link
Copy Markdown
Contributor

Fixes unresolved translation references leaking into compiled locale JS assets.

Symptom

Compiled locale assets (e.g. admin-en.js) sometimes contain unresolved translation references — literal values like:

"core.admin.nav.dashboard_button":"=> core.admin.dashboard.title"

The admin UI then displays raw => core.admin.… strings. Observed in production on Flarum 1.8.19 (community.sbb.ch); the affected logic is identical on 2.x and 1.x. Characteristics:

  • Only ever affects the fallback locale (en, hardcoded via setFallbackLocales(['en'])).
  • Plain (non-reference) entries in the same file are correct.
  • Intermittent: only manifests when a locale asset is (re)compiled by an "unlucky" request — the poisoned asset then persists (its revision exists, so it is never recompiled) until a cache flush.

Root cause

Flarum\Locale\Translator::getCatalogue() decided whether to run =>-reference resolution (parseCatalogue()) with a locale-keyed flag:

$parse = ! isset($this->catalogues[$locale]);

But Symfony's Translator::loadFallbackCatalogues() (same pattern verified on symfony/translation v5.4 and v7.4), when loading any other locale, pre-stores the original fallback catalogue in $this->catalogues['en'] while attaching only a fresh copy to the requesting catalogue:

if (!isset($this->catalogues[$fallback])) {
    $this->initializeCatalogue($fallback);          // stores the ORIGINAL — raw
}
$fallbackCatalogue = new MessageCatalogue($fallback, $this->getAllMessages($this->catalogues[$fallback]));
$current->addFallbackCatalogue($fallbackCatalogue); // attaches a COPY

So loading de first stores a raw, never-parsed en original. Flarum's parse-walk resolves de and the copy — the stored original stays raw. A later getCatalogue('en') finds the locale already set, skips parsing, and returns the raw original. Any consumer that snapshots the catalogue — most damagingly Flarum\Frontend\AddTranslations, which compiles the locale JS assets — emits the literal => … strings.

Trigger shape in production: any request that touches a non-en catalogue before compiling an en asset (forums whose default locale is not en, or cross-locale rendering such as notification emails).

Reproduction

Deterministic 12-line script (run from the monorepo root after composer install), mirroring LocaleServiceProvider's wiring — full script in the test file added by this PR; essence:

$t = makeTranslator();          // locale 'de', fallback ['en'], non-debug, cache dir
$t->getCatalogue('en');
$t->getCatalogue('en')->get('core.admin.nav.dashboard_button');
// => "Dashboard"                                       ✔

$t = makeTranslator();          // fresh translator, fresh cache
$t->getCatalogue('de');         // implicit fallback load of 'en'
$t->getCatalogue('en')->get('core.admin.nav.dashboard_button');
// => "=> core.admin.dashboard.title"                   ✘

Also confirmed end-to-end on a live Flarum 1.8.19 install: a script that calls getCatalogue('de') before force-recompiling the English admin locale asset produces literal => … strings in the real admin UI; the identical script minus that one line produces a clean asset.

Fix

Track parsed state per catalogue object (WeakMap) instead of per locale name, and walk the fallback chain on every getCatalogue() call. The raw stored original and its parsed copy share a locale name, so a locale-keyed flag cannot distinguish them; per-object tracking parses each catalogue object exactly once. Already-parsed objects cost one WeakMap lookup per call. parseCatalogue() is unchanged (and idempotent, so even a double parse would be harmless).

Note: reference resolution remains in-memory only, by design — the on-disk catalogue cache files always contain raw references. The fix guarantees the in-memory parse for every object handed out.

Tests

New TranslatorReferenceResolutionTest, wired like LocaleServiceProvider (non-debug, cache dir, en fallback):

  • en loaded directly → references resolved (regression guard, passed before).
  • en first loaded implicitly as a fallback of de, then requested directly → references resolved (failed before this fix with exactly '=> bar' vs 'Resolved').
  • The fallback copy attached to de is resolved, both via getFallbackCatalogue() and via trans() fallthrough (regression guard).

Full unit suite and PHPStan pass.

Context

Surfaced while working on FriendsOfFlarum/redis#34 — multi-instance cache invalidation increases recompile frequency, raising exposure — but the bug is reproducible on a bare single-instance install. A 1.x backport PR (PHP 7.3-compatible, SplObjectStorage instead of WeakMap) follows.

🤖 Generated with Claude Code

…alogues

Flarum's Translator::getCatalogue() only ran `=>`-reference resolution
(parseCatalogue()) when the requested locale was not yet present in
$this->catalogues. But Symfony's Translator::loadFallbackCatalogues()
pre-stores the *original* fallback catalogue in $this->catalogues while
attaching only a fresh copy to the requesting locale's catalogue.

So loading e.g. 'de' first stored a raw, never-parsed 'en' original;
a later getCatalogue('en') found the locale already set, skipped
parsing, and returned it with unresolved references. Consumers that
snapshot the catalogue - most damagingly Frontend\AddTranslations,
which compiles the locale JS assets - then emitted literal
"=> core.admin.dashboard.title" strings, and the poisoned compiled
asset persisted until a cache flush (revisions gate recompiles).

Track parsed state per catalogue *object* (WeakMap) instead of per
locale name, and walk the fallback chain on every access. The raw
stored original and its parsed copy share a locale name, so a
locale-keyed flag cannot distinguish them; per-object tracking parses
each catalogue object exactly once. parseCatalogue() is unchanged.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@gianniguida
gianniguida requested a review from a team as a code owner September 2, 2026 07:09
@imorland imorland added this to the 2.0-pre milestone Sep 2, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug: Switching locale via setLocale() wipes out other language translations, causing raw translation keys to display

2 participants