Don't override QDialog::setVisible() to implement native dialogs

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.

Pick-to: 6.5
Change-Id: Icafe31a7b84a75049365e4e04b80492de08614d2
Reviewed-by: Volker Hilsheimer <volker.hilsheimer@qt.io>

8 files changed, 71 insertions, 44 deletions

diff --git a/src/widgets/dialogs/qdialog.cpp b/src/widgets/dialogs/qdialog.cpp
index 8adebf6050..f29f1dfded 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<QPushButton*>(first))
-                    d->mainDef->setFocus();
+                if (first != mainDef && qobject_cast<QPushButton*>(first))
+                    mainDef->setFocus();
             }
-            if (!d->mainDef && isWindow()) {
+            if (!mainDef && q->isWindow()) {
                 QWidget *w = fw;
                 while ((w = w->nextInFocusChain()) != fw) {
                     QPushButton *pb = qobject_cast<QPushButton *>(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 28b5bc16b6..878049557a 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 a4cf132e7b..1f4d6aa2e9 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 c2e68fdb51..55f0ac058e 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 cbe365b8fc..08e7a1f213 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 fc2099eddd..cf8821bbd0 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<QAbstractButton *> 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 5cae8263dc..4a67e1c065 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 808cd41b28..122170e91d 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;