search

mozilla / addons

☂ Umbrella repository for Mozilla Addons ✨
Other
125 stars 41 forks source link

[Bug]: addons-server JavaScript strings are no longer shown in the user's language #15048

Open diox opened 5 days ago

diox commented 5 days ago

What happened?

What did you expect to happen?

Instead we can see: [Select a file...] Your add-on should end with .zip, .xpi or .crx Screenshot 2024-09-25 at 13-02-35 Envoyer votre module Pôle développeurs Modules pour Firefox

Likely a regression from 2a64618e9d1eebaed15639fb4f824f2844c92778

Is there an existing issue for this?

┆Issue is synchronized with this Jira Task

diox commented 5 days ago

We can see that we're correctly requesting the JS file with the translations - on dev, currently that's https://addons-dev.allizom.org/static-server/js/i18n/fr.f4d6614c81ff.js - but that file doesn't contain our strings (only the base ones from Django), despite the fact that they are translated in djangojs.po: https://github.com/mozilla/addons-server/blob/master/locale/fr/LC_MESSAGES/djangojs.po

diox commented 5 days ago

In the image, the fr.js files in static-build/ and site-static/ are present but incorrect (missing our translations).

Locally manually calling generate_jsi18n_files (and then collectstatic for DEBUG=False) fixes it.

KevinMind commented 5 days ago

I think we are putting the files created by generate_jsi18n_files in ./static that is likely the problem as we no longer copy static into the final image. We should put those files in static-build as built assets.

diox commented 5 days ago

We aren't. There is nothing i18n related in static/, and the command does use root = os.path.join(settings.STATIC_BUILD_PATH, 'js', 'i18n').

Note that as I said above, the file is generated. It's just generated incorrectly, without our translations (likely a dependency/ordering/mount problem in the Dockerfile ?)

diox commented 3 days ago

This is caused by the fact that locales and assets are now built in parallel. The problem is, when we run generate_jsi18n_files in the assets stage, it's completely ignoring the locales stage, so our own translations are absent. Django's are present because they are already compiled.

Technically we could move things around, make generate_jsi18n_files part of locales stage, since it doesn't depend on anything assets related besides the output directory, and then move the collecstatic call to be after both assets and locales stages are done, towards the end of the build. I'm not sure we want to do this now, I'd rather remove some parallelism to fix the issue and worry about doing the full refactor later.

diox commented 9 hours ago

Note that we have a test for this, but it's passing in CI because we are forcing recompilation of assets before running the tests.