Itemviews: start fixing mixups of int/enum for Qt's item roles

A model is supposed to return a Qt::CheckState for a CheckStateRole,
and a Qt::Alignment for a Qt::TextAlignmentRole. This is what the
documentation says (and what makes sense), but unfortunately
Qt's default delegate expected a plain `int` instead.
This sometimes worked (via QVariant conversions, e.g. when using a plain
enum) and sometimes didn't (e.g. when using a flag type).

This is confusing for end-users (and type unsafe, killing the whole
point of using enums and flags in the first place).

Adding some automatic flags<->int conversions through QVariant is
frowned upon, so I don't want to go there. Instead, add some private
convenience functions that extract either the right type from a variant,
or try to extract an `int` and convert it to the expected type.

Use these from within itemviews code.

Change-Id: I44bee98c4a26a1ef6c3b2fa1b8de2edfee7aef32
Pick-to: 6.2 6.3
Fixes: QTBUG-75172
Task-number: QTBUG-74639
Reviewed-by: David Faure <david.faure@kdab.com>
Reviewed-by: Richard Moe Gustavsen <richard.gustavsen@qt.io>
Reviewed-by: Volker Hilsheimer <volker.hilsheimer@qt.io>
This commit is contained in:
Giuseppe D'Angelo 2022-03-21 14:13:26 +01:00
parent af875e88f4
commit 6880b7c39b
5 changed files with 61 additions and 9 deletions

View File

@ -157,6 +157,52 @@ public:
};
Q_DECLARE_TYPEINFO(QAbstractItemModelPrivate::Change, Q_RELOCATABLE_TYPE);
namespace QtPrivate {
/*!
\internal
This is a workaround for QTBUG-75172.
Some predefined model roles are supposed to use certain enum/flag
types (e.g. fetching Qt::TextAlignmentRole is supposed to return a
variant containing a Qt::Alignment object).
For historical reasons, a plain `int` was used sometimes. This is
surprising to end-users and also sloppy on Qt's part; users were
forced to use `int` rather than the correct datatype.
This function tries both the "right" type and plain `int`, for a
given QVariant. This fixes the problem (using the correct datatype)
but also keeps compatibility with existing code using `int`.
### Qt 7: get rid of this. Always use the correct datatype.
*/
template <typename T>
T legacyEnumValueFromModelData(const QVariant &data)
{
static_assert(std::is_enum_v<T>);
if (data.userType() == qMetaTypeId<T>())
return data.value<T>();
else if (data.userType() == qMetaTypeId<int>())
return T(data.toInt());
return T();
}
template <typename T>
T legacyFlagValueFromModelData(const QVariant &data)
{
if (data.userType() == qMetaTypeId<T>())
return data.value<T>();
else if (data.userType() == qMetaTypeId<int>())
return T::fromInt(data.toInt());
return T();
}
} // namespace QtPrivate
QT_END_NAMESPACE
#endif // QABSTRACTITEMMODEL_P_H

View File

@ -55,6 +55,9 @@
#include "qabstractitemdelegate.h"
#include <private/qobject_p.h>
#include <qvariant.h>
#include <qmetatype.h>
QT_REQUIRE_CONFIG(itemviews);
QT_BEGIN_NAMESPACE

View File

@ -60,6 +60,7 @@
#endif
#include <private/qheaderview_p.h>
#include <private/qabstractitemmodel_p.h>
#include <private/qabstractitemdelegate_p.h>
#ifndef QT_NO_DATASTREAM
#include <qdatastream.h>
@ -2936,9 +2937,9 @@ void QHeaderView::initStyleOptionForIndex(QStyleOptionHeader *option, int logica
Qt::TextAlignmentRole);
opt.section = logicalIndex;
opt.state |= state;
opt.textAlignment = Qt::Alignment(textAlignment.isValid()
? Qt::Alignment(textAlignment.toInt())
: d->defaultAlignment);
opt.textAlignment = textAlignment.isValid()
? QtPrivate::legacyFlagValueFromModelData<Qt::Alignment>(textAlignment)
: d->defaultAlignment;
opt.iconAlignment = Qt::AlignVCenter;
opt.text = d->model->headerData(logicalIndex, d->orientation,

View File

@ -58,6 +58,7 @@
#include <qmetaobject.h>
#include <qtextlayout.h>
#include <private/qabstractitemdelegate_p.h>
#include <private/qabstractitemmodel_p.h>
#include <private/qtextengine_p.h>
#include <qdebug.h>
#include <qlocale.h>
@ -432,7 +433,7 @@ void QItemDelegate::paint(QPainter *painter,
Qt::CheckState checkState = Qt::Unchecked;
value = index.data(Qt::CheckStateRole);
if (value.isValid()) {
checkState = static_cast<Qt::CheckState>(value.toInt());
checkState = QtPrivate::legacyEnumValueFromModelData<Qt::CheckState>(value);
checkRect = doCheck(opt, opt.rect, value);
}
@ -1182,7 +1183,7 @@ bool QItemDelegate::editorEvent(QEvent *event,
return false;
}
Qt::CheckState state = static_cast<Qt::CheckState>(value.toInt());
Qt::CheckState state = QtPrivate::legacyEnumValueFromModelData<Qt::CheckState>(value);
if (flags & Qt::ItemIsUserTristate)
state = ((Qt::CheckState)((state + 1) % 3));
else
@ -1209,7 +1210,7 @@ QStyleOptionViewItem QItemDelegate::setOptions(const QModelIndex &index,
// set text alignment
value = index.data(Qt::TextAlignmentRole);
if (value.isValid())
opt.displayAlignment = Qt::Alignment(value.toInt());
opt.displayAlignment = QtPrivate::legacyFlagValueFromModelData<Qt::Alignment>(value);
// set foreground brush
value = index.data(Qt::ForegroundRole);

View File

@ -66,6 +66,7 @@
#include <qmetaobject.h>
#include <qtextlayout.h>
#include <private/qabstractitemdelegate_p.h>
#include <private/qabstractitemmodel_p.h>
#include <private/qtextengine_p.h>
#include <private/qlayoutengine_p.h>
#include <qdebug.h>
@ -302,7 +303,7 @@ void QStyledItemDelegate::initStyleOption(QStyleOptionViewItem *option,
value = modelRoleDataSpan.dataForRole(Qt::TextAlignmentRole);
if (value->isValid() && !value->isNull())
option->displayAlignment = Qt::Alignment(value->toInt());
option->displayAlignment = QtPrivate::legacyFlagValueFromModelData<Qt::Alignment>(*value);
value = modelRoleDataSpan.dataForRole(Qt::ForegroundRole);
if (value->canConvert<QBrush>())
@ -311,7 +312,7 @@ void QStyledItemDelegate::initStyleOption(QStyleOptionViewItem *option,
value = modelRoleDataSpan.dataForRole(Qt::CheckStateRole);
if (value->isValid() && !value->isNull()) {
option->features |= QStyleOptionViewItem::HasCheckIndicator;
option->checkState = static_cast<Qt::CheckState>(value->toInt());
option->checkState = QtPrivate::legacyEnumValueFromModelData<Qt::CheckState>(*value);
}
value = modelRoleDataSpan.dataForRole(Qt::DecorationRole);
@ -633,7 +634,7 @@ bool QStyledItemDelegate::editorEvent(QEvent *event,
return false;
}
Qt::CheckState state = static_cast<Qt::CheckState>(value.toInt());
Qt::CheckState state = QtPrivate::legacyEnumValueFromModelData<Qt::CheckState>(value);
if (flags & Qt::ItemIsUserTristate)
state = ((Qt::CheckState)((state + 1) % 3));
else