From 969ba5e36845cc72c413a5f949770460fb1138b3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tor=20Arne=20Vestb=C3=B8?= Date: Mon, 9 Jan 2023 13:17:52 +0100 Subject: [PATCH] Don't override QDialog::setVisible() to implement native dialogs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Clients who called the base-class implementation in QDialog would as a result start hitting the canBeNativeDialog code path at the start of QDialog::setVisible(), which would show the native dialog, but without updating the QWidget visibility state. To keep things 100% compatible, we shuffle the implementation of QDialog::setVisible() into QDialogPrivate, which allows us to override it in QMessageBoxPrivate and QErrorMessagePrivate. The existing subclasses of QDialog that override setVisible have been left as is, to not cause any unintended behavior change. Change-Id: Icafe31a7b84a75049365e4e04b80492de08614d2 Reviewed-by: Volker Hilsheimer (cherry picked from commit e0bb9e81ab1a9d71f2893844ea82430467422e21) Reviewed-by: Tor Arne Vestbø --- src/widgets/dialogs/qdialog.cpp | 58 ++++++++++--------- src/widgets/dialogs/qdialog_p.h | 2 + src/widgets/dialogs/qerrormessage.cpp | 16 ++--- src/widgets/dialogs/qerrormessage.h | 2 - src/widgets/dialogs/qmessagebox.cpp | 16 ++--- src/widgets/dialogs/qmessagebox.h | 2 - .../qerrormessage/tst_qerrormessage.cpp | 9 +++ .../dialogs/qmessagebox/tst_qmessagebox.cpp | 10 ++++ 8 files changed, 71 insertions(+), 44 deletions(-) diff --git a/src/widgets/dialogs/qdialog.cpp b/src/widgets/dialogs/qdialog.cpp index 8adebf60505..f29f1dfded5 100644 --- a/src/widgets/dialogs/qdialog.cpp +++ b/src/widgets/dialogs/qdialog.cpp @@ -742,35 +742,41 @@ void QDialog::closeEvent(QCloseEvent *e) void QDialog::setVisible(bool visible) { Q_D(QDialog); - if (!testAttribute(Qt::WA_DontShowOnScreen) && d->canBeNativeDialog() && d->setNativeDialogVisible(visible)) + d->setVisible(visible); +} + +void QDialogPrivate::setVisible(bool visible) +{ + Q_Q(QDialog); + if (!q->testAttribute(Qt::WA_DontShowOnScreen) && canBeNativeDialog() && setNativeDialogVisible(visible)) return; // We should not block windows by the invisible modal dialog // if a platform-specific dialog is implemented as an in-process // Qt window, because in this case it will also be blocked. - const bool dontBlockWindows = testAttribute(Qt::WA_DontShowOnScreen) - && d->styleHint(QPlatformDialogHelper::DialogIsQtWindow).toBool(); + const bool dontBlockWindows = q->testAttribute(Qt::WA_DontShowOnScreen) + && styleHint(QPlatformDialogHelper::DialogIsQtWindow).toBool(); Qt::WindowModality oldModality; bool wasModalitySet; if (dontBlockWindows) { - oldModality = windowModality(); - wasModalitySet = testAttribute(Qt::WA_SetWindowModality); - setWindowModality(Qt::NonModal); + oldModality = q->windowModality(); + wasModalitySet = q->testAttribute(Qt::WA_SetWindowModality); + q->setWindowModality(Qt::NonModal); } if (visible) { - if (testAttribute(Qt::WA_WState_ExplicitShowHide) && !testAttribute(Qt::WA_WState_Hidden)) + if (q->testAttribute(Qt::WA_WState_ExplicitShowHide) && !q->testAttribute(Qt::WA_WState_Hidden)) return; - QWidget::setVisible(visible); + q->QWidget::setVisible(visible); // Window activation might be prevented. We can't test isActiveWindow here, // as the window will be activated asynchronously by the window manager. - if (!testAttribute(Qt::WA_ShowWithoutActivating)) { - QWidget *fw = window()->focusWidget(); + if (!q->testAttribute(Qt::WA_ShowWithoutActivating)) { + QWidget *fw = q->window()->focusWidget(); if (!fw) - fw = this; + fw = q; /* The following block is to handle a special case, and does not @@ -783,14 +789,14 @@ void QDialog::setVisible(bool visible) have to use [widget*]->setFocus() themselves... */ #if QT_CONFIG(pushbutton) - if (d->mainDef && fw->focusPolicy() == Qt::NoFocus) { + if (mainDef && fw->focusPolicy() == Qt::NoFocus) { QWidget *first = fw; while ((first = first->nextInFocusChain()) != fw && first->focusPolicy() == Qt::NoFocus) ; - if (first != d->mainDef && qobject_cast(first)) - d->mainDef->setFocus(); + if (first != mainDef && qobject_cast(first)) + mainDef->setFocus(); } - if (!d->mainDef && isWindow()) { + if (!mainDef && q->isWindow()) { QWidget *w = fw; while ((w = w->nextInFocusChain()) != fw) { QPushButton *pb = qobject_cast(w); @@ -808,37 +814,37 @@ void QDialog::setVisible(bool visible) } #if QT_CONFIG(accessibility) - QAccessibleEvent event(this, QAccessible::DialogStart); + QAccessibleEvent event(q, QAccessible::DialogStart); QAccessible::updateAccessibility(&event); #endif } else { - if (testAttribute(Qt::WA_WState_ExplicitShowHide) && testAttribute(Qt::WA_WState_Hidden)) + if (q->testAttribute(Qt::WA_WState_ExplicitShowHide) && q->testAttribute(Qt::WA_WState_Hidden)) return; #if QT_CONFIG(accessibility) - if (isVisible()) { - QAccessibleEvent event(this, QAccessible::DialogEnd); + if (q->isVisible()) { + QAccessibleEvent event(q, QAccessible::DialogEnd); QAccessible::updateAccessibility(&event); } #endif // Reimplemented to exit a modal event loop when the dialog is hidden. - QWidget::setVisible(visible); - if (d->eventLoop) - d->eventLoop->exit(); + q->QWidget::setVisible(visible); + if (eventLoop) + eventLoop->exit(); } if (dontBlockWindows) { - setWindowModality(oldModality); - setAttribute(Qt::WA_SetWindowModality, wasModalitySet); + q->setWindowModality(oldModality); + q->setAttribute(Qt::WA_SetWindowModality, wasModalitySet); } #if QT_CONFIG(pushbutton) const QPlatformTheme *theme = QGuiApplicationPrivate::platformTheme(); - if (d->mainDef && isActiveWindow() + if (mainDef && q->isActiveWindow() && theme->themeHint(QPlatformTheme::DialogSnapToDefaultButton).toBool()) - QCursor::setPos(d->mainDef->mapToGlobal(d->mainDef->rect().center())); + QCursor::setPos(mainDef->mapToGlobal(mainDef->rect().center())); #endif } diff --git a/src/widgets/dialogs/qdialog_p.h b/src/widgets/dialogs/qdialog_p.h index 28b5bc16b6c..878049557af 100644 --- a/src/widgets/dialogs/qdialog_p.h +++ b/src/widgets/dialogs/qdialog_p.h @@ -51,6 +51,8 @@ public: {} ~QDialogPrivate(); + virtual void setVisible(bool visible); + QWindow *transientParentWindow() const; bool setNativeDialogVisible(bool visible); QVariant styleHint(QPlatformDialogHelper::StyleHint hint) const; diff --git a/src/widgets/dialogs/qerrormessage.cpp b/src/widgets/dialogs/qerrormessage.cpp index 2c36e535c7f..c6cb56fe2d8 100644 --- a/src/widgets/dialogs/qerrormessage.cpp +++ b/src/widgets/dialogs/qerrormessage.cpp @@ -53,6 +53,8 @@ public: bool nextPending(); void retranslateStrings(); + void setVisible(bool) override; + private: void initHelper(QPlatformDialogHelper *) override; void helperPrepareShow(QPlatformDialogHelper *) override; @@ -403,21 +405,21 @@ void QErrorMessage::showMessage(const QString &message, const QString &type) show(); } -void QErrorMessage::setVisible(bool visible) +void QErrorMessagePrivate::setVisible(bool visible) { - if (testAttribute(Qt::WA_WState_ExplicitShowHide) && testAttribute(Qt::WA_WState_Hidden) != visible) + Q_Q(QErrorMessage); + if (q->testAttribute(Qt::WA_WState_ExplicitShowHide) && q->testAttribute(Qt::WA_WState_Hidden) != visible) return; - Q_D(QErrorMessage); - if (d->canBeNativeDialog()) - d->setNativeDialogVisible(visible); + if (canBeNativeDialog()) + setNativeDialogVisible(visible); // Update WA_DontShowOnScreen based on whether the native dialog was shown, // so that QDialog::setVisible(visible) below updates the QWidget state correctly, // but skips showing the non-native version. - setAttribute(Qt::WA_DontShowOnScreen, d->nativeDialogInUse); + q->setAttribute(Qt::WA_DontShowOnScreen, nativeDialogInUse); - QDialog::setVisible(visible); + QDialogPrivate::setVisible(visible); } /*! diff --git a/src/widgets/dialogs/qerrormessage.h b/src/widgets/dialogs/qerrormessage.h index c2e68fdb51e..55f0ac058ed 100644 --- a/src/widgets/dialogs/qerrormessage.h +++ b/src/widgets/dialogs/qerrormessage.h @@ -28,8 +28,6 @@ public Q_SLOTS: void showMessage(const QString &message); void showMessage(const QString &message, const QString &type); - void setVisible(bool) override; - protected: void done(int) override; void changeEvent(QEvent *e) override; diff --git a/src/widgets/dialogs/qmessagebox.cpp b/src/widgets/dialogs/qmessagebox.cpp index e38826534a5..708ca50aa86 100644 --- a/src/widgets/dialogs/qmessagebox.cpp +++ b/src/widgets/dialogs/qmessagebox.cpp @@ -188,6 +188,8 @@ public: int layoutMinimumWidth(); void retranslateStrings(); + void setVisible(bool visible) override; + static int showOldMessageBox(QWidget *parent, QMessageBox::Icon icon, const QString &title, const QString &text, int button0, int button1, int button2); @@ -1563,21 +1565,21 @@ void QMessageBox::open(QObject *receiver, const char *member) QDialog::open(); } -void QMessageBox::setVisible(bool visible) +void QMessageBoxPrivate::setVisible(bool visible) { - if (testAttribute(Qt::WA_WState_ExplicitShowHide) && testAttribute(Qt::WA_WState_Hidden) != visible) + Q_Q(QMessageBox); + if (q->testAttribute(Qt::WA_WState_ExplicitShowHide) && q->testAttribute(Qt::WA_WState_Hidden) != visible) return; - Q_D(QMessageBox); - if (d->canBeNativeDialog()) - d->setNativeDialogVisible(visible); + if (canBeNativeDialog()) + setNativeDialogVisible(visible); // Update WA_DontShowOnScreen based on whether the native dialog was shown, // so that QDialog::setVisible(visible) below updates the QWidget state correctly, // but skips showing the non-native version. - setAttribute(Qt::WA_DontShowOnScreen, d->nativeDialogInUse); + q->setAttribute(Qt::WA_DontShowOnScreen, nativeDialogInUse); - QDialog::setVisible(visible); + QDialogPrivate::setVisible(visible); } /*! diff --git a/src/widgets/dialogs/qmessagebox.h b/src/widgets/dialogs/qmessagebox.h index 5d75ffd8428..4b1f5118c88 100644 --- a/src/widgets/dialogs/qmessagebox.h +++ b/src/widgets/dialogs/qmessagebox.h @@ -113,8 +113,6 @@ public: using QDialog::open; void open(QObject *receiver, const char *member); - void setVisible(bool visible) override; - QList buttons() const; ButtonRole buttonRole(QAbstractButton *button) const; diff --git a/tests/auto/widgets/dialogs/qerrormessage/tst_qerrormessage.cpp b/tests/auto/widgets/dialogs/qerrormessage/tst_qerrormessage.cpp index 5cae8263dc6..4a67e1c0656 100644 --- a/tests/auto/widgets/dialogs/qerrormessage/tst_qerrormessage.cpp +++ b/tests/auto/widgets/dialogs/qerrormessage/tst_qerrormessage.cpp @@ -18,6 +18,7 @@ private slots: void dontShowAgain(); void dontShowCategoryAgain(); + void baseClassSetVisible(); }; @@ -138,5 +139,13 @@ void tst_QErrorMessage::dontShowCategoryAgain() QVERIFY(errorMessageDialog.isVisible()); } +void tst_QErrorMessage::baseClassSetVisible() +{ + QErrorMessage errorMessage; + errorMessage.QDialog::setVisible(true); + QCOMPARE(errorMessage.isVisible(), true); + errorMessage.close(); +} + QTEST_MAIN(tst_QErrorMessage) #include "tst_qerrormessage.moc" diff --git a/tests/auto/widgets/dialogs/qmessagebox/tst_qmessagebox.cpp b/tests/auto/widgets/dialogs/qmessagebox/tst_qmessagebox.cpp index 808cd41b28c..122170e91de 100644 --- a/tests/auto/widgets/dialogs/qmessagebox/tst_qmessagebox.cpp +++ b/tests/auto/widgets/dialogs/qmessagebox/tst_qmessagebox.cpp @@ -27,6 +27,7 @@ private slots: void init(); void sanityTest(); + void baseClassSetVisible(); void defaultButton(); void escapeButton(); void clickedButton(); @@ -174,6 +175,15 @@ void tst_QMessageBox::sanityTest() msgBox.exec(); } +void tst_QMessageBox::baseClassSetVisible() +{ + QMessageBox msgBox; + msgBox.setText("Hello World"); + msgBox.QDialog::setVisible(true); + QCOMPARE(msgBox.isVisible(), true); + msgBox.close(); +} + void tst_QMessageBox::button() { QMessageBox msgBox;