QTabBar: don't overshoot when scrolling right

Amends ca15f650a1a914bb9a41131109c46c4e52c5ebb1, after which scrolling
right to fill any gap might have resulted in overshooting to a negative
scrollOffset.

When we scroll right to fit the current tab, then we never want to end
up with a negative scroll, so clamp the result accordingly.

Augment test case accordingly. Since some styles align the tab bar in
the center, replace the calculation of the scroll offset with access to
the private data member (which inverts the sign when compared to the
calculated value).

Task-number: QTBUG-113140
Fixes: QTBUG-113376
Pick-to: 6.5 6.5.1
Change-Id: Ibdc6686b9dbd41b1ae3560e2227fa121d9b20e18
Reviewed-by: Axel Spoerl <axel.spoerl@qt.io>

3 files changed, 33 insertions, 14 deletions

diff --git a/src/widgets/widgets/qtabbar.cpp b/src/widgets/widgets/qtabbar.cpp
index 837874aebf..75cb6dd4ea 100644
--- a/src/widgets/widgets/qtabbar.cpp
+++ b/src/widgets/widgets/qtabbar.cpp
@@ -693,10 +693,10 @@ void QTabBarPrivate::makeVisible(int index)
         scrollOffset = tabStart - scrollRect.left();
     } else if (tabEnd > scrolledTabBarEnd) {
         // Tab is outside on the right, so scroll right.
-        scrollOffset = tabEnd - scrollRect.right();
+        scrollOffset = qMax(0, tabEnd - scrollRect.right());
     } else if (scrollOffset + entireScrollRect.width() > lastTabEnd + 1) {
-        // there's space on the right
-        scrollOffset = lastTabEnd - entireScrollRect.width() + 1;
+        // fill any free space on the right without overshooting
+        scrollOffset = qMax(0, lastTabEnd - entireScrollRect.width() + 1);
     } else if (available >= lastTabEnd) {
         // the entire tabbar fits, reset scroll
         scrollOffset = 0;
diff --git a/tests/auto/widgets/widgets/qtabbar/CMakeLists.txt b/tests/auto/widgets/widgets/qtabbar/CMakeLists.txt
index 4abe0685a5..1b84c084ff 100644
--- a/tests/auto/widgets/widgets/qtabbar/CMakeLists.txt
+++ b/tests/auto/widgets/widgets/qtabbar/CMakeLists.txt
@@ -11,4 +11,5 @@ qt_internal_add_test(tst_qtabbar
     LIBRARIES
         Qt::Gui
         Qt::Widgets
+        Qt::WidgetsPrivate
 )
diff --git a/tests/auto/widgets/widgets/qtabbar/tst_qtabbar.cpp b/tests/auto/widgets/widgets/qtabbar/tst_qtabbar.cpp
index 8fd7b5c4b2..7d47b744b1 100644
--- a/tests/auto/widgets/widgets/qtabbar/tst_qtabbar.cpp
+++ b/tests/auto/widgets/widgets/qtabbar/tst_qtabbar.cpp
@@ -14,6 +14,8 @@
 #include <QScreen>
 #include <QWindow>
 
+#include <QtWidgets/private/qtabbar_p.h>
+
 using namespace Qt::StringLiterals;
 
 class TabBar;
@@ -1353,16 +1355,21 @@ void tst_QTabBar::hoverTab()
 void tst_QTabBar::resizeKeepsScroll_data()
 {
     QTest::addColumn<QTabBar::Shape>("tabShape");
+    QTest::addColumn<bool>("expanding");
 
-    QTest::addRow("North") << QTabBar::RoundedNorth;
-    QTest::addRow("East") << QTabBar::RoundedEast;
-    QTest::addRow("South") << QTabBar::RoundedSouth;
-    QTest::addRow("West") << QTabBar::RoundedWest;
+    QTest::addRow("North, expanding") << QTabBar::RoundedNorth << true;
+    QTest::addRow("East, expanding") << QTabBar::RoundedEast << true;
+    QTest::addRow("South, expanding") << QTabBar::RoundedSouth << true;
+    QTest::addRow("West, expanding") << QTabBar::RoundedWest << true;
+
+    QTest::addRow("North, not expanding") << QTabBar::RoundedNorth << false;
+    QTest::addRow("South, not expanding") << QTabBar::RoundedSouth << false;
 }
 
 void tst_QTabBar::resizeKeepsScroll()
 {
     QFETCH(QTabBar::Shape, tabShape);
+    QFETCH(const bool, expanding);
 
     QTabBar tabBar;
     TabBarScrollingProxyStyle proxyStyle;
@@ -1373,6 +1380,7 @@ void tst_QTabBar::resizeKeepsScroll()
 
     tabBar.setShape(tabShape);
     tabBar.setUsesScrollButtons(true);
+    tabBar.setExpanding(expanding);
 
     // resize to half
     const QSize fullSize = tabBar.sizeHint();
@@ -1385,13 +1393,15 @@ void tst_QTabBar::resizeKeepsScroll()
     tabBar.show();
     QVERIFY(QTest::qWaitForWindowExposed(&tabBar));
 
+    const auto getScrollOffset = [&]() -> int {
+        return static_cast<QTabBarPrivate *>(QObjectPrivate::get(&tabBar))->scrollOffset;
+    };
+
     // select a tab outside, this will scroll
     tabBar.setCurrentIndex(6);
     // the first tab is now scrolled out
-    const int scrollOffset = horizontal
-                           ? tabBar.tabRect(0).left()
-                           : tabBar.tabRect(0).top();
-    QCOMPARE_LT(scrollOffset, 0);
+    const int scrollOffset = getScrollOffset();
+    QCOMPARE_GT(scrollOffset, 0);
     // the current index is now fully visible, with margin on both sides
     tabBar.setCurrentIndex(5);
 
@@ -1402,10 +1412,18 @@ void tst_QTabBar::resizeKeepsScroll()
         tabBar.resize(tabBar.width(), tabBar.height() + tabBar.tabRect(5).height());
 
     // this should not change the scroll
-    QCOMPARE(scrollOffset, horizontal
-                         ? tabBar.tabRect(0).left()
-                         : tabBar.tabRect(0).top());
+    QCOMPARE(getScrollOffset(), scrollOffset);
+
+    // make the tab bar large enough to fit everything with extra space
+    tabBar.resize(fullSize + QSize(50, 50));
 
+    // there should be no scroll
+    QCOMPARE(getScrollOffset(), 0);
+
+    for (int i = 0; i < tabBar.count(); ++i) {
+        tabBar.setCurrentIndex(i);
+        QCOMPARE(getScrollOffset(), 0);
+    }
 }
 
 QTEST_MAIN(tst_QTabBar)