From 71cfd0d2d0e21ed35ca11ebb33a0c4bc9ce2e4bc Mon Sep 17 00:00:00 2001 From: Jarek Kobus Date: Wed, 22 Sep 2021 17:07:04 +0200 Subject: [PATCH] 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 --- src/shared/help/indexwindow.cpp | 67 ++++++++++++++++++++++----------- src/shared/help/indexwindow.h | 2 + 2 files changed, 48 insertions(+), 21 deletions(-) diff --git a/src/shared/help/indexwindow.cpp b/src/shared/help/indexwindow.cpp index 88e44f4b0e0..dd6a9ed1ac1 100644 --- a/src/shared/help/indexwindow.cpp +++ b/src/shared/help/indexwindow.cpp @@ -34,6 +34,7 @@ #include #include #include +#include #include #include @@ -51,7 +52,6 @@ #include #endif - using namespace Help::Internal; IndexWindow::IndexWindow() @@ -118,15 +118,12 @@ void IndexWindow::setOpenInNewPageActionVisible(bool visible) void IndexWindow::filterIndices(const QString &filter) { - QModelIndex bestMatch; - if (filter.contains(QLatin1Char('*'))) - bestMatch = m_filteredIndexModel->filter(filter, filter); - else - bestMatch = m_filteredIndexModel->filter(filter, QString()); - if (bestMatch.isValid()) { - m_indexWidget->setCurrentIndex(bestMatch); - m_indexWidget->scrollTo(bestMatch); - } + const QString wildcard = filter.contains(QLatin1Char('*')) ? filter : QString(); + const QModelIndex bestMatch = m_filteredIndexModel->filter(filter, wildcard); + if (!bestMatch.isValid()) + return; + m_indexWidget->setCurrentIndex(bestMatch); + m_indexWidget->scrollTo(bestMatch); } bool IndexWindow::eventFilter(QObject *obj, QEvent *e) @@ -215,12 +212,15 @@ void IndexWindow::open(const QModelIndex &index, bool newPage) Qt::DropActions IndexFilterModel::supportedDragActions() const { + if (!sourceModel()) + return Qt::IgnoreAction; return sourceModel()->supportedDragActions(); } QModelIndex IndexFilterModel::index(int row, int column, const QModelIndex &parent) const { Q_UNUSED(parent) + QTC_ASSERT(row < m_toSource.size(), return {}); return createIndex(row, column); } @@ -232,16 +232,31 @@ QModelIndex IndexFilterModel::parent(const QModelIndex &child) const int IndexFilterModel::rowCount(const QModelIndex &parent) const { - if (parent.isValid()) + if (parent.isValid()) // our items don't have children return 0; return m_toSource.size(); } int IndexFilterModel::columnCount(const QModelIndex &parent) const { + if (!sourceModel()) + return 0; 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) { QAbstractItemModel *previousModel = sourceModel(); @@ -293,6 +308,11 @@ QModelIndex IndexFilterModel::filter(const QString &filter, const QString &wildc m_wildcard = wildcard; m_toSource.clear(); + if (!sourceModel()) { + endResetModel(); + return {}; + } + // adapted copy from QHelpIndexModel 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) m_toSource.append(i); endResetModel(); + if (m_toSource.isEmpty()) + return {}; return index(0, 0); } @@ -330,7 +352,7 @@ QModelIndex IndexFilterModel::filter(const QString &filter, const QString &wildc } } else { int i = 0; - foreach (const QString &index, indices) { + for (const QString &index : indices) { if (index.contains(filter, Qt::CaseInsensitive)) { m_toSource.append(i); 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) perfectMatch = qMax(0, goodMatch); - - endResetModel(); - return index(perfectMatch, 0, QModelIndex()); + return index(perfectMatch, 0); } QModelIndex IndexFilterModel::mapToSource(const QModelIndex &proxyIndex) const { - if (!proxyIndex.isValid() || proxyIndex.parent().isValid() || proxyIndex.row() >= m_toSource.size()) - return QModelIndex(); - return index(m_toSource.at(proxyIndex.row()), proxyIndex.column()); + if (!sourceModel() || !proxyIndex.isValid() + || proxyIndex.parent().isValid() || proxyIndex.row() >= m_toSource.size()) { + return {}; + } + return sourceModel()->index(m_toSource.at(proxyIndex.row()), proxyIndex.column()); } QModelIndex IndexFilterModel::mapFromSource(const QModelIndex &sourceIndex) const { if (!sourceIndex.isValid() || sourceIndex.parent().isValid()) - return QModelIndex(); - int i = m_toSource.indexOf(sourceIndex.row()); + return {}; + const int i = m_toSource.indexOf(sourceIndex.row()); if (i < 0) - return QModelIndex(); + return {}; return index(i, sourceIndex.column()); } diff --git a/src/shared/help/indexwindow.h b/src/shared/help/indexwindow.h index 5fe0712dca1..ed872231de7 100644 --- a/src/shared/help/indexwindow.h +++ b/src/shared/help/indexwindow.h @@ -58,6 +58,8 @@ public: QModelIndex parent(const QModelIndex &child) const override; int rowCount(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;