Fix a crash in help index filter

The crash appeared when Creator was built against Qt 6.2,
however, the Creator code for IndexFilterModel was
broken before and was working just by coincidence with Qt 5.15.

1. mapToSource() reimplementation creates now indices on source
   model side, instead of on proxy side.
2. filter() returns invalid model index now in case when
   list is unfiltered and empty and in case when the filter
   was used but we got empty result. Before it was creating
   a new valid index to (0, 0), but in fact we didn't have a
   corresponding valid data for it.
3. Reimplement headerData(), as otherwise the default implementation
   of this function inside proxy was calling erroneously index(0, 0)
   when we had empty data.
4. Reimplement hasChildren(), as otherwise the default implementation
   returned true for hidden root item, what caused a call to
   index(0, 0) in case we had empty data.
5. Assert that the index() is being called only for a valid
   range of rows that match our internal proxy data.
6. Add some checks whether sourceModel() isn't nullptr prior to use.
7. Simplify IndexWindow::filterIndices().

Fixes: QTCREATORBUG-26309
Change-Id: I5d150488893abd117b7d68c0436e4e511537738b
Reviewed-by: Eike Ziller <eike.ziller@qt.io>
This commit is contained in:
Jarek Kobus
2021-09-22 17:07:04 +02:00
parent 478ce99942
commit 71cfd0d2d0
2 changed files with 48 additions and 21 deletions

View File

@@ -34,6 +34,7 @@
#include <utils/fancylineedit.h> #include <utils/fancylineedit.h>
#include <utils/hostosinfo.h> #include <utils/hostosinfo.h>
#include <utils/navigationtreeview.h> #include <utils/navigationtreeview.h>
#include <utils/qtcassert.h>
#include <utils/styledbar.h> #include <utils/styledbar.h>
#include <QAbstractItemModel> #include <QAbstractItemModel>
@@ -51,7 +52,6 @@
#include <QHelpLink> #include <QHelpLink>
#endif #endif
using namespace Help::Internal; using namespace Help::Internal;
IndexWindow::IndexWindow() IndexWindow::IndexWindow()
@@ -118,15 +118,12 @@ void IndexWindow::setOpenInNewPageActionVisible(bool visible)
void IndexWindow::filterIndices(const QString &filter) void IndexWindow::filterIndices(const QString &filter)
{ {
QModelIndex bestMatch; const QString wildcard = filter.contains(QLatin1Char('*')) ? filter : QString();
if (filter.contains(QLatin1Char('*'))) const QModelIndex bestMatch = m_filteredIndexModel->filter(filter, wildcard);
bestMatch = m_filteredIndexModel->filter(filter, filter); if (!bestMatch.isValid())
else return;
bestMatch = m_filteredIndexModel->filter(filter, QString()); m_indexWidget->setCurrentIndex(bestMatch);
if (bestMatch.isValid()) { m_indexWidget->scrollTo(bestMatch);
m_indexWidget->setCurrentIndex(bestMatch);
m_indexWidget->scrollTo(bestMatch);
}
} }
bool IndexWindow::eventFilter(QObject *obj, QEvent *e) bool IndexWindow::eventFilter(QObject *obj, QEvent *e)
@@ -215,12 +212,15 @@ void IndexWindow::open(const QModelIndex &index, bool newPage)
Qt::DropActions IndexFilterModel::supportedDragActions() const Qt::DropActions IndexFilterModel::supportedDragActions() const
{ {
if (!sourceModel())
return Qt::IgnoreAction;
return sourceModel()->supportedDragActions(); return sourceModel()->supportedDragActions();
} }
QModelIndex IndexFilterModel::index(int row, int column, const QModelIndex &parent) const QModelIndex IndexFilterModel::index(int row, int column, const QModelIndex &parent) const
{ {
Q_UNUSED(parent) Q_UNUSED(parent)
QTC_ASSERT(row < m_toSource.size(), return {});
return createIndex(row, column); return createIndex(row, column);
} }
@@ -232,16 +232,31 @@ QModelIndex IndexFilterModel::parent(const QModelIndex &child) const
int IndexFilterModel::rowCount(const QModelIndex &parent) const int IndexFilterModel::rowCount(const QModelIndex &parent) const
{ {
if (parent.isValid()) if (parent.isValid()) // our items don't have children
return 0; return 0;
return m_toSource.size(); return m_toSource.size();
} }
int IndexFilterModel::columnCount(const QModelIndex &parent) const int IndexFilterModel::columnCount(const QModelIndex &parent) const
{ {
if (!sourceModel())
return 0;
return sourceModel()->columnCount(mapToSource(parent)); return sourceModel()->columnCount(mapToSource(parent));
} }
QVariant IndexFilterModel::headerData(int section, Qt::Orientation orientation, int role) const
{
// we don't show header
return {};
}
bool IndexFilterModel::hasChildren(const QModelIndex &parent) const
{
if (parent.isValid()) // our items don't have children
return false;
return m_toSource.count();
}
void IndexFilterModel::setSourceModel(QAbstractItemModel *sm) void IndexFilterModel::setSourceModel(QAbstractItemModel *sm)
{ {
QAbstractItemModel *previousModel = sourceModel(); QAbstractItemModel *previousModel = sourceModel();
@@ -293,6 +308,11 @@ QModelIndex IndexFilterModel::filter(const QString &filter, const QString &wildc
m_wildcard = wildcard; m_wildcard = wildcard;
m_toSource.clear(); m_toSource.clear();
if (!sourceModel()) {
endResetModel();
return {};
}
// adapted copy from QHelpIndexModel // adapted copy from QHelpIndexModel
if (filter.isEmpty() && wildcard.isEmpty()) { if (filter.isEmpty() && wildcard.isEmpty()) {
@@ -301,6 +321,8 @@ QModelIndex IndexFilterModel::filter(const QString &filter, const QString &wildc
for (int i = 0; i < count; ++i) for (int i = 0; i < count; ++i)
m_toSource.append(i); m_toSource.append(i);
endResetModel(); endResetModel();
if (m_toSource.isEmpty())
return {};
return index(0, 0); return index(0, 0);
} }
@@ -330,7 +352,7 @@ QModelIndex IndexFilterModel::filter(const QString &filter, const QString &wildc
} }
} else { } else {
int i = 0; int i = 0;
foreach (const QString &index, indices) { for (const QString &index : indices) {
if (index.contains(filter, Qt::CaseInsensitive)) { if (index.contains(filter, Qt::CaseInsensitive)) {
m_toSource.append(i); m_toSource.append(i);
if (perfectMatch == -1 && index.startsWith(filter, Qt::CaseInsensitive)) { if (perfectMatch == -1 && index.startsWith(filter, Qt::CaseInsensitive)) {
@@ -347,27 +369,30 @@ QModelIndex IndexFilterModel::filter(const QString &filter, const QString &wildc
} }
} }
endResetModel();
if (goodMatch == -1)
return {};
if (perfectMatch == -1) if (perfectMatch == -1)
perfectMatch = qMax(0, goodMatch); perfectMatch = qMax(0, goodMatch);
return index(perfectMatch, 0);
endResetModel();
return index(perfectMatch, 0, QModelIndex());
} }
QModelIndex IndexFilterModel::mapToSource(const QModelIndex &proxyIndex) const QModelIndex IndexFilterModel::mapToSource(const QModelIndex &proxyIndex) const
{ {
if (!proxyIndex.isValid() || proxyIndex.parent().isValid() || proxyIndex.row() >= m_toSource.size()) if (!sourceModel() || !proxyIndex.isValid()
return QModelIndex(); || proxyIndex.parent().isValid() || proxyIndex.row() >= m_toSource.size()) {
return index(m_toSource.at(proxyIndex.row()), proxyIndex.column()); return {};
}
return sourceModel()->index(m_toSource.at(proxyIndex.row()), proxyIndex.column());
} }
QModelIndex IndexFilterModel::mapFromSource(const QModelIndex &sourceIndex) const QModelIndex IndexFilterModel::mapFromSource(const QModelIndex &sourceIndex) const
{ {
if (!sourceIndex.isValid() || sourceIndex.parent().isValid()) if (!sourceIndex.isValid() || sourceIndex.parent().isValid())
return QModelIndex(); return {};
int i = m_toSource.indexOf(sourceIndex.row()); const int i = m_toSource.indexOf(sourceIndex.row());
if (i < 0) if (i < 0)
return QModelIndex(); return {};
return index(i, sourceIndex.column()); return index(i, sourceIndex.column());
} }

View File

@@ -58,6 +58,8 @@ public:
QModelIndex parent(const QModelIndex &child) const override; QModelIndex parent(const QModelIndex &child) const override;
int rowCount(const QModelIndex &parent = QModelIndex()) const override; int rowCount(const QModelIndex &parent = QModelIndex()) const override;
int columnCount(const QModelIndex &parent = QModelIndex()) const override; int columnCount(const QModelIndex &parent = QModelIndex()) const override;
QVariant headerData(int section, Qt::Orientation orientation, int role) const override;
bool hasChildren(const QModelIndex &parent = QModelIndex()) const override;
void setSourceModel(QAbstractItemModel *sm) override; void setSourceModel(QAbstractItemModel *sm) override;