QTranslator: fix loading order when loading from locale

A locale's UI language includes the script, e.g. the QLocale::uiLanguage
list for the Australian locale en_AU is {"en_Latn_AU", "en_AU", "en"}.
The old code iterated over each language in the outer loop, and for each
language tried several times, removing a segment from the back with each
try in the inner loop. The de-facto search order was then

en_Latn_AU
en_latn_au
en_Latn
en_latn
en
en_AU
en_au
en

Usually, translation files are provided for country_Territory, i.e.
en_AU. But if an en file (for a generic English translation) was also
present, then en_AU was never tried.

Fix this by breaking the logic into two loops: first, create a list of
candidates by removing segments from each UI Language. Then sort that
list of candidates so that entries with more segments come first. The
search order is now:

en_Latn_AU
en_latn_au
en_Latn
en_latn
en_AU
en_au
en

This way, en_AU gets loaded correctly, before en is tried.

Adjust the test, which was essentially duplicating the logic from
QTranslator to generate files. This only tested that two identical loops
result in the same thing. Instead of using the system locale, make the
test data-driven, and explicitly try en_US and en_AU, with candidate
files generated based on a hardcoded list for each data row.

Pick-to: 6.8 6.7 6.5
Fixes: QTBUG-124898
Change-Id: I6bdcff289d2843e61c9053c116e955b79e09e95a
Reviewed-by: Mate Barany <mate.barany@qt.io>

src/corelib/kernel/qtranslator.cpp

@@ -638,25 +638,41 @@ static QString find_translation(const QLocale & locale,
             languages.insert(i + 1, lowerLang);
     }
 
-    for (QString localeName : std::as_const(languages)) {
-        // try the complete locale name first and progressively truncate from
-        // the end until a matching language tag is found (with or without suffix)
+    QStringList candidates;
+    // assume 3 segments for each entry
+    candidates.reserve(languages.size() * 3);
+    for (QStringView language : std::as_const(languages)) {
+        // for each language, add versions without territory and script
         for (;;) {
-            realname += localeName + suffixOrDotQM;
-            if (is_readable_file(realname))
-                return realname;
+            candidates += language.toString();
+            int rightmost = language.lastIndexOf(u'_');
+            if (rightmost <= 0)
+                break;
+            language.truncate(rightmost);
+        }
+    }
 
-            realname.truncate(realNameBaseSize + localeName.size());
-            if (is_readable_file(realname))
-                return realname;
+    // now sort the list of candidates
+    std::sort(candidates.begin(), candidates.end(), [](const auto &lhs, const auto &rhs){
+        const auto rhsSegments = rhs.count(u'_');
+        const auto lhsSegments = lhs.count(u'_');
+        // candidates with more segments come first
+        if (rhsSegments != lhsSegments)
+            return rhsSegments < lhsSegments;
+        // candidates with same number of segments are sorted alphanumerically
+        return lhs < rhs;
+    });
+
+    for (const QString &localeName : std::as_const(candidates)) {
+        realname += localeName + suffixOrDotQM;
+        if (is_readable_file(realname))
+            return realname;
 
-            realname.truncate(realNameBaseSize);
+        realname.truncate(realNameBaseSize + localeName.size());
+        if (is_readable_file(realname))
+            return realname;
 
-            int rightmost = localeName.lastIndexOf(u'_');
-            if (rightmost <= 0)
-                break; // no truncations anymore, break
-            localeName.truncate(rightmost);
-        }
+        realname.truncate(realNameBaseSize);
     }
 
     const int realNameBaseSizeFallbacks = path.size() + filename.size();

tests/auto/corelib/kernel/qtranslator/tst_qtranslator.cpp

@@ -27,6 +27,7 @@ private slots:
 
     void load_data();
     void load();
+    void loadLocale_data();
     void loadLocale();
     void threadLoad();
     void testLanguageChange();
@@ -116,12 +117,23 @@ void tst_QTranslator::load()
     }
 }
 
+void tst_QTranslator::loadLocale_data()
+{
+    QTest::addColumn<QString>("localeName");
+    QTest::addColumn<QStringList>("fileNames");
+
+    QTest::addRow("US English")
+                            << "en_US"
+                            << QStringList{"en_US.qm", "en_US", "en.qm", "en"};
+    QTest::addRow("Australia")
+                            << "en_AU"
+                            << QStringList{"en_Latn_AU.qm", "en_AU.qm", "en.qm"};
+}
+
 void tst_QTranslator::loadLocale()
 {
-    QLocale locale;
-    auto localeName = locale.uiLanguages(QLocale::TagSeparator::Underscore).value(0);
-    if (localeName.isEmpty())
-        QSKIP("This test requires at least one available UI language.");
+    QFETCH(const QString, localeName);
+    QFETCH(const QStringList, fileNames);
 
     QByteArray ba;
     {
@@ -134,36 +146,16 @@ void tst_QTranslator::loadLocale()
     QTemporaryDir dir;
     QVERIFY(dir.isValid());
 
-    auto path = dir.path();
+    const auto path = dir.path();
     QFile file(path + "/dummy");
     QVERIFY2(file.open(QFile::WriteOnly), qPrintable(file.errorString()));
     QCOMPARE(file.write(ba), ba.size());
     file.close();
 
-    /*
-        Test the following order:
-
-        /tmp/tmpDir/foo-en_US.qm
-        /tmp/tmpDir/foo-en_US
-        /tmp/tmpDir/foo-en.qm
-        /tmp/tmpDir/foo-en
-        /tmp/tmpDir/foo.qm
-        /tmp/tmpDir/foo-
-        /tmp/tmpDir/foo
-    */
-
     QStringList files;
-    while (true) {
-        files.append(path + "/foo-" + localeName + ".qm");
-        QVERIFY2(file.copy(files.last()), qPrintable(file.errorString()));
-
-        files.append(path + "/foo-" + localeName);
+    for (const auto &fileName : fileNames) {
+        files.append(path + "/foo-" + fileName);
         QVERIFY2(file.copy(files.last()), qPrintable(file.errorString()));
-
-        int rightmost = localeName.lastIndexOf(QLatin1Char('_'));
-        if (rightmost <= 0)
-            break;
-        localeName.truncate(rightmost);
     }
 
     files.append(path + "/foo.qm");
@@ -175,10 +167,14 @@ void tst_QTranslator::loadLocale()
     files.append(path + "/foo");
     QVERIFY2(file.rename(files.last()), qPrintable(file.errorString()));
 
+    QLocale locale(localeName);
     QTranslator tor;
     for (const auto &filePath : files) {
         QVERIFY(tor.load(locale, "foo", "-", path, ".qm"));
-        QCOMPARE(tor.filePath(), filePath);
+        // As the file system might be case insensitive, we can't guarantee that
+        // the casing of the file name is preserved. The order of loading
+        // en_AU vs en_au if both exist is undefined anyway.
+        QCOMPARE(tor.filePath().toLower(), filePath.toLower());
         QVERIFY2(file.remove(filePath), qPrintable(file.errorString()));
     }
 }