Simplify date-time formatting in textForRole()

QAbstractItemDelegatePrivate::textForRole() was formatting date and
time separately, then gluing the parts back together with space.
QLocale can do that just fine itself (it has a toString() overload
that handles a QDateTime) and might even (some day) do it better.  To
my mild surprise, this proved sufficient to fix a problem with
date-time display in tool-tips, when the date-time includes a zone.

Extracted the date-time part of an existing selftest into a test of
its own and extended it to test times of each spec-type; verified that
the non-local spec cases of this all failed before this fix.

Task-number: QTBUG-61069
Change-Id: I6d6be0c27be9a557d8afc3ced200a10b2aaff816
Reviewed-by: Marc Mutz <marc.mutz@kdab.com>
bb10
Edward Welbourne 2017-11-27 14:18:24 +01:00
parent 454bbbe787
commit c27348985d
2 changed files with 53 additions and 28 deletions

View File

@ -585,12 +585,9 @@ QString QAbstractItemDelegatePrivate::textForRole(Qt::ItemDataRole role, const Q
case QVariant::Time:
text = locale.toString(value.toTime(), formatType);
break;
case QVariant::DateTime: {
const QDateTime dateTime = value.toDateTime();
text = locale.toString(dateTime.date(), formatType)
+ QLatin1Char(' ')
+ locale.toString(dateTime.time(), formatType);
break; }
case QVariant::DateTime:
text = locale.toString(value.toDateTime(), formatType);
break;
default:
text = value.toString();
if (role == Qt::DisplayRole)

View File

@ -220,6 +220,20 @@ private slots:
void QTBUG4435_keepSelectionOnCheck();
void QTBUG16469_textForRole();
void dateTextForRole_data();
void dateTextForRole();
#ifdef QT_BUILD_INTERNAL
private:
struct RoleDelegate : public QItemDelegate
{
QString textForRole(Qt::ItemDataRole role, const QVariant &value, const QLocale &locale)
{
QAbstractItemDelegatePrivate *d = reinterpret_cast<QAbstractItemDelegatePrivate *>(qGetPtrHelper(d_ptr));
return d->textForRole(role, value, locale);
}
};
#endif
};
@ -1530,14 +1544,7 @@ void tst_QItemDelegate::QTBUG16469_textForRole()
#ifndef QT_BUILD_INTERNAL
QSKIP("This test requires a developer build");
#else
struct TestDelegate : public QItemDelegate
{
QString textForRole(Qt::ItemDataRole role, const QVariant &value, const QLocale &locale)
{
QAbstractItemDelegatePrivate *d = reinterpret_cast<QAbstractItemDelegatePrivate *>(qGetPtrHelper(d_ptr));
return d->textForRole(role, value, locale);
}
} delegate;
RoleDelegate delegate;
QLocale locale;
const float f = 123.456f;
@ -1559,20 +1566,6 @@ void tst_QItemDelegate::QTBUG16469_textForRole()
QCOMPARE(delegate.textForRole(Qt::DisplayRole, ull, locale), locale.toString(ull));
QCOMPARE(delegate.textForRole(Qt::ToolTipRole, ull, locale), locale.toString(ull));
const QDateTime dateTime = QDateTime::currentDateTime();
const QDate date = dateTime.date();
const QTime time = dateTime.time();
const QString shortDate = locale.toString(date, QLocale::ShortFormat);
const QString longDate = locale.toString(date, QLocale::LongFormat);
const QString shortTime = locale.toString(time, QLocale::ShortFormat);
const QString longTime = locale.toString(time, QLocale::LongFormat);
QCOMPARE(delegate.textForRole(Qt::DisplayRole, date, locale), shortDate);
QCOMPARE(delegate.textForRole(Qt::ToolTipRole, date, locale), longDate);
QCOMPARE(delegate.textForRole(Qt::DisplayRole, time, locale), shortTime);
QCOMPARE(delegate.textForRole(Qt::ToolTipRole, time, locale), longTime);
QCOMPARE(delegate.textForRole(Qt::DisplayRole, dateTime, locale), shortDate + QLatin1Char(' ') + shortTime);
QCOMPARE(delegate.textForRole(Qt::ToolTipRole, dateTime, locale), longDate + QLatin1Char(' ') + longTime);
const QString text("text");
QCOMPARE(delegate.textForRole(Qt::DisplayRole, text, locale), text);
QCOMPARE(delegate.textForRole(Qt::ToolTipRole, text, locale), text);
@ -1584,6 +1577,41 @@ void tst_QItemDelegate::QTBUG16469_textForRole()
#endif
}
void tst_QItemDelegate::dateTextForRole_data()
{
#ifdef QT_BUILD_INTERNAL
QTest::addColumn<QDateTime>("when");
QTest::newRow("now") << QDateTime::currentDateTime(); // It's a local time
QDate date(2013, 12, 11);
QTime time(10, 9, 8, 765);
// Ensure we exercise every time-spec variant:
QTest::newRow("local") << QDateTime(date, time, Qt::LocalTime);
QTest::newRow("UTC") << QDateTime(date, time, Qt::UTC);
QTest::newRow("zone") << QDateTime(date, time, QTimeZone("Europe/Dublin"));
QTest::newRow("offset") << QDateTime(date, time, Qt::OffsetFromUTC, 36000);
#endif
}
void tst_QItemDelegate::dateTextForRole()
{
#ifndef QT_BUILD_INTERNAL
QSKIP("This test requires a developer build");
#else
QFETCH(QDateTime, when);
RoleDelegate delegate;
QLocale locale;
# define CHECK(value) \
QCOMPARE(delegate.textForRole(Qt::DisplayRole, value, locale), locale.toString(value, QLocale::ShortFormat)); \
QCOMPARE(delegate.textForRole(Qt::ToolTipRole, value, locale), locale.toString(value, QLocale::LongFormat))
CHECK(when);
CHECK(when.date());
CHECK(when.time());
# undef CHECK
#endif
}
// ### _not_ covered:
// editing with a custom editor factory