From 71fa90a37c4f250aa4a6ae8e6bd957dd372566c8 Mon Sep 17 00:00:00 2001 From: Edward Welbourne Date: Mon, 6 Jan 2020 13:15:06 +0100 Subject: 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 --- src/corelib/text/qlocale.cpp | 36 ++++++++++++++++++++++++++++++++---- src/corelib/text/qlocale_mac.mm | 16 +++++++--------- src/corelib/text/qlocale_p.h | 4 ++-- 3 files changed, 41 insertions(+), 15 deletions(-) (limited to 'src/corelib') diff --git a/src/corelib/text/qlocale.cpp b/src/corelib/text/qlocale.cpp index 26db674a99..11e788c200 100644 --- a/src/corelib/text/qlocale.cpp +++ b/src/corelib/text/qlocale.cpp @@ -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, 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, defaultLocalePrivate, (QLocalePrivate::create(defaultData()))) -Q_GLOBAL_STATIC_WITH_ARGS(QExplicitlySharedDataPointer, systemLocalePrivate, - (QLocalePrivate::create(systemData()))) static QLocalePrivate *localePrivateByName(const QString &name) { diff --git a/src/corelib/text/qlocale_mac.mm b/src/corelib/text/qlocale_mac.mm index 7bdae662c7..5381f0f975 100644 --- a/src/corelib/text/qlocale_mac.mm +++ b/src/corelib/text/qlocale_mac.mm @@ -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 locale = CFLocaleCopyCurrent(); CFTypeRef value = CFLocaleGetValue(locale, key); + if (!value) + return QVariant(); return QString::fromCFString(CFStringRef(static_cast(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 diff --git a/src/corelib/text/qlocale_p.h b/src/corelib/text/qlocale_p.h index bb24009523..7e26e86fdf 100644 --- a/src/corelib/text/qlocale_p.h +++ b/src/corelib/text/qlocale_p.h @@ -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 -- cgit v1.2.3