QWidgetWindow: Don't meddle with WA_WState_{Hidden/ExplicitShowHide}

In ccd3bf0871b81dfc09bb469b161f32dfb47ee53e we introduced code that would
ensure that our call to QWidgetPrivate::setVisible() from QWidgetWindow
would not result in WA_WState_Hidden being set. This code was later
modified in 51300566ffe2ece2455e1d0479a556c5dbb3bb8e to apply to
widgets that were explicitly shown/hidden.

Unfortunately, the reset of the Hidden and ExplicitShowHide attributes
would in some cases result in the widget having only ExplicitShowHide
after being hidden, which is an invalid state.

It also resulted in the widget having both Visible, Hidden, and
ExplicitShowHide, if first being hidden via QWidget, and then
shown via QWindow, which in turn prevented the widget from being
hidden via QWidget::hide().

As we no longer rely on the adjustments to Hidden/ExplicitShowHide
to fix QTBUG-73021, we can remove the entire logic. Any setVisible
call to QWidgetWindow will either come from outside, in which case
we should respect that and set Visible/Hidden via QWidgetPrivate,
or the setVisible call is a result of QWidget itself (or its parent)
showing the QWidgetWindow, in which case the QWidget visible state
is already up to date and we skip the QWidgetPrivate::setVisible
call.

Task-number: QTBUG-121398
Task-number: QTBUG-73021
Fixes: QTBUG-120316
Pick-to: 6.7
Change-Id: I3174ad66b7e10c55aa99b7cb433267632169ca8f
Reviewed-by: Richard Moe Gustavsen <richard.gustavsen@qt.io>
Reviewed-by: Volker Hilsheimer <volker.hilsheimer@qt.io>
Reviewed-by: Qt CI Bot <qt_ci_bot@qt-project.org>

3 files changed, 171 insertions, 14 deletions

diff --git a/src/widgets/kernel/qwidgetwindow.cpp b/src/widgets/kernel/qwidgetwindow.cpp
index d80eb3346b..f51baba88b 100644
--- a/src/widgets/kernel/qwidgetwindow.cpp
+++ b/src/widgets/kernel/qwidgetwindow.cpp
@@ -45,21 +45,8 @@ public:
         if (QWidget *widget = q->widget()) {
             // If the widget's visible state is already matching the new QWindow
             // visible state we assume the widget has already synced up.
-            if (visible != widget->isVisible()) {
-                // Check if the widget was already hidden, as this indicates it was done
-                // explicitly and not because the parent window in this case made it hidden.
-                // In which case do not automatically show the widget when the parent
-                // window is shown.
-                const bool wasExplicitShowHide = widget->testAttribute(Qt::WA_WState_ExplicitShowHide);
-                const bool wasHidden = widget->testAttribute(Qt::WA_WState_Hidden);
-
+            if (visible != widget->isVisible())
                 QWidgetPrivate::get(widget)->setVisible(visible);
-
-                if (wasExplicitShowHide) {
-                    widget->setAttribute(Qt::WA_WState_ExplicitShowHide, wasExplicitShowHide);
-                    widget->setAttribute(Qt::WA_WState_Hidden, wasHidden);
-                }
-            }
         }
 
         // If we end up calling QWidgetPrivate::setVisible() above, we will
diff --git a/tests/auto/widgets/kernel/qwidget/tst_qwidget.cpp b/tests/auto/widgets/kernel/qwidget/tst_qwidget.cpp
index b64010f015..04ff648ea9 100644
--- a/tests/auto/widgets/kernel/qwidget/tst_qwidget.cpp
+++ b/tests/auto/widgets/kernel/qwidget/tst_qwidget.cpp
@@ -13656,6 +13656,25 @@ void tst_QWidget::explicitShowHide()
         QCOMPARE(child.testAttribute(Qt::WA_WState_ExplicitShowHide), false);
         QCOMPARE(child.testAttribute(Qt::WA_WState_Hidden), true);
     }
+
+    {
+        QWidget widget;
+        widget.show();
+        widget.hide();
+
+        QCOMPARE(widget.testAttribute(Qt::WA_WState_Visible), false);
+        QCOMPARE(widget.testAttribute(Qt::WA_WState_ExplicitShowHide), true);
+        QCOMPARE(widget.testAttribute(Qt::WA_WState_Hidden), true);
+
+        // The widget is now explicitly hidden. Showing it again, via QWindow,
+        // should make the widget visible, and it should not stay hidden, as
+        // that's an invalid state for a widget.
+
+        widget.windowHandle()->setVisible(true);
+        QCOMPARE(widget.testAttribute(Qt::WA_WState_Visible), true);
+        QCOMPARE(widget.testAttribute(Qt::WA_WState_ExplicitShowHide), true);
+        QCOMPARE(widget.testAttribute(Qt::WA_WState_Hidden), false);
+    }
 }
 
 /*!
diff --git a/tests/auto/widgets/kernel/qwidget_window/tst_qwidget_window.cpp b/tests/auto/widgets/kernel/qwidget_window/tst_qwidget_window.cpp
index 724c0c70c7..8e8cec6d4f 100644
--- a/tests/auto/widgets/kernel/qwidget_window/tst_qwidget_window.cpp
+++ b/tests/auto/widgets/kernel/qwidget_window/tst_qwidget_window.cpp
@@ -111,6 +111,9 @@ private slots:
     void mouseMoveWithPopup_data();
     void mouseMoveWithPopup();
 
+    void showHideWindowHandle_data();
+    void showHideWindowHandle();
+
     void resetFocusObjectOnDestruction();
 
     void cleanupOnDestruction();
@@ -1638,6 +1641,154 @@ void tst_QWidget_window::mouseMoveWithPopup()
     QCOMPARE(topLevel.popup->mouseReleaseCount, 1);
 }
 
+struct ShowHideEntry {
+    QEvent::Type action;
+    Qt::WindowType target;
+    using List = QList<ShowHideEntry>;
+};
+
+void tst_QWidget_window::showHideWindowHandle_data()
+{
+    QTest::addColumn<ShowHideEntry::List>("entries");
+
+    QTest::addRow("show/hide widget") << ShowHideEntry::List{
+        { QEvent::Show, Qt::Widget }, { QEvent::Hide, Qt::Widget }
+    };
+    QTest::addRow("show/hide window") << ShowHideEntry::List{
+        { QEvent::Show, Qt::Window }, { QEvent::Hide, Qt::Window }
+    };
+    QTest::addRow("show widget, hide window") << ShowHideEntry::List{
+        { QEvent::Show, Qt::Widget }, { QEvent::Hide, Qt::Window }
+    };
+    QTest::addRow("show window, hide widget") << ShowHideEntry::List{
+        { QEvent::Show, Qt::Window }, { QEvent::Hide, Qt::Widget }
+    };
+    QTest::addRow("show/hide widget, then show window, hide widget") << ShowHideEntry::List{
+        { QEvent::Show, Qt::Widget }, { QEvent::Hide, Qt::Widget },
+        { QEvent::Show, Qt::Window }, { QEvent::Hide, Qt::Widget }
+    };
+    QTest::addRow("show widget, close widget, show widget") << ShowHideEntry::List{
+        { QEvent::Show, Qt::Widget }, { QEvent::Close, Qt::Widget }, { QEvent::Show, Qt::Widget }
+    };
+    QTest::addRow("show widget, close widget, show window") << ShowHideEntry::List{
+        { QEvent::Show, Qt::Widget }, { QEvent::Close, Qt::Widget }, { QEvent::Show, Qt::Window }
+    };
+    QTest::addRow("show widget, close window, show widget") << ShowHideEntry::List{
+        { QEvent::Show, Qt::Widget }, { QEvent::Close, Qt::Window }, { QEvent::Show, Qt::Widget }
+    };
+    QTest::addRow("show widget, close window, show window") << ShowHideEntry::List{
+        { QEvent::Show, Qt::Widget }, { QEvent::Close, Qt::Window }, { QEvent::Show, Qt::Window }
+    };
+}
+
+void tst_QWidget_window::showHideWindowHandle()
+{
+    QWidget parent;
+    parent.setObjectName("Parent");
+    QCOMPARE(parent.isVisible(), false);
+    QCOMPARE(parent.testAttribute(Qt::WA_WState_ExplicitShowHide), false);
+    QCOMPARE(parent.testAttribute(Qt::WA_WState_Visible), false);
+    QCOMPARE(parent.testAttribute(Qt::WA_WState_Hidden), true);
+
+    QWidget child;
+    child.setObjectName("Child");
+    QCOMPARE(child.isVisible(), false);
+    QCOMPARE(child.testAttribute(Qt::WA_WState_ExplicitShowHide), false);
+    QCOMPARE(child.testAttribute(Qt::WA_WState_Visible), false);
+    QCOMPARE(child.testAttribute(Qt::WA_WState_Hidden), true);
+
+    child.setParent(&parent);
+    QCOMPARE(child.isVisible(), false);
+    QCOMPARE(child.testAttribute(Qt::WA_WState_ExplicitShowHide), false);
+    QCOMPARE(child.testAttribute(Qt::WA_WState_Visible), false);
+    QCOMPARE(child.testAttribute(Qt::WA_WState_Hidden), false);
+
+    QFETCH(QList<ShowHideEntry>, entries);
+    for (const auto entry : entries) {
+
+        if (entry.action == QEvent::Show) {
+            if (entry.target == Qt::Window && !parent.windowHandle()) {
+                parent.setAttribute(Qt::WA_NativeWindow);
+                QVERIFY(parent.windowHandle());
+
+                QCOMPARE(parent.isVisible(), false);
+                QCOMPARE(parent.testAttribute(Qt::WA_WState_ExplicitShowHide), false);
+                QCOMPARE(parent.testAttribute(Qt::WA_WState_Visible), false);
+                QCOMPARE(parent.testAttribute(Qt::WA_WState_Hidden), true);
+            }
+
+            bool wasExplicitShowHide = parent.testAttribute(Qt::WA_WState_ExplicitShowHide);
+
+            if (entry.target == Qt::Widget)
+                parent.show();
+            else
+                parent.windowHandle()->show();
+
+            QVERIFY(QTest::qWaitForWindowActive(&parent));
+
+            QCOMPARE(parent.isVisible(), true);
+            QVERIFY(parent.windowHandle());
+            QCOMPARE(parent.windowHandle()->isVisible(), true);
+
+            QCOMPARE(parent.testAttribute(Qt::WA_WState_Visible), true);
+            QCOMPARE(parent.testAttribute(Qt::WA_WState_Hidden), false);
+            QCOMPARE(parent.testAttribute(Qt::WA_WState_ExplicitShowHide),
+                entry.target == Qt::Widget || wasExplicitShowHide);
+
+            QCOMPARE(child.isVisible(), true);
+            QCOMPARE(child.testAttribute(Qt::WA_WState_ExplicitShowHide), false);
+            QCOMPARE(child.testAttribute(Qt::WA_WState_Visible), true);
+            QCOMPARE(child.testAttribute(Qt::WA_WState_Hidden), false);
+
+        } else if (entry.action == QEvent::Hide) {
+
+            bool wasExplicitShowHide = parent.testAttribute(Qt::WA_WState_ExplicitShowHide);
+
+            if (entry.target == Qt::Widget)
+                parent.hide();
+            else
+                parent.windowHandle()->hide();
+
+            QCOMPARE(parent.isVisible(), false);
+            QVERIFY(parent.windowHandle());
+            QCOMPARE(parent.windowHandle()->isVisible(), false);
+
+            QCOMPARE(parent.testAttribute(Qt::WA_WState_Visible), false);
+            QCOMPARE(parent.testAttribute(Qt::WA_WState_Hidden), true);
+            QCOMPARE(parent.testAttribute(Qt::WA_WState_ExplicitShowHide),
+                entry.target == Qt::Widget || wasExplicitShowHide);
+
+            QCOMPARE(child.isVisible(), false);
+            QCOMPARE(child.testAttribute(Qt::WA_WState_ExplicitShowHide), false);
+            QCOMPARE(child.testAttribute(Qt::WA_WState_Visible), false);
+            QCOMPARE(child.testAttribute(Qt::WA_WState_Hidden), false);
+
+        } else if (entry.action == QEvent::Close) {
+
+            bool wasExplicitShowHide = parent.testAttribute(Qt::WA_WState_ExplicitShowHide);
+
+            if (entry.target == Qt::Widget)
+                parent.close();
+            else
+                parent.windowHandle()->close();
+
+            QCOMPARE(parent.isVisible(), false);
+            QVERIFY(parent.windowHandle());
+            QCOMPARE(parent.windowHandle()->isVisible(), false);
+
+            QCOMPARE(parent.testAttribute(Qt::WA_WState_Visible), false);
+            QCOMPARE(parent.testAttribute(Qt::WA_WState_Hidden), true);
+            QCOMPARE(parent.testAttribute(Qt::WA_WState_ExplicitShowHide),
+                entry.target == Qt::Widget || wasExplicitShowHide);
+
+            QCOMPARE(child.isVisible(), false);
+            QCOMPARE(child.testAttribute(Qt::WA_WState_ExplicitShowHide), false);
+            QCOMPARE(child.testAttribute(Qt::WA_WState_Visible), false);
+            QCOMPARE(child.testAttribute(Qt::WA_WState_Hidden), false);
+        }
+    }
+}
+
 void tst_QWidget_window::resetFocusObjectOnDestruction()
 {
     if (!QGuiApplicationPrivate::platformIntegration()->hasCapability(QPlatformIntegration::WindowActivation))