Skip to content

PD-5781 Fix peer review i18n in print view - #2888

Open
cryptalith wants to merge 1 commit into
mainfrom
lmendoa/PD-5781-print-view-peer-review-i18n
Open

PD-5781 Fix peer review i18n in print view#2888
cryptalith wants to merge 1 commit into
mainfrom
lmendoa/PD-5781-print-view-peer-review-i18n

Conversation

@cryptalith

Copy link
Copy Markdown
Member

PD-5781

Problem

The peer review heading is the only print-view string with placeholders, and it was broken in every locale — not falling back to English, but rendering truncated with both counts gone:

rendered
before Revisión por pares ($
after Revisión por pares (7 revisiones de 3 publicaciones/concesiones)

@angular/localize emitted no diagnostic, so this shipped silently.

Root cause

fetch-orcid.js is a plain asset, so ng extract-i18n never sees it. normalize-xlf.prebuild.ts parses the $localize calls out of the file instead, and its placeholder converter only matched UPPERCASE names:

rawSource.replace(/\$\{[^}]*\}:([A-Z0-9_]+):/g, '{$$$1}')

The source uses camelCase (:reviewCount:), so nothing matched and the raw ${reviewsCount}:reviewCount: text went verbatim into the XLF.

Fixing the name pattern alone would not have worked. Xliff1TranslationParser only recognises real <x id="..."/> elements, so the {$NAME} text form that converter aimed for parses as literal content, leaves the message with zero placeholders, and truncates the translation at the first $.

Changes

  • Emit real XLIFF <x id="..."/> placeholder elements. These are carried through the xml2js Builder as sentinels, since it escapes any markup found in a text node; the sentinels never reach disk.
  • Always regenerate printView.* units from fetch-orcid.js instead of only adding them when absent, so re-parsing a unit that already holds <x/> cannot degrade it. Verified idempotent across repeated runs.
  • Convert the locale targets in place, preserving the placeholder reordering translators applied in tr and zh-TW.
  • Add a build-time check: fails on leftover legacy syntax, warns when a target drops a placeholder (which inlines cleanly but silently drops the value).

<x/> is the standard XLIFF 1.2 form Angular already emits elsewhere, and Transifex already round-trips it here — 7 translated targets per locale contain it today. The previous raw-text form was the outlier.

Verification

  • All 16 translated locales render both counts, run through the real makeEs2015TranslatePlugin pass; uk/ca correctly fall back to English.
  • Full suite: 721 specs, 0 failures. I confirmed the peer review spec actually executes by temporarily breaking it.
  • messages.xlf is a one-line change — the usual regeneration churn was kept out.

Follow-ups (not in this PR)

  • Push to Transifex before merging so translations stay on the <x/> form; the new check fails the build if a pull brings the old syntax back.
  • Pre-existing: writeXlf's xml2js round-trip does not preserve mixed content, so the ~32 Angular-extracted units in messages.xlf have their <x/> elements moved to the end of each source on every build. Confirmed independent of this change (the unmodified script from main reproduces it). Tracked separately.

The peer review heading is the only print-view string with placeholders,
and it rendered truncated in every locale ("Revisión por pares ($") with
both counts missing and no build diagnostic.

normalize-xlf parses $localize calls out of fetch-orcid.js, since the file
is a plain asset that ng extract-i18n never sees. Its placeholder converter
only matched UPPERCASE names, but the source uses camelCase, so the raw
`${reviewsCount}:reviewCount:` text was written verbatim into the XLF.

Fixing the name pattern alone is not enough: Xliff1TranslationParser only
recognises real <x id="..."/> elements, so the `{$NAME}` text form that
converter aimed for parses as literal content, leaves the message with zero
placeholders and truncates the translation at the first `$`. Emit proper
XLIFF placeholder elements instead, carried through the xml2js Builder as
sentinels because it escapes markup found in a text node.

printView.* units are now always regenerated from fetch-orcid.js rather than
only added when absent, so re-parsing a unit that already holds <x/> cannot
degrade it, and add a build-time check that fails on leftover legacy syntax
and warns when a target drops a placeholder.

Locale targets are converted in place, preserving the reordering translators
applied in tr and zh-TW.
@amontenegro

Copy link
Copy Markdown
Member

Task linked: PD-5781 Fix Peer Review i18n in Print View

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants