From 9a11273b745a30cebb5cd648c89eb224e9704492 Mon Sep 17 00:00:00 2001 From: Volker Hilsheimer Date: Mon, 17 Jun 2024 16:49:29 +0200 Subject: [PATCH] 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 --- src/corelib/kernel/qtranslator.cpp | 46 ++++++++++------ .../kernel/qtranslator/tst_qtranslator.cpp | 52 +++++++++---------- 2 files changed, 55 insertions(+), 43 deletions(-) diff --git a/src/corelib/kernel/qtranslator.cpp b/src/corelib/kernel/qtranslator.cpp index ec92404a154..5c4ebfedf14 100644 --- a/src/corelib/kernel/qtranslator.cpp +++ b/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(); diff --git a/tests/auto/corelib/kernel/qtranslator/tst_qtranslator.cpp b/tests/auto/corelib/kernel/qtranslator/tst_qtranslator.cpp index c76500ea119..f214572cfa2 100644 --- a/tests/auto/corelib/kernel/qtranslator/tst_qtranslator.cpp +++ b/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("localeName"); + QTest::addColumn("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())); } }