QAbstractScrollArea: Don't include size of invisible scrollbars

Amend 3e59a88e8968c6cdac788926bec34c259146b6a8, which incorrectly used
isHidden() to test whether the scrollbar is visible or not.
QWidget::isHidden() is only true for child widgets that are explicitly
hidden (or created for visible parents, which the scrollbars are not).
Since the scrollbars are children of a container that is hidden and
shown, isHidden always returns false.

Instead, use QWidget::isVisibleTo, passing the scroll area, as that
tells us if the scrollbar's visibility is relevant for the layout of the
scroll area.

Add a test case for QAbstractScrollArea, verifying that the scrollbar's
size is correctly taken into account when calculating the size hint.
This change revealed an instability in the tests introduced in the
earlier commit: the layout process is asynchronous, requiring event
processing to update the visibility of the scrollbars. Add a call to
processEvents before storing the reference size hint. Also, explicitly
set a style that doesn't use transient scrollbars as otherwise we cannot
control when the scrollbars are shown.

The chagne also revealed an inaccuracy in the QListView test, which
only passed because the width of the vertical scrollbar was included.
We cannot use font metrics results to compare expected width, as the
item delegate's text rendering uses text layouts.

Task-number: QTBUG-69120
Fixes: QTBUG-109326
Fixes: QTBUG-113552
Change-Id: I1f06f9e88046a77722291ac17c56090f8dff7cf3
Reviewed-by: Qt CI Bot <qt_ci_bot@qt-project.org>
Reviewed-by: Axel Spoerl <axel.spoerl@qt.io>
(cherry picked from commit 8c18a245b0245de20c064cd53d03498088bd57df)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>
This commit is contained in:
Volker Hilsheimer 2023-07-01 12:17:16 +02:00 committed by Qt Cherry-pick Bot
parent b35f5a187d
commit 0c9787babd
5 changed files with 76 additions and 5 deletions

View File

@ -1431,9 +1431,9 @@ QSize QAbstractScrollArea::sizeHint() const
if (!d->sizeHint.isValid() || d->sizeAdjustPolicy == QAbstractScrollArea::AdjustToContents) { if (!d->sizeHint.isValid() || d->sizeAdjustPolicy == QAbstractScrollArea::AdjustToContents) {
const int f = 2 * d->frameWidth; const int f = 2 * d->frameWidth;
const QSize frame( f, f ); const QSize frame(f, f);
const bool vbarHidden = d->vbar->isHidden() || d->vbarpolicy == Qt::ScrollBarAlwaysOff; const bool vbarHidden = !d->vbar->isVisibleTo(this) || d->vbarpolicy == Qt::ScrollBarAlwaysOff;
const bool hbarHidden = d->hbar->isHidden() || d->hbarpolicy == Qt::ScrollBarAlwaysOff; const bool hbarHidden = !d->vbar->isVisibleTo(this) || d->hbarpolicy == Qt::ScrollBarAlwaysOff;
const QSize scrollbars(vbarHidden ? 0 : d->vbar->sizeHint().width(), const QSize scrollbars(vbarHidden ? 0 : d->vbar->sizeHint().width(),
hbarHidden ? 0 : d->hbar->sizeHint().height()); hbarHidden ? 0 : d->hbar->sizeHint().height());
d->sizeHint = frame + scrollbars + viewportSizeHint(); d->sizeHint = frame + scrollbars + viewportSizeHint();

View File

@ -2564,8 +2564,9 @@ void tst_QListView::taskQTBUG_58749_adjustToContent()
// use the long text and make sure the width is adjusted. // use the long text and make sure the width is adjusted.
model.setData(model.index(0, 0), longText); model.setData(model.index(0, 0), longText);
QApplication::processEvents(); QApplication::processEvents();
QVERIFY(w.width() > longTextWidth); const QRect itemRect = view->visualRect(model.index(0, 0));
QVERIFY(view->width() >= longTextWidth); QVERIFY(w.width() > itemRect.width());
QCOMPARE_GE(view->width(), itemRect.width());
} }
void tst_QListView::taskQTBUG_51086_skippingIndexesInSelectedIndexes() void tst_QListView::taskQTBUG_51086_skippingIndexesInSelectedIndexes()

View File

@ -4,6 +4,7 @@
#include <QHeaderView> #include <QHeaderView>
#include <QLineEdit> #include <QLineEdit>
#include <QMimeData> #include <QMimeData>
#include <QScrollBar>
#include <QSignalSpy> #include <QSignalSpy>
#include <QTableWidget> #include <QTableWidget>
#include <QTest> #include <QTest>
@ -1556,6 +1557,12 @@ void tst_QTableWidget::sizeHint()
QFETCH(Qt::ScrollBarPolicy, scrollBarPolicy); QFETCH(Qt::ScrollBarPolicy, scrollBarPolicy);
QFETCH(QSize, viewSize); QFETCH(QSize, viewSize);
const QString defaultStyle = QApplication::style()->name();
QApplication::setStyle("windows");
const auto resetStyle = qScopeGuard([defaultStyle]{
QApplication::setStyle(defaultStyle);
});
QTableWidget view(2, 2); QTableWidget view(2, 2);
view.setSizeAdjustPolicy(QAbstractScrollArea::AdjustToContents); view.setSizeAdjustPolicy(QAbstractScrollArea::AdjustToContents);
view.setVerticalScrollBarPolicy(scrollBarPolicy); view.setVerticalScrollBarPolicy(scrollBarPolicy);
@ -1575,18 +1582,21 @@ void tst_QTableWidget::sizeHint()
QTRY_COMPARE(view.size(), viewSize); QTRY_COMPARE(view.size(), viewSize);
} }
QApplication::processEvents(); // execute delayed layouts
auto sizeHint = view.sizeHint(); auto sizeHint = view.sizeHint();
view.hide(); view.hide();
QCOMPARE(view.sizeHint(), sizeHint); QCOMPARE(view.sizeHint(), sizeHint);
view.horizontalHeader()->hide(); view.horizontalHeader()->hide();
view.show(); view.show();
QApplication::processEvents(); // execute delayed layouts
sizeHint = view.sizeHint(); sizeHint = view.sizeHint();
view.hide(); view.hide();
QCOMPARE(view.sizeHint(), sizeHint); QCOMPARE(view.sizeHint(), sizeHint);
view.verticalHeader()->hide(); view.verticalHeader()->hide();
view.show(); view.show();
QApplication::processEvents(); // execute delayed layouts
sizeHint = view.sizeHint(); sizeHint = view.sizeHint();
view.hide(); view.hide();
QCOMPARE(view.sizeHint(), sizeHint); QCOMPARE(view.sizeHint(), sizeHint);

View File

@ -2627,6 +2627,12 @@ void tst_QTreeWidget::sizeHint()
QFETCH(Qt::ScrollBarPolicy, scrollBarPolicy); QFETCH(Qt::ScrollBarPolicy, scrollBarPolicy);
QFETCH(QSize, viewSize); QFETCH(QSize, viewSize);
const QString defaultStyle = QApplication::style()->name();
QApplication::setStyle("fusion");
const auto resetStyle = qScopeGuard([defaultStyle]{
QApplication::setStyle(defaultStyle);
});
QTreeWidget view; QTreeWidget view;
view.setSizeAdjustPolicy(QAbstractScrollArea::AdjustToContents); view.setSizeAdjustPolicy(QAbstractScrollArea::AdjustToContents);
view.setVerticalScrollBarPolicy(scrollBarPolicy); view.setVerticalScrollBarPolicy(scrollBarPolicy);
@ -2644,6 +2650,7 @@ void tst_QTreeWidget::sizeHint()
QTRY_COMPARE(view.size(), viewSize); QTRY_COMPARE(view.size(), viewSize);
} }
QApplication::processEvents(); // execute delayed layouts
auto sizeHint = view.sizeHint(); auto sizeHint = view.sizeHint();
view.hide(); view.hide();
QCOMPARE(view.sizeHint(), sizeHint); QCOMPARE(view.sizeHint(), sizeHint);

View File

@ -13,6 +13,7 @@
#include <qwidget.h> #include <qwidget.h>
#include <qdialog.h> #include <qdialog.h>
#include <qscroller.h> #include <qscroller.h>
#include <qstyle.h>
class tst_QAbstractScrollArea : public QObject class tst_QAbstractScrollArea : public QObject
{ {
@ -34,6 +35,7 @@ private slots:
void margins(); void margins();
void resizeWithOvershoot(); void resizeWithOvershoot();
void sizeHint();
}; };
tst_QAbstractScrollArea::tst_QAbstractScrollArea() tst_QAbstractScrollArea::tst_QAbstractScrollArea()
@ -408,5 +410,56 @@ void tst_QAbstractScrollArea::resizeWithOvershoot()
QTRY_COMPARE(scrollArea.viewport()->pos(), originAtRest); QTRY_COMPARE(scrollArea.viewport()->pos(), originAtRest);
} }
void tst_QAbstractScrollArea::sizeHint()
{
class ScrollArea : public QAbstractScrollArea
{
public:
QSize viewportSizeHint() const override { return {200, 200}; }
} scrollArea;
// We cannot reliable test the impact of the scrollbars on the size hint
// if the style uses transient scrollbars, so use the class Windows style.
const QString defaultStyle = QApplication::style()->name();
QApplication::setStyle("Windows");
auto resetStyle = qScopeGuard([defaultStyle]{
QApplication::setStyle(defaultStyle);
});
scrollArea.setFrameShape(QFrame::NoFrame);
scrollArea.setSizeAdjustPolicy(QAbstractScrollArea::AdjustToContents);
scrollArea.show();
QSize sizeHint = scrollArea.sizeHint();
QCOMPARE(sizeHint, scrollArea.viewportSizeHint());
scrollArea.setVerticalScrollBarPolicy(Qt::ScrollBarAlwaysOn);
scrollArea.setHorizontalScrollBarPolicy(Qt::ScrollBarAlwaysOn);
const QSize sizeHintWithScrollBars = scrollArea.sizeHint();
QTRY_COMPARE_GT(sizeHintWithScrollBars.width(), sizeHint.width());
QTRY_COMPARE_GT(sizeHintWithScrollBars.height(), sizeHint.height());
sizeHint = scrollArea.sizeHint();
// whether the scroll area itself is visible or not should not influence
// the size hint
scrollArea.hide();
QCOMPARE(scrollArea.sizeHint(), sizeHint);
scrollArea.show();
QCOMPARE(scrollArea.sizeHint(), sizeHint);
scrollArea.setVerticalScrollBarPolicy(Qt::ScrollBarAlwaysOff);
scrollArea.setHorizontalScrollBarPolicy(Qt::ScrollBarAlwaysOff);
QCOMPARE(scrollArea.sizeHint(), scrollArea.viewportSizeHint());
scrollArea.setVerticalScrollBarPolicy(Qt::ScrollBarAsNeeded);
scrollArea.setHorizontalScrollBarPolicy(Qt::ScrollBarAsNeeded);
scrollArea.verticalScrollBar()->setRange(0, 1);
scrollArea.horizontalScrollBar()->setRange(0, 1);
scrollArea.resize(sizeHint / 2);
QApplication::processEvents(); // trigger lazy layout process
QCOMPARE(scrollArea.sizeHint(), sizeHintWithScrollBars);
}
QTEST_MAIN(tst_QAbstractScrollArea) QTEST_MAIN(tst_QAbstractScrollArea)
#include "tst_qabstractscrollarea.moc" #include "tst_qabstractscrollarea.moc"