QTable/TreeView: fix sortByColumn() when view is already sorted

When the view was already sorted by the column and order given to
sortByColumn(), a resort was not triggered which is a regression since
d0f909f8dbdd8594b0d950822f0e7ab8728da513.
Therefore don't rely on sortIndicatorChanged() in this case and trigger
an explicit sort as it's done when no user-triggered sorting is enabled

Fixes: QTBUG-86268
Change-Id: I3ec30ab81f304d5a19cef6d67b8a81c21b4a112d
Pick-to: 5.15
Reviewed-by: Richard Moe Gustavsen <richard.gustavsen@qt.io>

4 files changed, 77 insertions, 8 deletions

diff --git a/src/widgets/itemviews/qtableview.cpp b/src/widgets/itemviews/qtableview.cpp
index 30f2eba912..cf927ef30d 100644
--- a/src/widgets/itemviews/qtableview.cpp
+++ b/src/widgets/itemviews/qtableview.cpp
@@ -3238,11 +3238,11 @@ void QTableView::sortByColumn(int column, Qt::SortOrder order)
     Q_D(QTableView);
     if (column < -1)
         return;
-    // If sorting is enabled it will emit a signal connected to
-    // _q_sortIndicatorChanged, which then actually sorts
     d->horizontalHeader->setSortIndicator(column, order);
-    // If sorting is not enabled, force to sort now
-    if (!d->sortingEnabled)
+    // If sorting is not enabled or has the same order as before, force to sort now
+    // else sorting will be trigger through sortIndicatorChanged()
+    if (!d->sortingEnabled ||
+        (d->horizontalHeader->sortIndicatorSection() == column && d->horizontalHeader->sortIndicatorOrder() == order))
         d->model->sort(column, order);
 }
 
diff --git a/src/widgets/itemviews/qtreeview.cpp b/src/widgets/itemviews/qtreeview.cpp
index 863a26db4b..ccfeffeda3 100644
--- a/src/widgets/itemviews/qtreeview.cpp
+++ b/src/widgets/itemviews/qtreeview.cpp
@@ -2629,11 +2629,11 @@ void QTreeView::sortByColumn(int column, Qt::SortOrder order)
     Q_D(QTreeView);
     if (column < -1)
         return;
-    // If sorting is enabled it will emit a signal connected to
-    // _q_sortIndicatorChanged, which then actually sorts
     d->header->setSortIndicator(column, order);
-    // If sorting is not enabled, force to sort now
-    if (!d->sortingEnabled)
+    // If sorting is not enabled or has the same order as before, force to sort now
+    // else sorting will be trigger through sortIndicatorChanged()
+    if (!d->sortingEnabled ||
+        (d->header->sortIndicatorSection() == column && d->header->sortIndicatorOrder() == order))
         d->model->sort(column, order);
 }
 
diff --git a/tests/auto/widgets/itemviews/qtableview/tst_qtableview.cpp b/tests/auto/widgets/itemviews/qtableview/tst_qtableview.cpp
index 74f480c318..37f37eaac8 100644
--- a/tests/auto/widgets/itemviews/qtableview/tst_qtableview.cpp
+++ b/tests/auto/widgets/itemviews/qtableview/tst_qtableview.cpp
@@ -384,6 +384,9 @@ private slots:
     void sortingEnabled_data();
     void sortingEnabled();
 
+    void sortByColumn_data();
+    void sortByColumn();
+
     void scrollTo_data();
     void scrollTo();
 
@@ -2681,6 +2684,64 @@ void tst_QTableView::sortingEnabled()
 //    QFETCH(int, columnCount);
 }
 
+void tst_QTableView::sortByColumn_data()
+{
+    QTest::addColumn<bool>("sortingEnabled");
+    QTest::newRow("sorting enabled") << true;
+    QTest::newRow("sorting disabled") << false;
+}
+
+// Checks sorting and that sortByColumn also sets the sortIndicator
+void tst_QTableView::sortByColumn()
+{
+    QFETCH(bool, sortingEnabled);
+    QTableView view;
+    QStandardItemModel model(4, 2);
+    QSortFilterProxyModel sfpm; // default QStandardItemModel does not support 'unsorted' state
+    sfpm.setSourceModel(&model);
+    model.setItem(0, 0, new QStandardItem("b"));
+    model.setItem(1, 0, new QStandardItem("d"));
+    model.setItem(2, 0, new QStandardItem("c"));
+    model.setItem(3, 0, new QStandardItem("a"));
+    model.setItem(0, 1, new QStandardItem("e"));
+    model.setItem(1, 1, new QStandardItem("g"));
+    model.setItem(2, 1, new QStandardItem("h"));
+    model.setItem(3, 1, new QStandardItem("f"));
+
+    view.setSortingEnabled(sortingEnabled);
+    view.setModel(&sfpm);
+    view.show();
+
+    view.sortByColumn(1, Qt::DescendingOrder);
+    QCOMPARE(view.horizontalHeader()->sortIndicatorSection(), 1);
+    QCOMPARE(view.model()->data(view.model()->index(0, 0)).toString(), QString::fromLatin1("c"));
+    QCOMPARE(view.model()->data(view.model()->index(1, 0)).toString(), QString::fromLatin1("d"));
+    QCOMPARE(view.model()->data(view.model()->index(0, 1)).toString(), QString::fromLatin1("h"));
+    QCOMPARE(view.model()->data(view.model()->index(1, 1)).toString(), QString::fromLatin1("g"));
+
+    view.sortByColumn(0, Qt::AscendingOrder);
+    QCOMPARE(view.horizontalHeader()->sortIndicatorSection(), 0);
+    QCOMPARE(view.model()->data(view.model()->index(0, 0)).toString(), QString::fromLatin1("a"));
+    QCOMPARE(view.model()->data(view.model()->index(1, 0)).toString(), QString::fromLatin1("b"));
+    QCOMPARE(view.model()->data(view.model()->index(0, 1)).toString(), QString::fromLatin1("f"));
+    QCOMPARE(view.model()->data(view.model()->index(1, 1)).toString(), QString::fromLatin1("e"));
+
+    view.sortByColumn(-1, Qt::AscendingOrder);
+    QCOMPARE(view.horizontalHeader()->sortIndicatorSection(), -1);
+    QCOMPARE(view.model()->data(view.model()->index(0, 0)).toString(), QString::fromLatin1("b"));
+    QCOMPARE(view.model()->data(view.model()->index(1, 0)).toString(), QString::fromLatin1("d"));
+    QCOMPARE(view.model()->data(view.model()->index(0, 1)).toString(), QString::fromLatin1("e"));
+    QCOMPARE(view.model()->data(view.model()->index(1, 1)).toString(), QString::fromLatin1("g"));
+
+    // a new 'sortByColumn()' should do a re-sort (e.g. due to the data changed), QTBUG-86268
+    view.setModel(&model);
+    view.sortByColumn(0, Qt::AscendingOrder);
+    QCOMPARE(view.model()->data(view.model()->index(0, 0)).toString(), QString::fromLatin1("a"));
+    model.setItem(0, 0, new QStandardItem("x"));
+    view.sortByColumn(0, Qt::AscendingOrder);
+    QCOMPARE(view.model()->data(view.model()->index(0, 0)).toString(), QString::fromLatin1("b"));
+}
+
 void tst_QTableView::scrollTo_data()
 {
     QTest::addColumn<QAbstractItemView::ScrollMode>("verticalScrollMode");
diff --git a/tests/auto/widgets/itemviews/qtreeview/tst_qtreeview.cpp b/tests/auto/widgets/itemviews/qtreeview/tst_qtreeview.cpp
index 3445e1edef..94f3e52ff3 100644
--- a/tests/auto/widgets/itemviews/qtreeview/tst_qtreeview.cpp
+++ b/tests/auto/widgets/itemviews/qtreeview/tst_qtreeview.cpp
@@ -2852,6 +2852,14 @@ void tst_QTreeView::sortByColumn()
     QCOMPARE(view.model()->data(view.model()->index(1, 0)).toString(), QString::fromLatin1("d"));
     QCOMPARE(view.model()->data(view.model()->index(0, 1)).toString(), QString::fromLatin1("e"));
     QCOMPARE(view.model()->data(view.model()->index(1, 1)).toString(), QString::fromLatin1("g"));
+
+    // a new 'sortByColumn()' should do a re-sort (e.g. due to the data changed), QTBUG-86268
+    view.setModel(&model);
+    view.sortByColumn(0, Qt::AscendingOrder);
+    QCOMPARE(view.model()->data(view.model()->index(0, 0)).toString(), QString::fromLatin1("a"));
+    model.setItem(0, 0, new QStandardItem("x"));
+    view.sortByColumn(0, Qt::AscendingOrder);
+    QCOMPARE(view.model()->data(view.model()->index(0, 0)).toString(), QString::fromLatin1("b"));
 }
 
 /*