VaadinSession uses first provided Locale which breaks the default bundle usage

[simasch]

Description of the bug

If a i18n provider is available, the locale is determined by selecting the locale from {@link I18NProvider#getProvidedLocales()} that best matches the user agent preferences (i.e. the <code>Accept-Language</code> header). If an exact match is found, then that locale is used. Otherwise, the matching logic looks for the first provided locale that uses the same language regardless of the country. If no other match is found, then the first item from {@link I18NProvider#getProvidedLocales()} is used.

This initially breaks the default behavior of the Java ResourceBundle.
Typically, a translations.properties without language is provided as a catch-all translation bundle.

Expected behavior

If we have translations_de.properties and translations.properties and the user has a language with no specific resource bundle, the translations from the translations.properties file must be used.

Minimal reproducible example

https://github.com/simasch/vaadin-default-translation-provider-bug

Versions

All versions since DefaultI18NProvider

[Legioth]

I guess the problem is related to the fact that some Locale still needs to be picked and there's no way of knowing which one to use for the default bundle. I wonder if it would be reasonable to change I18NUtil::collectLocalesFromFileNames to use Locale.getDefault() for the default bundle instead of the current logic that just ignores it? Or would Locale.ROOT be more semantically correct?

We would then also have to make sure we remove duplicates (since there might also be a non-default bundle for the default locale) and put the default locale as the first one in the list.

[simasch]

I don't understand why your mechanism is so complicated. Why do you even need to know the locales?

Isn't the default behavior just good enough?

https://docs.oracle.com/javase/tutorial/i18n/resbundle/concept.html

[simasch]

I just disable this mechanism by returning an empty List:

@Component
public class TranslationProvider implements I18NProvider {

    private static final Logger LOGGER = LoggerFactory.getLogger(TranslationProvider.class);

    @Override
    public List<Locale> getProvidedLocales() {
        return List.of();
    }

    @Override
    public String getTranslation(String key, Locale locale, Object... params) {
        if (key == null) {
            LOGGER.warn("Got lang request for key with null value!");
            return "";
        }
        var bundle = ResourceBundle.getBundle("messages", locale);
        try {
            var value = bundle.getString(key);
            if (params.length > 0) {
                value = MessageFormat.format(value, params);
            }
            return value;
        }
        catch (MissingResourceException e) {
            LOGGER.warn("Missing resource", e);
            return "!" + locale.getLanguage() + ": " + key;
        }
    }

}

[Legioth]

We're limiting the automatic choice to a list of application-provided locales because of components like DatePicker that automatically support any locale even without the help of language bundles in the application. Without that list, those components would just use whatever language the user's browser is set to even if that is inconsistent with the rest of the application. I personally find it very annoying when an application is otherwise fully in English but still uses , instead of . as the decimal separator based on some system setting (but I also know that this is subjective).

The idea of initializing the list based on available bundles is to automatically configure those components based on the locales that the application supports while avoiding unsupported ones. But there's indeed a problem with the default bundle since we can't know how those components should be configured when that bundle is used. This makes me think that Locale.getDefault() would be the best option.

[simasch]

In my experience using the DatePicker or DateTimePicker without explicitly configuring I18N doesn't work.

[Legioth]

I guess that's because it doesn't know what locale to use for the default case. If you have a suitable example project close by, then you could try using DefaultI18NProvider as it's configured by VaadinApplicationConfiguration.vaadinI18nProvider(String) but overriding it to use something like Stream.concat(Stream.of(yourDefaultLocale), defaultProvidedLocales.stream()).toList() as the list of locales. If that works, then we should consider changing collectLocalesFromFileNames to include Locale.getDefault() if there's a default bundle instead of just ignoring it.

[simasch]

The problem with the DatePicker is a bit more complex because of the language and the formatting.

The formatting depends on the Country, not the language.
Switzerland has four languages, but the formatting is always the same.

So we use resource bundles without the country code, and the one without the language is the default, which is usually German.

For the DatePicker, we configure this and the parsing format.

var symbols = new DateFormatSymbols(locale);
var datePickerI18n = new DatePicker.DatePickerI18n();
datePickerI18n.setMonthNames(Arrays.asList(symbols.getMonths()));
datePickerI18n.setFirstDayOfWeek(1);
datePickerI18n.setWeekdays(Arrays.stream(symbols.getWeekdays()).filter(s -> !s.isEmpty()).toList());
datePickerI18n.setWeekdaysShort(Arrays.stream(symbols.getShortWeekdays()).filter(s -> !s.isEmpty()).toList());

[Legioth]

Does DatePicker work as you expect if you configure it with a locale that includes the country, e.g. picker.setLocale(new Locale("fr", "ch")); but no other configuration?

If not, then that's probably something we should try to fix (but that's a separate ticket). And if that works as expected, then one partial solution for your case might be to explicitly list the Swiss locales rather than ones with only language but no country (but we'd still have to do something about the default language as well).