From 9cecce414c194003e4ee1ca556197c94fb18f2c6 Mon Sep 17 00:00:00 2001 From: Lothar Haeger Date: Tue, 23 Jun 2026 18:24:19 +0200 Subject: [PATCH] AvailableLocale: sort the GUI language picker by display name The old sort key was country -> language -> variant on the Locale identifier. Because Locale.getCountry() returns "" (empty string, not null) for a language-only locale and compareStrings only short-circuits on null, every country-coded entry (en_US, pt_BR, zh_CN, en_GB) silently sorted after every language-only entry (de, fr, ...) in the GUI language selector - regardless of how its display name read. This made a typical "add en_GB and en_US for clearer English variants" configuration land at the bottom of the picker, dependent on country code rather than on visible name. There is no separate sort or order property in locale.properties; the comparator IS the only knob. Switch the comparator to sort by the configured .name field (the display name users actually see) using java.text.Collator with the JVM default locale. Country code, language code and variant no longer participate in the order; a country-coded locale is interleaved with the rest by its display name. A small TestNG suite in infra/common covers three cases: monotonicity of the sorted list under the default Collator, the specific scenario where a country-coded "Chinese" (zh_CN) precedes a language-only "Deutsch" (de) because "C" collates before "D", and the edge case of two locales with identical display names compareTo()-equal. --- .../midpoint/common/AvailableLocale.java | 44 ++++++---- .../midpoint/common/AvailableLocaleTest.java | 87 +++++++++++++++++++ 2 files changed, 112 insertions(+), 19 deletions(-) create mode 100644 infra/common/src/test/java/com/evolveum/midpoint/common/AvailableLocaleTest.java diff --git a/infra/common/src/main/java/com/evolveum/midpoint/common/AvailableLocale.java b/infra/common/src/main/java/com/evolveum/midpoint/common/AvailableLocale.java index 1896dc58e7c..48bff6e19ed 100644 --- a/infra/common/src/main/java/com/evolveum/midpoint/common/AvailableLocale.java +++ b/infra/common/src/main/java/com/evolveum/midpoint/common/AvailableLocale.java @@ -8,6 +8,7 @@ import java.io.*; import java.nio.charset.StandardCharsets; +import java.text.Collator; import java.util.*; import org.jetbrains.annotations.NotNull; @@ -244,30 +245,35 @@ public int hashCode() { return Objects.hash(name, flag, locale, def); } + /** + * Sort by display name as it appears in the GUI language selector. The previous + * implementation sorted by locale identifier (country, then language, then variant), + * which produced the surprising result that any locale with a country code (e.g. + * {@code en_US}, {@code pt_BR}, {@code zh_CN}) was pushed below every language-only + * entry (e.g. {@code de}, {@code fr}) regardless of how it read in the selector. Sorting + * by the configured {@code .name} matches what users see and works for the bare + * language entries and the country-suffixed entries side by side. + * + *

Uses the JVM's default locale for the {@link Collator}, so accents and + * non-Latin scripts collate sensibly for the server's environment. Null names are + * treated as sorting first; in practice every descriptor loaded from + * {@code locale.properties} has a non-null name (the loader skips entries without + * {@code .name}). + */ @Override public int compareTo(@NotNull LocaleDescriptor o) { - Locale other = o.getLocale(); - - int val = compareStrings(locale.getCountry(), other.getCountry()); - if (val != 0) { - return val; + if (name == null && o.name == null) { + return 0; } - - val = compareStrings(locale.getLanguage(), other.getLanguage()); - if (val != 0) { - return val; + if (name == null) { + return -1; } - - val = compareStrings(locale.getVariant(), other.getVariant()); - return val; - } - - private int compareStrings(String s1, String s2) { - if (s1 == null || s2 == null) { - return 0; + if (o.name == null) { + return 1; } - - return String.CASE_INSENSITIVE_ORDER.compare(s1, s2); + return NAME_COLLATOR.compare(name, o.name); } + + private static final Collator NAME_COLLATOR = Collator.getInstance(); } } diff --git a/infra/common/src/test/java/com/evolveum/midpoint/common/AvailableLocaleTest.java b/infra/common/src/test/java/com/evolveum/midpoint/common/AvailableLocaleTest.java new file mode 100644 index 00000000000..b3c803d76ba --- /dev/null +++ b/infra/common/src/test/java/com/evolveum/midpoint/common/AvailableLocaleTest.java @@ -0,0 +1,87 @@ +/* + * Copyright (C) 2026 Evolveum and contributors + * + * Licensed under the EUPL-1.2 or later. + */ + +package com.evolveum.midpoint.common; + +import static org.assertj.core.api.Assertions.assertThat; + +import java.text.Collator; +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; +import java.util.Locale; + +import org.testng.annotations.Test; + +/** + * Verifies that {@link AvailableLocale.LocaleDescriptor} sorts by display name (the value + * configured as {@code .name} in {@code locale.properties}) rather than by locale + * identifier. The old sort key was country -> language -> variant, which pushed every + * country-coded locale ({@code en_US}, {@code pt_BR}, {@code zh_CN}) below every + * language-only locale ({@code de}, {@code fr}) regardless of how the entries read in the + * GUI language selector. + */ +public class AvailableLocaleTest { + + private static AvailableLocale.LocaleDescriptor desc(String name, String flag, Locale locale) { + return new AvailableLocale.LocaleDescriptor(name, flag, null, locale); + } + + /** + * After sorting, the display names must be in non-decreasing collation order per the JVM + * default {@link Collator}. The exact order may vary slightly by JVM locale (accent + * handling etc.), but the list must remain monotonically sorted. + */ + @Test + public void sortedListIsMonotonicByDisplayName() { + List list = new ArrayList<>(List.of( + desc("English (US)", "us", new Locale("en", "US")), + desc("English (UK)", "gb", new Locale("en", "GB")), + desc("Français", "fr", new Locale("fr")), + desc("Deutsch", "de", new Locale("de")), + desc("Português (Brasil)", "br", new Locale("pt", "BR")))); + Collections.sort(list); + + Collator collator = Collator.getInstance(); + for (int i = 1; i < list.size(); i++) { + assertThat(collator.compare(list.get(i - 1).getName(), list.get(i).getName())) + .as("element %d (%s) must collate <= element %d (%s)", + i - 1, list.get(i - 1).getName(), i, list.get(i).getName()) + .isLessThanOrEqualTo(0); + } + } + + /** + * A country-coded locale ({@code zh_CN}, display name "Chinese") must NOT be pushed below + * a language-only locale ({@code de}, display name "Deutsch") just because it has a + * country code. "C" collates before "D" in every Locale's collation, so this assertion is + * robust across test environments. + */ + @Test + public void countryCodedLocaleIsNotPushedBelowLanguageOnlyByCountryCode() { + AvailableLocale.LocaleDescriptor zhCn = desc("Chinese", "cn", new Locale("zh", "CN")); + AvailableLocale.LocaleDescriptor de = desc("Deutsch", "de", new Locale("de")); + + List list = new ArrayList<>(List.of(zhCn, de)); + Collections.sort(list); + + assertThat(list.get(0).getName()).isEqualTo("Chinese"); + assertThat(list.get(1).getName()).isEqualTo("Deutsch"); + } + + /** + * Two locales with identical display names compare as equal regardless of their locale + * identifier. Edge case to guard against the previous implementation's fallback to + * country/language comparison sneaking back in. + */ + @Test + public void identicalDisplayNamesCompareEqual() { + AvailableLocale.LocaleDescriptor a = desc("Same", "us", new Locale("en", "US")); + AvailableLocale.LocaleDescriptor b = desc("Same", "gb", new Locale("en", "GB")); + assertThat(a.compareTo(b)).isZero(); + assertThat(b.compareTo(a)).isZero(); + } +}