Skip to content

Commit

Permalink
macOS: Be honest about the system locale
Browse files Browse the repository at this point in the history
The system locale of a macOS application is not affected by environment
variables like LANG. Yet, we were reporting a name determined from
environment variables as the fallbackUiLocale(), rather than one based
on the language and country of the actual system locale.

This lead, via the usual CLDR likely-subtag fallback, to claiming the
system locale's name, language, script and country were those obtained
from these environment variables, even when they were at odds with the
actual locale being used by the system, which was being used for some
queries.

Worse yet, any data not supplied by these queries was being obtained
from the same CLDR locale as the name, making for an inconsistent mix
of locale data.

While we cannot avoid the likely-subtag fallback step for fallback
data, it is more consistent to use the actual system locale's name
as start-point for that fallback.

At the same time, add support for the language, script and country
queries, so that the QLocale::system() describes itself faithfully,
instead of claiming to be the locale that results from that fallback.

If we want to support LANG or other environment variable overrides,
they should be handled by the layer above the system locale, by
changing the default locale of the Qt application, as if the user
had called QLocale::setDefault().

[ChangeLog][QtCore][QLocale] QLocale::system() on macOS no longer
pretends to support LANG or other environment variables as overrides,
as this is not a feature that the system locale on macOS supports.
To override the locale of an application, use QLocale::setDefault(),
or pass -AppleLocale en_US.

Fixes: QTBUG-90971
Change-Id: Ibdaf5ff9a2050f61233a88eabf3c29094f7757f1
Reviewed-by: Volker Hilsheimer <volker.hilsheimer@qt.io>
Reviewed-by: Edward Welbourne <edward.welbourne@qt.io>
  • Loading branch information
torarnv committed Feb 11, 2021
1 parent 62af655 commit d8158c6
Show file tree
Hide file tree
Showing 2 changed files with 38 additions and 27 deletions.
47 changes: 24 additions & 23 deletions src/corelib/text/qlocale_mac.mm
Original file line number Diff line number Diff line change
Expand Up @@ -54,29 +54,11 @@
** Wrappers for Mac locale system functions
*/

static QByteArray envVarLocale()
{
QByteArray
#ifdef Q_OS_UNIX
lang = qgetenv("LC_ALL");
if (lang.isEmpty())
lang = qgetenv("LC_NUMERIC");
if (lang.isEmpty())
#endif
lang = qgetenv("LANG");
return lang;
}

static QString getMacLocaleName()
{
QString result = QString::fromLocal8Bit(envVarLocale());
if (result.isEmpty()
|| (result != QLatin1String("C") && !qt_splitLocaleName(result))) {
QCFType<CFLocaleRef> l = CFLocaleCopyCurrent();
CFStringRef locale = CFLocaleGetIdentifier(l);
result = QString::fromCFString(locale);
}
return result;
QCFType<CFLocaleRef> l = CFLocaleCopyCurrent();
CFStringRef locale = CFLocaleGetIdentifier(l);
return QString::fromCFString(locale);
}

static QString macMonthName(int month, QSystemLocale::QueryType type)
Expand Down Expand Up @@ -423,12 +405,31 @@ static QVariant macQuoteString(QSystemLocale::QueryType type, QStringView str)
return QLocale(getMacLocaleName());
}

template <auto CodeToValueFunction>
static QVariant getLocaleValue(CFStringRef key)
{
if (auto code = getCFLocaleValue(key); !code.isNull()) {
// If an invalid locale is requested with -AppleLocale, the system APIs
// will report invalid or empty locale values back to us, which codeToLanguage()
// and friends will fail to parse, resulting in returning QLocale::Any{L/C/S}.
// If this is the case, we fall down and return a null-variant, which
// QLocale's updateSystemPrivate() will interpret to use fallback logic.
if (auto value = CodeToValueFunction(code.toString()))
return value;
}
return QVariant();
}

QVariant QSystemLocale::query(QueryType type, QVariant in) const
{
QMacAutoReleasePool pool;
switch(type) {
// case Name:
// return getMacLocaleName();
case LanguageId:
return getLocaleValue<QLocalePrivate::codeToLanguage>(kCFLocaleLanguageCode);
case CountryId:
return getLocaleValue<QLocalePrivate::codeToCountry>(kCFLocaleCountryCode);
case ScriptId:
return getLocaleValue<QLocalePrivate::codeToScript>(kCFLocaleScriptCode);
case DecimalPoint:
return getCFLocaleValue(kCFLocaleDecimalSeparator);
case GroupSeparator:
Expand Down
18 changes: 14 additions & 4 deletions tests/auto/corelib/text/qlocale/tst_qlocale.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -504,6 +504,7 @@ void tst_QLocale::defaulted_ctor()

#if QT_CONFIG(process)
static inline bool runSysApp(const QString &binary,
const QStringList &args,
const QStringList &env,
QString *output,
QString *errorMessage)
Expand All @@ -512,7 +513,7 @@ static inline bool runSysApp(const QString &binary,
errorMessage->clear();
QProcess process;
process.setEnvironment(env);
process.start(binary, QStringList());
process.start(binary, args);
process.closeWriteChannel();
if (!process.waitForStarted()) {
*errorMessage = QLatin1String("Cannot start '") + binary
Expand All @@ -535,8 +536,12 @@ static inline bool runSysAppTest(const QString &binary,
QString *errorMessage)
{
QString output;
QStringList args;
#ifdef Q_OS_MACOS
args << "-AppleLocale" << requestedLocale;
#endif
baseEnv.append(QStringLiteral("LANG=") + requestedLocale);
if (!runSysApp(binary, baseEnv, &output, errorMessage))
if (!runSysApp(binary, args, baseEnv, &output, errorMessage))
return false;

if (output.isEmpty()) {
Expand Down Expand Up @@ -617,8 +622,13 @@ void tst_QLocale::emptyCtor_data()
// Get default locale.
QString defaultLoc;
QString errorMessage;
if (runSysApp(m_sysapp, cleanEnv, &defaultLoc, &errorMessage)) {
#define ADD_CTOR_TEST(give) QTest::newRow(give) << defaultLoc;
if (runSysApp(m_sysapp, QStringList(), cleanEnv, &defaultLoc, &errorMessage)) {
#if defined(Q_OS_MACOS)
QString localeForInvalidLocale = "C";
#else
QString localeForInvalidLocale = defaultLoc;
#endif
#define ADD_CTOR_TEST(give) QTest::newRow(give) << localeForInvalidLocale;
ADD_CTOR_TEST("en/");
ADD_CTOR_TEST("asdfghj");
ADD_CTOR_TEST("123456");
Expand Down

0 comments on commit d8158c6

Please sign in to comment.