Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
If Loader loads Window, set its transient parent to the Loader's window
This makes the Item { Loader { sourceComponent: Window { } } } case
consistent with the Item { Window { } } case: the inner Window is
transient for the outer Window.

It works even if the Loader's Window has a visible: true declaration:
in that case, until now, the Loader's Window would become visible
at component creation time, before the outer Item became visible.
So the test to check whether it had a transient parent did not work.

We now change the delayed-visibility mechanism in QQuickWindowQmlImpl
to wait for the parent Item to acquire a window of its own rather
than waiting for the transient-parent-if-any to become visible.
It should still take care of all the old cases too, e.g. in the
Window { Window { } } case, the inner Window's QObject parent is actually
the QQuickRootItem.  (Amends 701255c76f671f100338a799f0194bf10e26c9d1)

[ChangeLog][QtQuick][QQuickWindow] When a Window is declared inside
another Item or Window, the window will not be created until
the parent window is created.  This allows it to have the correct
transientParent and be managed as a transient window.

Task-number: QTBUG-52944
Change-Id: Iaf4aafbd696f6e8dd0eec1d02db8bd181483bd07
Reviewed-by: Morten Johan Sørvig <morten.sorvig@qt.io>
Reviewed-by: Shawn Rutledge <shawn.rutledge@qt.io>
  • Loading branch information
ec1oud committed Feb 2, 2018
1 parent 7bd5d93 commit 4b014ef
Show file tree
Hide file tree
Showing 6 changed files with 166 additions and 4 deletions.
19 changes: 19 additions & 0 deletions src/quick/items/qquickloader.cpp
Expand Up @@ -674,6 +674,13 @@ void QQuickLoaderPrivate::incubatorStateChanged(QQmlIncubator::Status status)
if (status == QQmlIncubator::Ready) {
object = incubator->object();
item = qmlobject_cast<QQuickItem*>(object);
if (!item) {
QQuickWindow *window = qmlobject_cast<QQuickWindow*>(object);
if (window) {
qCDebug(lcTransient) << window << "is transient for" << q->window();
window->setTransientParent(q->window());
}
}
emit q->itemChanged();
initResize();
incubator->clear();
Expand Down Expand Up @@ -818,6 +825,18 @@ void QQuickLoader::componentComplete()
}
}

void QQuickLoader::itemChange(QQuickItem::ItemChange change, const QQuickItem::ItemChangeData &value)
{
if (change == ItemSceneChange) {
QQuickWindow *loadedWindow = qmlobject_cast<QQuickWindow *>(item());
if (loadedWindow) {
qCDebug(lcTransient) << loadedWindow << "is transient for" << value.window;
loadedWindow->setTransientParent(value.window);
}
}
QQuickItem::itemChange(change, value);
}

/*!
\qmlsignal QtQuick::Loader::loaded()
Expand Down
1 change: 1 addition & 0 deletions src/quick/items/qquickloader_p.h
Expand Up @@ -107,6 +107,7 @@ class Q_AUTOTEST_EXPORT QQuickLoader : public QQuickImplicitSizeItem
protected:
void geometryChanged(const QRectF &newGeometry, const QRectF &oldGeometry) Q_DECL_OVERRIDE;
void componentComplete() Q_DECL_OVERRIDE;
void itemChange(ItemChange change, const ItemChangeData &value) override;

private:
void setSource(const QUrl &sourceUrl, bool needsClear);
Expand Down
15 changes: 11 additions & 4 deletions src/quick/items/qquickwindowmodule.cpp
Expand Up @@ -123,7 +123,13 @@ void QQuickWindowQmlImpl::componentComplete()
{
Q_D(QQuickWindowQmlImpl);
d->complete = true;
if (transientParent() && !transientParent()->isVisible()) {
QQuickItem *itemParent = qmlobject_cast<QQuickItem *>(QObject::parent());
if (itemParent && !itemParent->window()) {
qCDebug(lcTransient) << "window" << title() << "has invisible Item parent" << itemParent << "transientParent"
<< transientParent() << "declared visibility" << d->visibility << "; delaying show";
connect(itemParent, &QQuickItem::windowChanged, this,
&QQuickWindowQmlImpl::setWindowVisibility, Qt::QueuedConnection);
} else if (transientParent() && !transientParent()->isVisible()) {
connect(transientParent(), &QQuickWindow::visibleChanged, this,
&QQuickWindowQmlImpl::setWindowVisibility, Qt::QueuedConnection);
} else {
Expand All @@ -137,9 +143,10 @@ void QQuickWindowQmlImpl::setWindowVisibility()
if (transientParent() && !transientParent()->isVisible())
return;

if (sender()) {
disconnect(transientParent(), &QWindow::visibleChanged, this,
&QQuickWindowQmlImpl::setWindowVisibility);
if (QQuickItem *senderItem = qmlobject_cast<QQuickItem *>(sender())) {
disconnect(senderItem, &QQuickItem::windowChanged, this, &QQuickWindowQmlImpl::setWindowVisibility);
} else if (sender()) {
disconnect(transientParent(), &QWindow::visibleChanged, this, &QQuickWindowQmlImpl::setWindowVisibility);
}

// We have deferred window creation until we have the full picture of what
Expand Down
27 changes: 27 additions & 0 deletions tests/auto/quick/qquickloader/data/itemLoaderItemWindow.qml
@@ -0,0 +1,27 @@
import QtQuick 2.0
import QtQuick.Window 2.1

Item {
width: 400
height: 400
objectName: "root Item"

Loader {
sourceComponent: Rectangle {
objectName: "yellow rectangle"
x: 50; y: 50; width: 300; height: 300
color: "yellow"
Window {
objectName: "red transient Window"
width: 100
height: 100
visible: true // makes it harder, because it wants to become visible before root has a window
color: "red"
title: "red"
flags: Qt.Dialog
onVisibilityChanged: console.log("visibility " + visibility)
onVisibleChanged: console.log("visible " + visible)
}
}
}
}
22 changes: 22 additions & 0 deletions tests/auto/quick/qquickloader/data/itemLoaderWindow.qml
@@ -0,0 +1,22 @@
import QtQuick 2.0
import QtQuick.Window 2.1

Item {
width: 400
height: 400
objectName: "root Item"

Loader {
sourceComponent: Window {
objectName: "red transient Window"
width: 100
height: 100
visible: true // makes it harder, because it wants to become visible before root has a window
color: "red"
title: "red"
flags: Qt.Dialog
onVisibilityChanged: console.log("visibility " + visibility)
onVisibleChanged: console.log("visible " + visible)
}
}
}
86 changes: 86 additions & 0 deletions tests/auto/quick/qquickloader/tst_qquickloader.cpp
Expand Up @@ -32,11 +32,15 @@
#include <QtQml/qqmlengine.h>
#include <QtQml/qqmlcomponent.h>
#include <QtQml/qqmlincubator.h>
#include <QtQuick/qquickview.h>
#include <private/qquickloader_p.h>
#include <private/qquickwindowmodule_p.h>
#include "testhttpserver.h"
#include "../../shared/util.h"
#include "../shared/geometrytestutil.h"

Q_LOGGING_CATEGORY(lcTests, "qt.quick.tests")

class SlowComponent : public QQmlComponent
{
Q_OBJECT
Expand Down Expand Up @@ -114,6 +118,8 @@ private slots:
void parented();
void sizeBound();
void QTBUG_30183();
void transientWindow();
void nestedTransientWindow();

void sourceComponentGarbageCollection();

Expand Down Expand Up @@ -1207,6 +1213,86 @@ void tst_QQuickLoader::QTBUG_30183()
delete loader;
}

void tst_QQuickLoader::transientWindow() // QTBUG-52944
{
QQuickView view;
view.setSource(testFileUrl("itemLoaderWindow.qml"));
QQuickItem *root = qobject_cast<QQuickItem*>(view.rootObject());
QVERIFY(root);
QQuickLoader *loader = root->findChild<QQuickLoader *>();
QVERIFY(loader);
QTRY_COMPARE(loader->status(), QQuickLoader::Ready);
QQuickWindowQmlImpl *loadedWindow = qobject_cast<QQuickWindowQmlImpl *>(loader->item());
QVERIFY(loadedWindow);
QCOMPARE(loadedWindow->visibility(), QWindow::Hidden);

QElapsedTimer timer;
qint64 viewVisibleTime = -1;
qint64 loadedWindowVisibleTime = -1;
connect(&view, &QWindow::visibleChanged,
[&viewVisibleTime, &timer]() { viewVisibleTime = timer.elapsed(); } );
connect(loadedWindow, &QQuickWindowQmlImpl::visibilityChanged,
[&loadedWindowVisibleTime, &timer]() { loadedWindowVisibleTime = timer.elapsed(); } );
timer.start();
view.show();

QTest::qWaitForWindowExposed(&view);
QTRY_VERIFY(loadedWindowVisibleTime >= 0);
QVERIFY(viewVisibleTime >= 0);

// now that we're sure they are both visible, which one became visible first?
qCDebug(lcTests) << "transient Window became visible" << (loadedWindowVisibleTime - viewVisibleTime) << "ms after the root Item";
QVERIFY((loadedWindowVisibleTime - viewVisibleTime) >= 0);

QWindowList windows = QGuiApplication::topLevelWindows();
QTRY_COMPARE(windows.size(), 2);

// TODO Ideally we would now close the outer window and make sure the transient window closes too.
// It works during manual testing because of QWindowPrivate::maybeQuitOnLastWindowClosed()
// but quitting an autotest doesn't make sense.
}

void tst_QQuickLoader::nestedTransientWindow() // QTBUG-52944
{
QQuickView view;
view.setSource(testFileUrl("itemLoaderItemWindow.qml"));
QQuickItem *root = qobject_cast<QQuickItem*>(view.rootObject());
QVERIFY(root);
QQuickLoader *loader = root->findChild<QQuickLoader *>();
QVERIFY(loader);
QTRY_COMPARE(loader->status(), QQuickLoader::Ready);
QQuickItem *loadedItem = qobject_cast<QQuickItem *>(loader->item());
QVERIFY(loadedItem);
QQuickWindowQmlImpl *loadedWindow = loadedItem->findChild<QQuickWindowQmlImpl *>();
QVERIFY(loadedWindow);
QCOMPARE(loadedWindow->visibility(), QWindow::Hidden);

QElapsedTimer timer;
qint64 viewVisibleTime = -1;
qint64 loadedWindowVisibleTime = -1;
connect(&view, &QWindow::visibleChanged,
[&viewVisibleTime, &timer]() { viewVisibleTime = timer.elapsed(); } );
connect(loadedWindow, &QQuickWindowQmlImpl::visibilityChanged,
[&loadedWindowVisibleTime, &timer]() { loadedWindowVisibleTime = timer.elapsed(); } );
timer.start();
view.show();

QTest::qWaitForWindowExposed(&view);
QTRY_VERIFY(loadedWindowVisibleTime >= 0);
QVERIFY(viewVisibleTime >= 0);

// now that we're sure they are both visible, which one became visible first?
qCDebug(lcTests) << "transient Window became visible" << (loadedWindowVisibleTime - viewVisibleTime) << "ms after the root Item";
QVERIFY((loadedWindowVisibleTime - viewVisibleTime) >= 0);

QWindowList windows = QGuiApplication::topLevelWindows();
QTRY_COMPARE(windows.size(), 2);

// TODO Ideally we would now close the outer window and make sure the transient window closes too.
// It works during manual testing because of QWindowPrivate::maybeQuitOnLastWindowClosed()
// but quitting an autotest doesn't make sense.
}

void tst_QQuickLoader::sourceComponentGarbageCollection()
{
QQmlComponent component(&engine, testFileUrl("sourceComponentGarbageCollection.qml"));
Expand Down

0 comments on commit 4b014ef

Please sign in to comment.