From 1a0f056f318417ba5e54c8110257b75cf5537acb 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 Pick-to: 6.8 Change-Id: Ic0810c5439b1e696cfbf199e701f41217acc5742 Reviewed-by: Volker Hilsheimer --- 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 5f90c2a5313..7394bcdbbf8 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 2854b22d05c..984072fbdbf 100644 --- a/tests/auto/widgets/kernel/qwidget/tst_qwidget.cpp +++ b/tests/auto/widgets/kernel/qwidget/tst_qwidget.cpp @@ -468,6 +468,7 @@ private slots: #endif void setVisibleDuringDestruction(); + void setVisibleOverrideIsCalled(); void explicitShowHide(); @@ -13616,6 +13617,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() { {