From 79be5fa98663ca6b7e905ecb08950f17ccc06e98 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tor=20Arne=20Vestb=C3=B8?= Date: Mon, 12 Aug 2024 12:38:11 +0200 Subject: [PATCH] Make QWidgetPrivate::setVisible virtual Initially the function was used as a helper function for QWidget, implemented in da55a1b04121abd44d9c72e0c7cba7d72f16d4f2. But with e0bb9e81ab1a9d71f2893844ea82430467422e21 we started overriding it e.g. QDialog. This "worked" because QDialog itself would call the private helper, but left a footgun when called via a plain QWidget pointer, as we did in 5ba0982b2879a1a4c13bf97467b8e5ad296e57a2. That specific instance of the problem was solved by the change in fc4c6fb5f6bd6bd63b13f1f8b5b7a7289a5fd230, but the trap still exists. To ensure we can use the function in a polymorphic context in the future we make it virtual. Task-number: QTBUG-127806 Change-Id: Ic0810c5439b1e696cfbf199e701f41217acc5742 Reviewed-by: Volker Hilsheimer (cherry picked from commit 1a0f056f318417ba5e54c8110257b75cf5537acb) Reviewed-by: Qt Cherry-pick Bot --- src/widgets/dialogs/qdialog.cpp | 4 +-- src/widgets/dialogs/qdialog_p.h | 2 +- src/widgets/kernel/qwidget_p.h | 2 +- src/widgets/widgets/qmdisubwindow_p.h | 1 + .../widgets/kernel/qwidget/tst_qwidget.cpp | 26 +++++++++++++++++++ 5 files changed, 31 insertions(+), 4 deletions(-) diff --git a/src/widgets/dialogs/qdialog.cpp b/src/widgets/dialogs/qdialog.cpp index 27466d03d6c..ffdef998573 100644 --- a/src/widgets/dialogs/qdialog.cpp +++ b/src/widgets/dialogs/qdialog.cpp @@ -772,7 +772,7 @@ void QDialogPrivate::setVisible(bool visible) } if (visible) { - q->QWidget::setVisible(visible); + QWidgetPrivate::setVisible(visible); // Window activation might be prevented. We can't test isActiveWindow here, // as the window will be activated asynchronously by the window manager. @@ -831,7 +831,7 @@ void QDialogPrivate::setVisible(bool visible) #endif // Reimplemented to exit a modal event loop when the dialog is hidden. - q->QWidget::setVisible(visible); + QWidgetPrivate::setVisible(visible); if (eventLoop) eventLoop->exit(); } diff --git a/src/widgets/dialogs/qdialog_p.h b/src/widgets/dialogs/qdialog_p.h index bac33bdea95..63750540654 100644 --- a/src/widgets/dialogs/qdialog_p.h +++ b/src/widgets/dialogs/qdialog_p.h @@ -51,7 +51,7 @@ public: {} ~QDialogPrivate(); - virtual void setVisible(bool visible); + void setVisible(bool visible) override; QWindow *transientParentWindow() const; bool setNativeDialogVisible(bool visible); diff --git a/src/widgets/kernel/qwidget_p.h b/src/widgets/kernel/qwidget_p.h index b27ffdb1be6..1742999ee35 100644 --- a/src/widgets/kernel/qwidget_p.h +++ b/src/widgets/kernel/qwidget_p.h @@ -387,7 +387,7 @@ public: void hide_helper(); bool isExplicitlyHidden() const; void _q_showIfNotHidden(); - void setVisible(bool); + virtual void setVisible(bool); void setEnabled_helper(bool); static void adjustFlags(Qt::WindowFlags &flags, QWidget *w = nullptr); diff --git a/src/widgets/widgets/qmdisubwindow_p.h b/src/widgets/widgets/qmdisubwindow_p.h index 2a1f83269a5..e75f5402ed8 100644 --- a/src/widgets/widgets/qmdisubwindow_p.h +++ b/src/widgets/widgets/qmdisubwindow_p.h @@ -236,6 +236,7 @@ public: bool restoreFocus(); void storeFocusWidget(); void setWindowFlags(Qt::WindowFlags windowFlags) override; + using QWidgetPrivate::setVisible; void setVisible(WindowStateAction, bool visible = true); #ifndef QT_NO_ACTION void setEnabled(WindowStateAction, bool enable = true); diff --git a/tests/auto/widgets/kernel/qwidget/tst_qwidget.cpp b/tests/auto/widgets/kernel/qwidget/tst_qwidget.cpp index ecf7daeada7..6913a849334 100644 --- a/tests/auto/widgets/kernel/qwidget/tst_qwidget.cpp +++ b/tests/auto/widgets/kernel/qwidget/tst_qwidget.cpp @@ -469,6 +469,7 @@ private slots: #endif void setVisibleDuringDestruction(); + void setVisibleOverrideIsCalled(); void explicitShowHide(); @@ -13628,6 +13629,31 @@ void tst_QWidget::setVisibleDuringDestruction() QTRY_COMPARE(signalSpy.count(), 4); } +void tst_QWidget::setVisibleOverrideIsCalled() +{ + struct WidgetPrivate : public QWidgetPrivate + { + void setVisible(bool visible) override + { + wasCalled = true; + } + + bool wasCalled = false; + }; + class Widget : public QWidget { + public: + explicit Widget(QWidget *p = nullptr) + : QWidget(*new WidgetPrivate, p, {}) + { + } + }; + + Widget widget; + widget.setVisible(true); + auto *widgetPrivate = static_cast(QWidgetPrivate::get(&widget)); + QCOMPARE(widgetPrivate->wasCalled, true); +} + void tst_QWidget::explicitShowHide() { {