Enable system locale to skip digit-grouping if configured to do so

On macOS it's possible to configure the system locale to not do digit
grouping (separating "thousands", in most western locales); it then
returns an empty string when asked for the grouping character, which
QLocale's system-configuration then ignored, falling back on using the
base UI locale's grouping separator. This could lead to the same
separator being used for decimal and grouping, which should never
happen, least of all when configured to not group at all.

In order to notice when this happens, query() must take care to return
an empty QString (as a QVariant, which is then non-null) when it *has*
a value for the locale property, and that value is empty, as opposed
to a null QVariant when it doesn't find a configured value. The caller
can then distinguish the two cases.

Furthermore, the group and decimal separators need to be distinct, so
we need to take care to avoid cases where the system overrides one
with what the CLDR has given for the other and doesn't over-ride that
other.

Only presently implemented for macOS and MS-Win, since the (other)
Unix implementation of the system locale returns single QChar values
for the numeric tokens - see QTBUG-69324, QTBUG-81053.

Fixes: QTBUG-80459
Change-Id: Ic3fbb0fb86e974604a60781378b09abc13bab15d
Reviewed-by: Ulf Hermann <ulf.hermann@qt.io>
This commit is contained in:
Edward Welbourne 2020-01-06 13:15:06 +01:00
parent 3730452bfe
commit 71fa90a37c
5 changed files with 59 additions and 28 deletions

View File

@ -660,6 +660,11 @@ static QLocalePrivate *c_private()
return &c_locale;
}
static const QLocaleData *systemData();
static QLocale::NumberOptions system_number_options = QLocale::DefaultNumberOptions;
Q_GLOBAL_STATIC_WITH_ARGS(QExplicitlySharedDataPointer<QLocalePrivate>, systemLocalePrivate,
(QLocalePrivate::create(systemData(), system_number_options)))
#ifndef QT_NO_SYSTEMLOCALE
/******************************************************************************
** Default system locale behavior
@ -711,6 +716,7 @@ static void updateSystemPrivate()
{
// This function is NOT thread-safe!
// It *should not* be called by anything but systemData()
// It *is* called before {system,default}LocalePrivate exist.
const QSystemLocale *sys_locale = systemLocale();
// tell the object that the system locale has changed.
@ -718,11 +724,14 @@ static void updateSystemPrivate()
// Populate global with fallback as basis:
globalLocaleData = *sys_locale->fallbackUiLocaleData();
system_number_options = QLocale::DefaultNumberOptions;
QVariant res = sys_locale->query(QSystemLocale::LanguageId, QVariant());
if (!res.isNull()) {
globalLocaleData.m_language_id = res.toInt();
globalLocaleData.m_script_id = QLocale::AnyScript; // default for compatibility
if (globalLocaleData.m_language_id == QLocale::C)
system_number_options = QLocale::OmitGroupSeparator;
}
res = sys_locale->query(QSystemLocale::CountryId, QVariant());
if (!res.isNull()) {
@ -737,9 +746,26 @@ static void updateSystemPrivate()
if (!res.isNull() && !res.toString().isEmpty())
globalLocaleData.m_decimal = res.toString().at(0).unicode();
// System may supply empty group separator to say we should omit grouping;
// and it makes no sense to use the same separator for decimal and grouping
// (which might happen by system supplying, as decimal, what CLDR has given
// us for grouping; or the other way round). Assume, at least, that each of
// system and CLDR has decimal != group, all the same.
res = sys_locale->query(QSystemLocale::GroupSeparator, QVariant());
if (!res.isNull() && !res.toString().isEmpty())
globalLocaleData.m_group = res.toString().at(0).unicode();
if (res.isNull()) {
// The case where system over-rides decimal but not group, and its
// decimal clashes with CLDR's group.
if (globalLocaleData.m_group == globalLocaleData.m_decimal)
system_number_options |= QLocale::OmitGroupSeparator;
} else if (res.toString().isEmpty()) {
system_number_options |= QLocale::OmitGroupSeparator;
} else {
const ushort group = res.toString().at(0).unicode();
if (group != globalLocaleData.m_decimal)
globalLocaleData.m_group = group;
else if (group == globalLocaleData.m_group)
qWarning("System-supplied decimal and grouping character are both 0x%hx", group);
}
res = sys_locale->query(QSystemLocale::ZeroDigit, QVariant());
if (!res.isNull() && !res.toString().isEmpty())
@ -752,6 +778,10 @@ static void updateSystemPrivate()
res = sys_locale->query(QSystemLocale::PositiveSign, QVariant());
if (!res.isNull() && !res.toString().isEmpty())
globalLocaleData.m_plus = res.toString().at(0).unicode();
if (systemLocalePrivate.exists())
systemLocalePrivate->data()->m_numberOptions = system_number_options;
// else: system_number_options will be passed to create() when constructing.
}
#endif // !QT_NO_SYSTEMLOCALE
@ -834,8 +864,6 @@ static const int locale_data_size = sizeof(locale_data)/sizeof(QLocaleData) - 1;
Q_GLOBAL_STATIC_WITH_ARGS(QSharedDataPointer<QLocalePrivate>, defaultLocalePrivate,
(QLocalePrivate::create(defaultData())))
Q_GLOBAL_STATIC_WITH_ARGS(QExplicitlySharedDataPointer<QLocalePrivate>, systemLocalePrivate,
(QLocalePrivate::create(systemData())))
static QLocalePrivate *localePrivateByName(const QString &name)
{

View File

@ -283,10 +283,12 @@ static QString getMacTimeFormat(CFDateFormatterStyle style)
return macToQtFormat(QString::fromCFString(CFDateFormatterGetFormat(formatter)));
}
static QString getCFLocaleValue(CFStringRef key)
static QVariant getCFLocaleValue(CFStringRef key)
{
QCFType<CFLocaleRef> locale = CFLocaleCopyCurrent();
CFTypeRef value = CFLocaleGetValue(locale, key);
if (!value)
return QVariant();
return QString::fromCFString(CFStringRef(static_cast<CFTypeRef>(value)));
}
@ -411,14 +413,10 @@ QVariant QSystemLocale::query(QueryType type, QVariant in) const
switch(type) {
// case Name:
// return getMacLocaleName();
case DecimalPoint: {
QString value = getCFLocaleValue(kCFLocaleDecimalSeparator);
return value.isEmpty() ? QVariant() : value;
}
case GroupSeparator: {
QString value = getCFLocaleValue(kCFLocaleGroupingSeparator);
return value.isEmpty() ? QVariant() : value;
}
case DecimalPoint:
return getCFLocaleValue(kCFLocaleDecimalSeparator);
case GroupSeparator:
return getCFLocaleValue(kCFLocaleGroupingSeparator);
case DateFormatLong:
case DateFormatShort:
return getMacDateFormat(type == DateFormatShort

View File

@ -1,6 +1,6 @@
/****************************************************************************
**
** Copyright (C) 2019 The Qt Company Ltd.
** Copyright (C) 2020 The Qt Company Ltd.
** Copyright (C) 2016 Intel Corporation.
** Contact: https://www.qt.io/licensing/
**
@ -87,7 +87,7 @@ public:
LanguageId, // uint
CountryId, // uint
DecimalPoint, // QString
GroupSeparator, // QString
GroupSeparator, // QString (empty QString means: don't group digits)
ZeroDigit, // QString
NegativeSign, // QString
DateFormatLong, // QString

View File

@ -1,6 +1,6 @@
/****************************************************************************
**
** Copyright (C) 2019 The Qt Company Ltd.
** Copyright (C) 2020 The Qt Company Ltd.
** Contact: https://www.qt.io/licensing/
**
** This file is part of the test suite of the Qt Toolkit.
@ -1869,14 +1869,16 @@ void tst_QLocale::toDateTime()
// Format number string according to system locale settings.
// Expected in format is US "1,234.56".
QString systemLocaleFormatNumber(const QString &numberString)
QString systemLocaleFormatNumber(QString &&numberString)
{
QLocale locale = QLocale::system();
QString numberStringCopy = numberString;
return numberStringCopy.replace(QChar(','), QChar('G'))
.replace(QChar('.'), QChar('D'))
.replace(QChar('G'), locale.groupSeparator())
.replace(QChar('D'), locale.decimalPoint());
QString numberStringMunged =
numberString.replace(QChar(','), QChar('G')).replace(QChar('.'), QChar('D'));
if (locale.numberOptions() & QLocale::OmitGroupSeparator)
numberStringMunged.remove(QLatin1Char('G'));
else
numberStringMunged.replace(QChar('G'), locale.groupSeparator());
return numberStringMunged.replace(QChar('D'), locale.decimalPoint());
}
void tst_QLocale::macDefaultLocale()
@ -1899,12 +1901,14 @@ void tst_QLocale::macDefaultLocale()
// independently of the locale. Verify that they have one of the
// allowed values and are not the same.
QVERIFY(locale.decimalPoint() == QChar('.') || locale.decimalPoint() == QChar(','));
QVERIFY(locale.groupSeparator() == QChar(',')
|| locale.groupSeparator() == QChar('.')
|| locale.groupSeparator() == QChar('\xA0') // no-breaking space
|| locale.groupSeparator() == QChar('\'')
|| locale.groupSeparator() == QChar());
QVERIFY(locale.decimalPoint() != locale.groupSeparator());
if (!(locale.numberOptions() & QLocale::OmitGroupSeparator)) {
QVERIFY(locale.groupSeparator() == QChar(',')
|| locale.groupSeparator() == QChar('.')
|| locale.groupSeparator() == QChar('\xA0') // no-breaking space
|| locale.groupSeparator() == QChar('\'')
|| locale.groupSeparator() == QChar());
QVERIFY(locale.decimalPoint() != locale.groupSeparator());
}
// make sure we are using the system to parse them
QCOMPARE(locale.toString(1234.56), systemLocaleFormatNumber(QString("1,234.56")));

View File

@ -305,6 +305,7 @@ def _generateLocaleInfo(path, language_code, script_code, country_code, variant_
result['decimal'] = get_number_in_system(path, "numbers/symbols/decimal", numbering_system)
result['group'] = get_number_in_system(path, "numbers/symbols/group", numbering_system)
assert result['decimal'] != result['group']
result['list'] = get_number_in_system(path, "numbers/symbols/list", numbering_system)
result['percent'] = get_number_in_system(path, "numbers/symbols/percentSign", numbering_system)
try: