From 1e74f67e54238fda26e29bd4c72207339b636e22 Mon Sep 17 00:00:00 2001 From: Edward Welbourne Date: Tue, 24 Oct 2023 15:48:57 +0200 Subject: [PATCH] Work round ICU disagreeing with TZDB about zone transitions Since the TZ backend resorts to ICU for display names, we got inconsistent results if they disagree about when transitions happened. Also, ICU uses the current (or only recent history) names for the zone, so one currently not doing DST (Africa/Tripoli) doesn't get a report of its DST name at a historical time when it did DST (but ICU doesn't know about it). Since the ICU backend, in any case, doesn't override the displayName(qint64, ...), we can simply use the QTZP base version instead of overloading in the TZ back-end, so we only delegate to ICU when it might actually help. This also saves duplicating some locking and lazy-initialization code. In the process, turn a unique lock into a scoped lock within a suitable scope. Also, make a comment more grammatical and less verbose. Change-Id: Iaaed1fb6a380be8b4258c43cbc4bacd5784345fb Reviewed-by: Ivan Solovev --- src/corelib/time/qtimezoneprivate_p.h | 3 -- src/corelib/time/qtimezoneprivate_tz.cpp | 41 +++++-------------- .../auto/corelib/text/qlocale/tst_qlocale.cpp | 6 ++- 3 files changed, 16 insertions(+), 34 deletions(-) diff --git a/src/corelib/time/qtimezoneprivate_p.h b/src/corelib/time/qtimezoneprivate_p.h index 51e093e002c..456a1db6064 100644 --- a/src/corelib/time/qtimezoneprivate_p.h +++ b/src/corelib/time/qtimezoneprivate_p.h @@ -290,9 +290,6 @@ public: QLocale::Territory territory() const override; QString comment() const override; - QString displayName(qint64 atMSecsSinceEpoch, - QTimeZone::NameType nameType, - const QLocale &locale) const override; QString displayName(QTimeZone::TimeType timeType, QTimeZone::NameType nameType, const QLocale &locale) const override; diff --git a/src/corelib/time/qtimezoneprivate_tz.cpp b/src/corelib/time/qtimezoneprivate_tz.cpp index 0046782fc9a..ab5978e0a69 100644 --- a/src/corelib/time/qtimezoneprivate_tz.cpp +++ b/src/corelib/time/qtimezoneprivate_tz.cpp @@ -1022,46 +1022,27 @@ QString QTzTimeZonePrivate::comment() const return QString::fromUtf8(tzZones->value(m_id).comment); } -QString QTzTimeZonePrivate::displayName(qint64 atMSecsSinceEpoch, - QTimeZone::NameType nameType, - const QLocale &locale) const -{ -#if QT_CONFIG(icu) - auto lock = qt_unique_lock(s_icu_mutex); - if (!m_icu) - m_icu = new QIcuTimeZonePrivate(m_id); - // TODO small risk may not match if tran times differ due to outdated files - // TODO Some valid TZ names are not valid ICU names, use translation table? - if (m_icu->isValid()) - return m_icu->displayName(atMSecsSinceEpoch, nameType, locale); - lock.unlock(); -#else - Q_UNUSED(nameType); - Q_UNUSED(locale); -#endif - // Fall back to base-class: - return QTimeZonePrivate::displayName(atMSecsSinceEpoch, nameType, locale); -} - QString QTzTimeZonePrivate::displayName(QTimeZone::TimeType timeType, QTimeZone::NameType nameType, const QLocale &locale) const { + // TZ DB lacks localized names (it only has IANA IDs), so delegate to ICU + // for those, when available. #if QT_CONFIG(icu) - auto lock = qt_unique_lock(s_icu_mutex); - if (!m_icu) - m_icu = new QIcuTimeZonePrivate(m_id); - // TODO small risk may not match if tran times differ due to outdated files - // TODO Some valid TZ names are not valid ICU names, use translation table? - if (m_icu->isValid()) - return m_icu->displayName(timeType, nameType, locale); - lock.unlock(); + { + auto lock = qt_scoped_lock(s_icu_mutex); + // TODO Some valid TZ names are not valid ICU names, use translation table? + if (!m_icu) + m_icu = new QIcuTimeZonePrivate(m_id); + if (m_icu->isValid()) + return m_icu->displayName(timeType, nameType, locale); + } #else Q_UNUSED(timeType); Q_UNUSED(nameType); Q_UNUSED(locale); #endif - // If no ICU available then have to use abbreviations instead + // If ICU is unavailable, fall back to abbreviations. // Abbreviations don't have GenericTime if (timeType == QTimeZone::GenericTime) timeType = QTimeZone::StandardTime; diff --git a/tests/auto/corelib/text/qlocale/tst_qlocale.cpp b/tests/auto/corelib/text/qlocale/tst_qlocale.cpp index 53b4f10447d..1b0fc57ce5c 100644 --- a/tests/auto/corelib/text/qlocale/tst_qlocale.cpp +++ b/tests/auto/corelib/text/qlocale/tst_qlocale.cpp @@ -173,7 +173,11 @@ tst_QLocale::tst_QLocale() // Some tests are specific to CET, test if it applies: : europeanTimeZone( QDate(2013, 1, 1).startOfDay().offsetFromUtc() == 3600 - && QDate(2013, 6, 1).startOfDay().offsetFromUtc() == 7200) + && QDate(2013, 6, 1).startOfDay().offsetFromUtc() == 7200 + // ICU in a zone not currently doing DST may ignore any historical DST + // excursions in its display-names (Africa/Tripoli). + && QDate(QDate::currentDate().year(), 1, 1).startOfDay().offsetFromUtc() == 3600 + && QDate(QDate::currentDate().year(), 7, 1).startOfDay().offsetFromUtc() == 7200) { qRegisterMetaType("QLocale::FormatType"); }