QComboBox: reset indexBeforeChange to -1 if index is invalidated

The member variable indexBeforeChange is member-initialized with -1 and
changed to the current row, if a valid index is set.
It is used to check, if the model has been reset to an empty model
and/or the index has been invalidated. The result is used to decide, if
the currentIndexChanged signal is emitted or not.

If a combo box had a valid index and it is invalidated afterwards
(e.g. because the combobox has no more entries), indexBeforeChange is
not reset to -1. The redundant signal emission is therefore prevented
only the first time.

This patch resets indexBeforeChange if the index is invalidated or the
last item is removed from the combo box.

It also adds a no-op check to tst_QComboBox::currentIndex.
The test data sets "check that setting the index to -1 works" and
"check that current index is invalid when removing the only item" check
the respective use cases.

As a drive-by, variable names and QObect::connect syntax have been
cleaned up in tst_QComboBox::currentIndex.

Fixes: QTBUG-108614
Pick-to: 6.5 6.2
Change-Id: Ib6dfa887d9247f2c47df065039d69ba57c32fa24
Reviewed-by: Richard Moe Gustavsen <richard.gustavsen@qt.io>
This commit is contained in:
Axel Spoerl 2023-03-22 14:34:40 +01:00
parent c4f59f96b9
commit c4b62ee501
2 changed files with 55 additions and 39 deletions

View File

@ -1121,6 +1121,12 @@ void QComboBoxPrivate::_q_rowsRemoved(const QModelIndex &parent, int /*start*/,
q->updateGeometry();
}
// model has removed the last row
if (model->rowCount(root) == 0) {
setCurrentIndex(QModelIndex());
return;
}
// model has changed the currentIndex
if (currentIndex.row() != indexBeforeChange) {
if (!currentIndex.isValid() && q->count()) {
@ -2127,10 +2133,15 @@ void QComboBoxPrivate::setCurrentIndex(const QModelIndex &mi)
}
updateLineEditGeometry();
}
// If the model was reset to an empty, currentIndex will be invalidated
// If the model was reset to an empty one, currentIndex will be invalidated
// (because it's a QPersistentModelIndex), but the index change will never
// be advertised. So we need an explicit check for such condition.
// be advertised. So an explicit check for this condition is needed.
// The variable used for that check has to be reset when a previously valid
// index becomes invalid.
const bool modelResetToEmpty = !normalized.isValid() && indexBeforeChange != -1;
if (modelResetToEmpty)
indexBeforeChange = -1;
if (indexChanged || modelResetToEmpty) {
q->update();
_q_emitCurrentIndexChanged(currentIndex);

View File

@ -1018,7 +1018,7 @@ void tst_QComboBox::currentIndex_data()
expectedCurrentIndex = -1;
expectedCurrentText = "";
expectedSignalCount = 2;
QTest::newRow("check that isetting the index to -1 works")
QTest::newRow("check that setting the index to -1 works")
<< initialItems << setCurrentIndex << removeIndex
<< insertIndex << insertText << expectedCurrentIndex << expectedCurrentText
<< expectedSignalCount;
@ -1161,66 +1161,71 @@ void tst_QComboBox::currentIndex()
TestWidget topLevel;
topLevel.show();
QVERIFY(QTest::qWaitForWindowExposed(&topLevel));
QComboBox *testWidget = topLevel.comboBox();
QComboBox *comboBox = topLevel.comboBox();
// test both editable/non-editable combobox
for (int edit = 0; edit < 2; ++edit) {
testWidget->clear();
testWidget->setEditable(edit ? true : false);
comboBox->clear();
comboBox->setEditable(edit ? true : false);
if (edit)
QVERIFY(testWidget->lineEdit());
QVERIFY(comboBox->lineEdit());
// verify it is empty, has no current index and no current text
QCOMPARE(testWidget->count(), 0);
QCOMPARE(testWidget->currentIndex(), -1);
QVERIFY(testWidget->currentText().isEmpty());
QCOMPARE(comboBox->count(), 0);
QCOMPARE(comboBox->currentIndex(), -1);
QVERIFY(comboBox->currentText().isEmpty());
// spy on currentIndexChanged
QSignalSpy indexChangedInt(testWidget, SIGNAL(currentIndexChanged(int)));
QSignalSpy indexChangedSpy(comboBox, &QComboBox::currentIndexChanged);
// stuff items into it
foreach(QString text, initialItems) {
testWidget->addItem(text);
}
QCOMPARE(testWidget->count(), initialItems.size());
for (const QString &text : initialItems)
comboBox->addItem(text);
QCOMPARE(comboBox->count(), initialItems.size());
// set current index, remove and/or insert
if (setCurrentIndex >= -1) {
testWidget->setCurrentIndex(setCurrentIndex);
QCOMPARE(testWidget->currentIndex(), setCurrentIndex);
comboBox->setCurrentIndex(setCurrentIndex);
QCOMPARE(comboBox->currentIndex(), setCurrentIndex);
}
if (removeIndex >= 0)
testWidget->removeItem(removeIndex);
comboBox->removeItem(removeIndex);
if (insertIndex >= 0)
testWidget->insertItem(insertIndex, insertText);
comboBox->insertItem(insertIndex, insertText);
// compare with expected index and text
QCOMPARE(testWidget->currentIndex(), expectedCurrentIndex);
QCOMPARE(testWidget->currentText(), expectedCurrentText);
QCOMPARE(comboBox->currentIndex(), expectedCurrentIndex);
QCOMPARE(comboBox->currentText(), expectedCurrentText);
// check that signal count is correct
QCOMPARE(indexChangedInt.size(), expectedSignalCount);
QCOMPARE(indexChangedSpy.size(), expectedSignalCount);
// compare with last sent signal values
if (indexChangedInt.size())
QCOMPARE(indexChangedInt.at(indexChangedInt.size() - 1).at(0).toInt(),
testWidget->currentIndex());
if (indexChangedSpy.size())
QCOMPARE(indexChangedSpy.at(indexChangedSpy.size() - 1).at(0).toInt(),
comboBox->currentIndex());
// Test a no-op index change
const int index = comboBox->currentIndex();
comboBox->setCurrentIndex(index);
QCOMPARE(indexChangedSpy.size(), expectedSignalCount);
if (edit) {
testWidget->setCurrentIndex(-1);
testWidget->setInsertPolicy(QComboBox::InsertAtBottom);
QTest::keyPress(testWidget, 'a');
QTest::keyPress(testWidget, 'b');
QCOMPARE(testWidget->currentText(), QString("ab"));
QCOMPARE(testWidget->currentIndex(), -1);
int numItems = testWidget->count();
QTest::keyPress(testWidget, Qt::Key_Return);
QCOMPARE(testWidget->count(), numItems + 1);
QCOMPARE(testWidget->currentIndex(), numItems);
testWidget->setCurrentIndex(-1);
QTest::keyPress(testWidget, 'a');
QTest::keyPress(testWidget, 'b');
QCOMPARE(testWidget->currentIndex(), -1);
comboBox->setCurrentIndex(-1);
comboBox->setInsertPolicy(QComboBox::InsertAtBottom);
QTest::keyPress(comboBox, 'a');
QTest::keyPress(comboBox, 'b');
QCOMPARE(comboBox->currentText(), QString("ab"));
QCOMPARE(comboBox->currentIndex(), -1);
int numItems = comboBox->count();
QTest::keyPress(comboBox, Qt::Key_Return);
QCOMPARE(comboBox->count(), numItems + 1);
QCOMPARE(comboBox->currentIndex(), numItems);
comboBox->setCurrentIndex(-1);
QTest::keyPress(comboBox, 'a');
QTest::keyPress(comboBox, 'b');
QCOMPARE(comboBox->currentIndex(), -1);
}
}
}