QTabBar: don't scroll when laying out the tabs

QTabBar lays out the tabs when the bar's size or content changes,
often lazily. This should not change the scroll offset of the tabbar,
which is controlled by the user, or at most when a tab needs to be made
visible (e.g. when it becomes the current tab).

Move the logic of updating the scoll offset into the makeVisible
function, so that the scroll is only adjusted to either leave no gap
between the last tab and the right edge of the widget of there is still
a scroll; or to reset it to 0 if there is enough space for the entire
tab bar. Since layoutTabs does show and hide the scroll buttons, we
cannot skip this when the buttons are invisible. However, the
normalizedScrollRect helper now needs to return the entire widget rect
if there are no visible scroll buttons.

Add a test case that simulates the previously broken behavior where
the scroll got unnecessarily reset to 0 when resizing.

Fixes: QTBUG-113140
Change-Id: Ic19fbb82821ea09cc5e7646dcbce3aa7607909c2
Reviewed-by: Qt CI Bot <qt_ci_bot@qt-project.org>
Reviewed-by: Santhosh Kumar <santhosh.kumar.selvaraj@qt.io>
Reviewed-by: Axel Spoerl <axel.spoerl@qt.io>
(cherry picked from commit ca15f650a1a914bb9a41131109c46c4e52c5ebb1)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>

2 files changed, 79 insertions, 4 deletions

diff --git a/src/widgets/widgets/qtabbar.cpp b/src/widgets/widgets/qtabbar.cpp
index 1b64301fa6..f644e23e6b 100644
--- a/src/widgets/widgets/qtabbar.cpp
+++ b/src/widgets/widgets/qtabbar.cpp
@@ -534,7 +534,6 @@ void QTabBarPrivate::layoutTabs()
 
     if (useScrollButtons && tabList.size() && last > available) {
         const QRect scrollRect = normalizedScrollRect(0);
-        scrollOffset = -scrollRect.left();
 
         Q_Q(QTabBar);
         QStyleOption opt;
@@ -571,10 +570,9 @@ void QTabBarPrivate::layoutTabs()
         leftB->show();
 
         rightB->setGeometry(scrollButtonRightRect);
-        rightB->setEnabled(last - scrollOffset > scrollRect.x() + scrollRect.width());
+        rightB->setEnabled(last + scrollRect.left() > scrollRect.x() + scrollRect.width());
         rightB->show();
     } else {
-        scrollOffset = 0;
         rightB->hide();
         leftB->hide();
     }
@@ -591,6 +589,11 @@ QRect QTabBarPrivate::normalizedScrollRect(int index)
     // tab bar itself is in a different orientation.
 
     Q_Q(QTabBar);
+    // If scrollbuttons are not visible, then there's no tear either, and
+    // the entire widget is the scroll rect.
+    if (leftB->isHidden())
+        return verticalTabs(shape) ? q->rect().transposed() : q->rect();
+
     QStyleOptionTab opt;
     q->initStyleOption(&opt, currentIndex);
     opt.rect = q->rect();
@@ -670,16 +673,18 @@ int QTabBarPrivate::hoveredTabIndex() const
 void QTabBarPrivate::makeVisible(int index)
 {
     Q_Q(QTabBar);
-    if (!validIndex(index) || leftB->isHidden())
+    if (!validIndex(index))
         return;
 
     const QRect tabRect = tabList.at(index)->rect;
     const int oldScrollOffset = scrollOffset;
     const bool horiz = !verticalTabs(shape);
+    const int available = horiz ? q->width() : q->height();
     const int tabStart = horiz ? tabRect.left() : tabRect.top();
     const int tabEnd = horiz ? tabRect.right() : tabRect.bottom();
     const int lastTabEnd = horiz ? tabList.constLast()->rect.right() : tabList.constLast()->rect.bottom();
     const QRect scrollRect = normalizedScrollRect(index);
+    const QRect entireScrollRect = normalizedScrollRect(0); // ignore tears
     const int scrolledTabBarStart = qMax(1, scrollRect.left() + scrollOffset);
     const int scrolledTabBarEnd = qMin(lastTabEnd - 1, scrollRect.right() + scrollOffset);
 
@@ -689,6 +694,12 @@ void QTabBarPrivate::makeVisible(int index)
     } else if (tabEnd > scrolledTabBarEnd) {
         // Tab is outside on the right, so scroll right.
         scrollOffset = tabEnd - scrollRect.right();
+    } else if (scrollOffset + entireScrollRect.width() > lastTabEnd + 1) {
+        // there's space on the right
+        scrollOffset = lastTabEnd - entireScrollRect.width() + 1;
+    } else if (available >= lastTabEnd) {
+        // the entire tabbar fits, reset scroll
+        scrollOffset = 0;
     }
 
     leftB->setEnabled(scrollOffset > -scrollRect.left());
@@ -1778,6 +1789,8 @@ void QTabBar::resizeEvent(QResizeEvent *)
     Q_D(QTabBar);
     if (d->layoutDirty)
         updateGeometry();
+
+    // when resizing, we want to keep the scroll offset as much as possible
     d->layoutTabs();
 
     d->makeVisible(d->currentIndex);
diff --git a/tests/auto/widgets/widgets/qtabbar/tst_qtabbar.cpp b/tests/auto/widgets/widgets/qtabbar/tst_qtabbar.cpp
index 1d7e999b10..8fd7b5c4b2 100644
--- a/tests/auto/widgets/widgets/qtabbar/tst_qtabbar.cpp
+++ b/tests/auto/widgets/widgets/qtabbar/tst_qtabbar.cpp
@@ -94,6 +94,9 @@ private slots:
     void hoverTab_data();
     void hoverTab();
 
+    void resizeKeepsScroll_data();
+    void resizeKeepsScroll();
+
 private:
     void checkPositions(const TabBar &tabbar, const QList<int> &positions);
 };
@@ -1346,5 +1349,64 @@ void tst_QTabBar::hoverTab()
     QCOMPARE(tabbar.styleOptions[1].state & QStyle::State_MouseOver, QStyle::State_None);
 }
 
+
+void tst_QTabBar::resizeKeepsScroll_data()
+{
+    QTest::addColumn<QTabBar::Shape>("tabShape");
+
+    QTest::addRow("North") << QTabBar::RoundedNorth;
+    QTest::addRow("East") << QTabBar::RoundedEast;
+    QTest::addRow("South") << QTabBar::RoundedSouth;
+    QTest::addRow("West") << QTabBar::RoundedWest;
+}
+
+void tst_QTabBar::resizeKeepsScroll()
+{
+    QFETCH(QTabBar::Shape, tabShape);
+
+    QTabBar tabBar;
+    TabBarScrollingProxyStyle proxyStyle;
+    tabBar.setStyle(&proxyStyle);
+
+    for (int i = 0; i < 10; ++i)
+        tabBar.addTab(u"Tab Number %1"_s.arg(i));
+
+    tabBar.setShape(tabShape);
+    tabBar.setUsesScrollButtons(true);
+
+    // resize to half
+    const QSize fullSize = tabBar.sizeHint();
+    const bool horizontal = fullSize.width() > fullSize.height();
+    if (horizontal)
+        tabBar.resize(fullSize.width() / 2, fullSize.height());
+    else
+        tabBar.resize(fullSize.width(), fullSize.height() / 2);
+
+    tabBar.show();
+    QVERIFY(QTest::qWaitForWindowExposed(&tabBar));
+
+    // 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);
+    // the current index is now fully visible, with margin on both sides
+    tabBar.setCurrentIndex(5);
+
+    // make the tab bar a bit larger, by the width of a tab
+    if (horizontal)
+        tabBar.resize(tabBar.width() + tabBar.tabRect(5).width(), tabBar.height());
+    else
+        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());
+
+}
+
 QTEST_MAIN(tst_QTabBar)
 #include "tst_qtabbar.moc"