Copilot commented on code in PR #12863:
URL: https://github.com/apache/cloudstack/pull/12863#discussion_r2964615841
##########
ui/src/utils/guiTheme.js:
##########
@@ -85,6 +87,9 @@ async function applyDynamicCustomization (response) {
vueProps.$config.favicon = jsonConfig?.favicon ?? vueProps.$config.favicon
vueProps.$config.css = response?.css ?? null
+ vueProps.$localStorage.set('LOCALE', vueProps.$config.defaultLanguage)
+ loadLanguageAsync(vueProps.$config.defaultLanguage)
Review Comment:
`loadLanguageAsync(...)` returns a Promise but is not awaited/handled here.
Since `main.js` calls `loadLanguageAsync()` right after `applyCustomGuiTheme`,
this can trigger duplicate concurrent locale fetches and a race on
`loadedLanguage`/`setLocaleMessage`. Either await this Promise (and handle
errors) or remove this call and rely on the `main.js` initialization load after
setting `LOCALE`.
```suggestion
```
##########
ui/src/components/header/TranslationMenu.vue:
##########
@@ -63,7 +64,7 @@ export default {
}
},
mounted () {
- this.language = this.$localStorage.get('LOCALE') || 'en'
+ this.language = this.$localStorage.get('LOCALE') ||
vueProps.$config?.defaultLanguage || 'en'
this.setLocale(this.language)
Review Comment:
`this.$localStorage.get('LOCALE')` may not always be a string (the locale
loader guards against `typeof locale === 'object'`). If it’s an object, this
code will pass a non-string into `setLocale`, which then calls
`moment.locale(localeValue)` and can misbehave. Consider normalizing/validating
the stored LOCALE before using it, and falling back to `defaultLanguage`/`en`
when invalid.
##########
ui/src/utils/guiTheme.js:
##########
@@ -69,6 +70,7 @@ async function applyDynamicCustomization (response) {
vueProps.$config.logo = jsonConfig?.logo ?? vueProps.$config.logo
vueProps.$config.minilogo = jsonConfig?.minilogo ?? vueProps.$config.minilogo
vueProps.$config.banner = jsonConfig?.banner ?? vueProps.$config.banner
+ vueProps.$config.defaultLanguage = vueProps.$localStorage.get('LOCALE') ??
jsonConfig?.defaultLanguage ?? vueProps.$config.defaultLanguage
Review Comment:
`vueProps.$localStorage.get('LOCALE')` can be a non-string (the locale
loader explicitly treats `typeof locale === 'object'` as invalid). Using it
directly here can set `defaultLanguage` to an object and later build a bad
fetch URL like `locales/[object Object].json`. Consider normalizing/validating
the stored LOCALE value (string + supported pattern) before using it, and
falling back to `jsonConfig.defaultLanguage` / existing config when invalid.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
