Only apply QWindow::setVisibility() on first show or explicit request

As a result of 16023fc77c423a267fcc48894ff942e94cf35b86 we were no longer
calling QWindow::setVisible or setVisibility from the QQuickWindowQmlImpl
overrides, but instead deferred them until applyWindowVisibility().

As a consequence, if the QWindow::Visibility of the QQuickWindowQmlImpl
was left at AutomaticVisibility, we would always apply the window state
when making the window visible or hidden, which is a major regression.

We should only apply the window state on first show, if the window's
visibility is AutomaticVisibility, or on explicit requests to set the
window's visibility.

This means a window with visibility: Window.Maximized will initially
show as maximized, but if the user then resizes the window and then
hides and shows it again, the window will show in the normal window
state, instead of re-applying the Window.Maximized visibility, which
matches the pre-6.7.0 behavior.

Fixes: QTBUG-124363
Pick-to: 6.7 6.7.1
Change-Id: Idc2078c17bd56026425acbabdc40174fd12558ca
Reviewed-by: Fabian Kosmale <fabian.kosmale@qt.io>

2 files changed, 27 insertions, 6 deletions

diff --git a/src/quick/items/qquickwindowmodule.cpp b/src/quick/items/qquickwindowmodule.cpp
index c4a978dec8..e539880452 100644
--- a/src/quick/items/qquickwindowmodule.cpp
+++ b/src/quick/items/qquickwindowmodule.cpp
@@ -35,7 +35,15 @@ QQuickWindowQmlImpl::QQuickWindowQmlImpl(QQuickWindowQmlImplPrivate &dd, QWindow
     : QQuickWindow(dd, parent)
 {
     connect(this, &QWindow::visibleChanged, this, &QQuickWindowQmlImpl::visibleChanged);
-    connect(this, &QWindow::visibilityChanged, this, &QQuickWindowQmlImpl::visibilityChanged);
+    connect(this, &QWindow::visibilityChanged, this, [&]{
+        Q_D(QQuickWindowQmlImpl);
+        // Update the window's actual visibility and turn off visibilityExplicitlySet,
+        // so that future applyWindowVisibility() calls do not apply both window state
+        // and visible state, unless setVisibility() is called again by the user.
+        d->visibility = QWindow::visibility();
+        d->visibilityExplicitlySet = false;
+        emit QQuickWindowQmlImpl::visibilityChanged(d->visibility);
+    });
     connect(this, &QWindow::screenChanged, this, &QQuickWindowQmlImpl::screenChanged);
 
     // We shadow the x and y properties, so that we can re-map them in case
@@ -110,6 +118,7 @@ void QQuickWindowQmlImpl::setVisibility(Visibility visibility)
 {
     Q_D(QQuickWindowQmlImpl);
     d->visibility = visibility;
+    d->visibilityExplicitlySet = true;
     if (d->componentComplete)
         applyWindowVisibility();
 }
@@ -190,8 +199,8 @@ void QQuickWindowQmlImpl::applyWindowVisibility()
 
     Q_ASSERT(d->componentComplete);
 
-    const bool visible = d->visibility == AutomaticVisibility
-        ? d->visible : d->visibility != Hidden;
+    const bool visible = d->visibilityExplicitlySet
+        ? d->visibility != Hidden : d->visible;
 
     qCDebug(lcQuickWindow) << "Applying visible" << visible << "for" << this;
 
@@ -234,20 +243,30 @@ void QQuickWindowQmlImpl::applyWindowVisibility()
         }
     }
 
-    if (d->visibleExplicitlySet && ((d->visibility == Hidden && d->visible) ||
-                                    (d->visibility > AutomaticVisibility && !d->visible))) {
+    if (d->visibleExplicitlySet && d->visibilityExplicitlySet &&
+        ((d->visibility == Hidden && d->visible) ||
+         (d->visibility > AutomaticVisibility && !d->visible))) {
         // FIXME: Should we bail out in this case?
         qmlWarning(this) << "Conflicting properties 'visible' and 'visibility'";
     }
 
     if (d->visibility == AutomaticVisibility) {
+        // We're either showing for the first time, with the default
+        // visibility of AutomaticVisibility, or the user has called
+        // setVisibility with AutomaticVisibility at some point, so
+        // apply both window state and visible.
         if (QWindow::parent() || visualParent())
             setWindowState(Qt::WindowNoState);
         else
             setWindowState(QGuiApplicationPrivate::platformIntegration()->defaultWindowState(flags()));
         QQuickWindow::setVisible(d->visible);
-    } else {
+    } else if (d->visibilityExplicitlySet) {
+        // We're not AutomaticVisibility, but the user has requested
+        // an explicit visibility, so apply both window state and visible.
         QQuickWindow::setVisibility(d->visibility);
+    } else {
+        // Our window state should be up to date, so only apply visible
+        QQuickWindow::setVisible(d->visible);
     }
 }
 
diff --git a/src/quick/items/qquickwindowmodule_p_p.h b/src/quick/items/qquickwindowmodule_p_p.h
index bda666e15b..227b8aa01e 100644
--- a/src/quick/items/qquickwindowmodule_p_p.h
+++ b/src/quick/items/qquickwindowmodule_p_p.h
@@ -30,6 +30,8 @@ public:
     bool visible = false;
     bool visibleExplicitlySet = false;
     QQuickWindow::Visibility visibility = QQuickWindow::AutomaticVisibility;
+    bool visibilityExplicitlySet = false;
+
     QV4::PersistentValue rootItemMarker;
 
     QMetaObject::Connection itemParentWindowChangeListener;