From f48fea81cf9a43e2c602a7dc476b750ccc02d8b0 Mon Sep 17 00:00:00 2001 From: Martin Pitt Date: Mon, 13 Jan 2025 15:58:03 +0100 Subject: [PATCH] bridge: Drop translations from manifests.js, introduce m-i18n.js Pages don't (shouldn't) care about manifest translations when including `manifests.js` -- they usually do that to query for a package's existence or some feature flags. Only the Shell cares about the translations. So drop translations from `manifests.js`, and support a new `manifests-i18n.js` which only the shell uses. This isolates translations from different pages from another. Concretely, subscription-manager-cockpit and our systemd page both translate "$0 important hit" differently, and even to different data types (in s-m it results in an array, in systemd in a normal string). This caused a page crash, and also wrong strings, as s-m's translations are really not expected to be used on the Overview page. This also speeds up the loading of frames, as they now don't have to go through umpteen `cockpit.locale()` calls any more. Note that this still leaves i18n conflicts in the shell itself, so this isn't a full fix, just an "80%" mitigation. Fixes #21486 --- pkg/shell/index.html | 2 +- src/cockpit/packages.py | 31 +++++++++++++++++-------------- test/pytest/test_packages.py | 2 +- 3 files changed, 19 insertions(+), 16 deletions(-) diff --git a/pkg/shell/index.html b/pkg/shell/index.html index 644a32a52723..895d7e5e0703 100644 --- a/pkg/shell/index.html +++ b/pkg/shell/index.html @@ -7,7 +7,7 @@ - + diff --git a/src/cockpit/packages.py b/src/cockpit/packages.py index 13fbe3e83967..3377ded69e0a 100644 --- a/src/cockpit/packages.py +++ b/src/cockpit/packages.py @@ -516,26 +516,27 @@ def reload_hint(self): self.reload() self.saw_first_reload_hint = True - def load_manifests_js(self, headers: JsonObject) -> Document: + def load_manifests_js(self, headers: JsonObject, *, i18n: bool) -> Document: logger.debug('Serving /manifests.js') chunks: List[bytes] = [] # Send the translations required for the manifest files, from each package - locales = parse_accept_language(get_str(headers, 'Accept-Language', '')) - for name, package in self.packages.items(): - if name in ['static', 'base1']: - continue + if i18n: + locales = parse_accept_language(get_str(headers, 'Accept-Language', '')) + for name, package in self.packages.items(): + if name in ['static', 'base1']: + continue - # find_translation will always find at least 'en' - translation = package.load_translation('po.manifest.js', locales) - with translation.data: - if translation.content_encoding == 'gzip': - data = gzip.decompress(translation.data.read()) - else: - data = translation.data.read() + # find_translation will always find at least 'en' + translation = package.load_translation('po.manifest.js', locales) + with translation.data: + if translation.content_encoding == 'gzip': + data = gzip.decompress(translation.data.read()) + else: + data = translation.data.read() - chunks.append(data) + chunks.append(data) chunks.append(b""" (function (root, data) { @@ -573,7 +574,9 @@ def load_path(self, path: str, headers: JsonObject) -> Document: if packagename is not None: return self.packages[packagename].load_path(filename, headers) elif filename == 'manifests.js': - return self.load_manifests_js(headers) + return self.load_manifests_js(headers, i18n=False) + elif filename == 'manifests-i18n.js': + return self.load_manifests_js(headers, i18n=True) elif filename == 'manifests.json': return self.load_manifests_json() else: diff --git a/test/pytest/test_packages.py b/test/pytest/test_packages.py index eaa8b6827dfb..e3aed57af9aa 100644 --- a/test/pytest/test_packages.py +++ b/test/pytest/test_packages.py @@ -233,7 +233,7 @@ def test_translation(pkgdir): assert document.data.read() == b'' # make sure the manifest translations get sent along with manifests.js - document = packages.load_path('/manifests.js', {'Accept-Language': 'de'}) + document = packages.load_path('/manifests-i18n.js', {'Accept-Language': 'de'}) contents = document.data.read() assert b'eins\n' in contents assert b'zwo\n' in contents