From ac80cee846e270a42ce736cac80ca865321b5422 Mon Sep 17 00:00:00 2001
From: Volker Hilsheimer <volker.hilsheimer@qt.io>
Date: Fri, 10 Jun 2022 22:44:19 +0200
Subject: [PATCH] Don't crash when removing the last visible tab

The code incorrectly tried to ensure that the firstVisible tab was a
valid index, even though there might not be any visible tab left after
removing the last visible tab.

The same logic didn't exist of the lastVisible tab, so we tripped the
assert in qBound, as max (being -1) ended up smaller than min (0).

Fix this by removing the wrong correcting of firstVisible to be always
valid. Make sure we emit currentChanged with -1 when no visible tab is
left after removing the current tab.

Add a test.

Fixes: QTBUG-104003
Pick-to: 6.3 6.2
Change-Id: I27e6438a02d0a0f1ac4d0e0160cee4f33b3f3766
Reviewed-by: Richard Moe Gustavsen <richard.gustavsen@qt.io>
---
 src/widgets/widgets/qtabbar.cpp               |  7 ++--
 .../widgets/widgets/qtabbar/tst_qtabbar.cpp   | 34 +++++++++++++++++++
 2 files changed, 37 insertions(+), 4 deletions(-)

diff --git a/src/widgets/widgets/qtabbar.cpp b/src/widgets/widgets/qtabbar.cpp
index ed0b5f2f210..6dd7babbc8a 100644
--- a/src/widgets/widgets/qtabbar.cpp
+++ b/src/widgets/widgets/qtabbar.cpp
@@ -1052,8 +1052,6 @@ void QTabBar::removeTab(int index)
                     break;
                 case SelectLeftTab:
                     newIndex = qBound(d->firstVisible, index-1, d->lastVisible);
-                    if (newIndex < 0)
-                        newIndex = 0;
                     break;
                 default:
                     break;
@@ -1064,6 +1062,9 @@ void QTabBar::removeTab(int index)
                     int bump = d->tabList.at(newIndex)->lastTab;
                     setCurrentIndex(newIndex);
                     d->tabList.at(newIndex)->lastTab = bump;
+                } else {
+                    // we had a valid current index, but there are no visible tabs left
+                    emit currentChanged(-1);
                 }
             } else {
                 emit currentChanged(-1);
@@ -1921,8 +1922,6 @@ void QTabBarPrivate::calculateFirstLastVisible(int index, bool visible, bool rem
                     break;
                 }
             }
-            if (firstVisible < 0)
-                firstVisible = 0;
         }
         if (remove || (index == lastVisible)) {
             lastVisible = -1;
diff --git a/tests/auto/widgets/widgets/qtabbar/tst_qtabbar.cpp b/tests/auto/widgets/widgets/qtabbar/tst_qtabbar.cpp
index d575a46bc5b..1e19617be9f 100644
--- a/tests/auto/widgets/widgets/qtabbar/tst_qtabbar.cpp
+++ b/tests/auto/widgets/widgets/qtabbar/tst_qtabbar.cpp
@@ -57,6 +57,7 @@ private slots:
     void setUsesScrollButtons();
 
     void removeLastTab();
+    void removeLastVisibleTab();
 
     void closeButton();
 
@@ -467,6 +468,39 @@ void tst_QTabBar::removeLastTab()
     spy.clear();
 }
 
+void tst_QTabBar::removeLastVisibleTab()
+{
+    QTabBar tabbar;
+    tabbar.setSelectionBehaviorOnRemove(QTabBar::SelectionBehavior::SelectRightTab);
+
+    int invisible = tabbar.addTab("invisible");
+    int visible = tabbar.addTab("visible");
+    tabbar.setCurrentIndex(visible);
+    tabbar.adjustSize();
+
+    tabbar.setTabVisible(invisible, false);
+    {
+        QSignalSpy spy(&tabbar, SIGNAL(currentChanged(int)));
+        tabbar.removeTab(visible);
+        QCOMPARE(spy.count(), 1);
+        QCOMPARE(spy.at(0).at(0).toInt(), -1);
+        QCOMPARE(tabbar.currentIndex(), -1);
+    }
+
+    tabbar.setSelectionBehaviorOnRemove(QTabBar::SelectionBehavior::SelectLeftTab);
+    visible = tabbar.insertTab(0, "visible");
+    ++invisible;
+    QVERIFY(!tabbar.isTabVisible(invisible));
+    tabbar.setCurrentIndex(visible);
+    {
+        QSignalSpy spy(&tabbar, SIGNAL(currentChanged(int)));
+        tabbar.removeTab(visible);
+        QCOMPARE(spy.count(), 1);
+        QCOMPARE(spy.at(0).at(0).toInt(), -1);
+        QCOMPARE(tabbar.currentIndex(), -1);
+    }
+}
+
 void tst_QTabBar::closeButton()
 {
     QTabBar tabbar;