QListView: Fix Shift+click selection for non-default itemAlignment

QListView::setSelection() algorithm is designed for items to
occupy their cells completely, which is not the case when
itemAlignment is used. The middle part of the selection rect
goes beyond the column borders and extra items are selected.

Use the introduced cellRectForIndex() instead of rectForIndex()
to calculate the middle part correctly.

Fixes: QTBUG-73684
Change-Id: I4a1e42a056d56e85a16d8ae0ffe18b78d1d6deb7
Reviewed-by: Richard Moe Gustavsen <richard.gustavsen@qt.io>

3 files changed, 66 insertions, 3 deletions

diff --git a/src/widgets/itemviews/qlistview.cpp b/src/widgets/itemviews/qlistview.cpp
index 6b5857f1ca..e07514f297 100644
--- a/src/widgets/itemviews/qlistview.cpp
+++ b/src/widgets/itemviews/qlistview.cpp
@@ -1315,8 +1315,8 @@ void QListView::setSelection(const QRect &rect, QItemSelectionModel::SelectionFl
             if (tl.isValid() && br.isValid()
                 && d->isIndexEnabled(tl)
                 && d->isIndexEnabled(br)) {
-                QRect first = rectForIndex(tl);
-                QRect last = rectForIndex(br);
+                QRect first = d->cellRectForIndex(tl);
+                QRect last = d->cellRectForIndex(br);
                 QRect middle;
                 if (d->flow == LeftToRight) {
                     QRect &top = first;
diff --git a/src/widgets/itemviews/qlistview_p.h b/src/widgets/itemviews/qlistview_p.h
index 181386d4d0..c94357afe9 100644
--- a/src/widgets/itemviews/qlistview_p.h
+++ b/src/widgets/itemviews/qlistview_p.h
@@ -333,14 +333,31 @@ public:
     inline QModelIndex listViewItemToIndex(const QListViewItem &item) const
         { return model->index(commonListView->itemIndex(item), column, root); }
 
+    inline bool hasRectForIndex(const QModelIndex &index) const
+    {
+        return isIndexValid(index) && index.parent() == root && index.column() == column && !isHidden(index.row());
+    }
+
     QRect rectForIndex(const QModelIndex &index) const
     {
-        if (!isIndexValid(index) || index.parent() != root || index.column() != column || isHidden(index.row()))
+        if (!hasRectForIndex(index))
             return QRect();
         executePostedLayout();
         return viewItemRect(indexToListViewItem(index));
     }
 
+    QRect cellRectForIndex(const QModelIndex &index)
+    {
+        if (!hasRectForIndex(index))
+            return QRect();
+        executePostedLayout();
+        auto oldItemAlignment = itemAlignment;
+        itemAlignment = Qt::Alignment();
+        const QRect rect = rectForIndex(index);
+        itemAlignment = oldItemAlignment;
+        return rect;
+    }
+
     void viewUpdateGeometries() { q_func()->updateGeometries(); }
 
 
diff --git a/tests/auto/widgets/itemviews/qlistview/tst_qlistview.cpp b/tests/auto/widgets/itemviews/qlistview/tst_qlistview.cpp
index 9175c0bff4..9511654110 100644
--- a/tests/auto/widgets/itemviews/qlistview/tst_qlistview.cpp
+++ b/tests/auto/widgets/itemviews/qlistview/tst_qlistview.cpp
@@ -121,6 +121,7 @@ private slots:
     void task254449_draggingItemToNegativeCoordinates();
     void keyboardSearch();
     void shiftSelectionWithNonUniformItemSizes();
+    void shiftSelectionWithItemAlignment();
     void clickOnViewportClearsSelection();
     void task262152_setModelColumnNavigate();
     void taskQTBUG_2233_scrollHiddenItems_data();
@@ -1798,6 +1799,51 @@ void tst_QListView::shiftSelectionWithNonUniformItemSizes()
     }
 }
 
+void tst_QListView::shiftSelectionWithItemAlignment()
+{
+    QStringList items;
+    for (int c = 0; c < 2; c++) {
+        for (int i = 10; i > 0; i--)
+            items << QString(i, QLatin1Char('*'));
+
+        for (int i = 1; i < 11; i++)
+            items << QString(i, QLatin1Char('*'));
+    }
+
+    QListView view;
+    view.setFlow(QListView::TopToBottom);
+    view.setWrapping(true);
+    view.setItemAlignment(Qt::AlignLeft);
+    view.setSelectionMode(QAbstractItemView::ExtendedSelection);
+
+    QStringListModel model(items);
+    view.setModel(&model);
+
+    QFont font = view.font();
+    font.setPixelSize(10);
+    view.setFont(font);
+    view.resize(300, view.sizeHintForRow(0) * items.size() / 2 + view.horizontalScrollBar()->height());
+
+    view.show();
+    QApplication::setActiveWindow(&view);
+    QVERIFY(QTest::qWaitForWindowActive(&view));
+    QCOMPARE(static_cast<QWidget *>(&view), QApplication::activeWindow());
+
+    QModelIndex index1 = view.model()->index(items.size() / 4, 0);
+    QPoint p = view.visualRect(index1).center();
+    QVERIFY(view.viewport()->rect().contains(p));
+    QTest::mouseClick(view.viewport(), Qt::LeftButton, 0, p);
+    QCOMPARE(view.currentIndex(), index1);
+    QCOMPARE(view.selectionModel()->selectedIndexes().size(), 1);
+
+    QModelIndex index2 = view.model()->index(items.size() / 4 * 3, 0);
+    p = view.visualRect(index2).center();
+    QVERIFY(view.viewport()->rect().contains(p));
+    QTest::mouseClick(view.viewport(), Qt::LeftButton, Qt::ShiftModifier, p);
+    QCOMPARE(view.currentIndex(), index2);
+    QCOMPARE(view.selectionModel()->selectedIndexes().size(), index2.row() - index1.row() + 1);
+}
+
 void tst_QListView::clickOnViewportClearsSelection()
 {
     QStringList items;