From 980cd76e6543e6a89cdd4454eee7ca151d6ee76b Mon Sep 17 00:00:00 2001 From: Volker Hilsheimer Date: Mon, 15 Apr 2024 17:00:19 +0200 Subject: [PATCH] QTableView: fix infinite loop when resizing rows/columns on model reset When a slot is connected to the modelReset signal before the model is set on the view, then such a slot might try to resize the columns or rows to fit the contents before the view had a chance to perform layout work. In that case, the while-loop that calculated the size hint of a rows (or column) by checking the size hint of each cell might not exit, as cached information about visual indexes would be invalid. To prevent this, don't loop over the cells if the actual last row/column of the model (after the reset) is less than the first (cached) visible row/column. Also, make sure we increase the counter for number of rows/columns processed with each iteration. Add a test (that loops infinitely without the fix, resulting in the test being killed by the framework), and make the 'actual' variables constant for clarity. Fixes: QTBUG-124301 Change-Id: I0adb2f1e8a1e78760ef7c19e9686c9572eca8be6 Reviewed-by: Axel Spoerl (cherry picked from commit 960867566a75931c338f67cfe429c0756f41ad89) Reviewed-by: Qt Cherry-pick Bot --- src/widgets/itemviews/qtableview.cpp | 30 +++++++++---------- .../itemviews/qtableview/tst_qtableview.cpp | 30 +++++++++++++++++++ 2 files changed, 44 insertions(+), 16 deletions(-) diff --git a/src/widgets/itemviews/qtableview.cpp b/src/widgets/itemviews/qtableview.cpp index 6f9c32090e5..5726348bc55 100644 --- a/src/widgets/itemviews/qtableview.cpp +++ b/src/widgets/itemviews/qtableview.cpp @@ -2432,12 +2432,12 @@ int QTableView::sizeHintForRow(int row) const break; } - int actualRight = d->model->columnCount(d->root) - 1; + const int actualRight = d->model->columnCount(d->root) - 1; int idxLeft = left; int idxRight = column - 1; - if (maximumProcessCols == 0) - columnsProcessed = 0; // skip the while loop + if (maximumProcessCols == 0 || actualRight < idxLeft) + columnsProcessed = maximumProcessCols; // skip the while loop while (columnsProcessed != maximumProcessCols && (idxLeft > 0 || idxRight < actualRight)) { int logicalIdx = -1; @@ -2461,11 +2461,10 @@ int QTableView::sizeHintForRow(int row) const break; } } - if (logicalIdx < 0) - continue; - - index = d->model->index(row, logicalIdx, d->root); - hint = d->heightHintForIndex(index, hint, option); + if (logicalIdx >= 0) { + index = d->model->index(row, logicalIdx, d->root); + hint = d->heightHintForIndex(index, hint, option); + } ++columnsProcessed; } @@ -2521,12 +2520,12 @@ int QTableView::sizeHintForColumn(int column) const break; } - int actualBottom = d->model->rowCount(d->root) - 1; + const int actualBottom = d->model->rowCount(d->root) - 1; int idxTop = top; int idxBottom = row - 1; - if (maximumProcessRows == 0) - rowsProcessed = 0; // skip the while loop + if (maximumProcessRows == 0 || actualBottom < idxTop) + rowsProcessed = maximumProcessRows; // skip the while loop while (rowsProcessed != maximumProcessRows && (idxTop > 0 || idxBottom < actualBottom)) { int logicalIdx = -1; @@ -2550,11 +2549,10 @@ int QTableView::sizeHintForColumn(int column) const break; } } - if (logicalIdx < 0) - continue; - - index = d->model->index(logicalIdx, column, d->root); - hint = d->widthHintForIndex(index, hint, option); + if (logicalIdx >= 0) { + index = d->model->index(logicalIdx, column, d->root); + hint = d->widthHintForIndex(index, hint, option); + } ++rowsProcessed; } diff --git a/tests/auto/widgets/itemviews/qtableview/tst_qtableview.cpp b/tests/auto/widgets/itemviews/qtableview/tst_qtableview.cpp index 55de7d6b234..45a467d4232 100644 --- a/tests/auto/widgets/itemviews/qtableview/tst_qtableview.cpp +++ b/tests/auto/widgets/itemviews/qtableview/tst_qtableview.cpp @@ -391,6 +391,7 @@ private slots: void resizeToContents(); void resizeToContentsSpans(); + void resizeToContentsEarly(); void tabFocus(); void bigModel(); @@ -3777,6 +3778,35 @@ void tst_QTableView::resizeToContentsSpans() QCOMPARE(view2.columnWidth(0), view3.columnWidth(0) - view2.columnWidth(1)); } +void tst_QTableView::resizeToContentsEarly() +{ + QStringListModel model; + QTableView view; + + // connect to the model before setting it on the view + connect(&model, &QStringListModel::modelReset, &model, [&view]{ + view.resizeColumnsToContents(); + }); + connect(&model, &QStringListModel::modelReset, &model, [&view]{ + view.resizeRowsToContents(); + }); + + // the view only connects now to the model's signals, so responds to the + // reset signal *after* the lambdas above + view.setModel(&model); + + QStringList data(200, QString("Hello World")); + model.setStringList(data); + + view.show(); + QVERIFY(QTest::qWaitForWindowExposed(&view)); + + view.verticalScrollBar()->setValue(view.verticalScrollBar()->maximum()); + + data = data.sliced(data.size() / 2); + model.setStringList(data); +} + QT_BEGIN_NAMESPACE extern bool Q_WIDGETS_EXPORT qt_tab_all_widgets(); // qapplication.cpp QT_END_NAMESPACE