From 4c71db756741d35ccb32dc4c32aa1823264c85df Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Tor=20Arne=20Vestb=C3=B8?=
<tor.arne.vestbo@theqtcompany.com>
Date: Thu, 12 Mar 2015 14:00:52 +0100
Subject: [PATCH] QWindow::destroy(): only reset QGuiApp::focus_window and
friends as a last resort
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Resetting focus_window and other internal QGuiApplication variables before
calling setVisible(false) and destroying the platform window means that the
platform window can't reason about whether or not it was the focus window
unless it can resolve that using native APIs. We should let the platform
window take care of resetting the focus window and related states, and
only execute our fallback logic if the plugin doesn't do the right
thing.
We also use QPA to update the state instead of modifying the internal
QGuiApplication variables directly, so that events and signals are
emitted as a result of the reset.
The QLineEdit test gets two added calls to processEvents(), since
assuming that activateWindow() is synchronous is not correct, and
would result in the QMenu resetting the focus window to 0 on destroy.
Task-number: QTBUG-46414
Change-Id: I562788393ed0ffd77d7a4be2279862322f721c1a
Reviewed-by: Błażej Szczygieł <spaz16@wp.pl>
Reviewed-by: Tor Arne Vestbø <tor.arne.vestbo@theqtcompany.com>
---
src/gui/kernel/qplatformwindow.cpp | 2 +
src/gui/kernel/qwindow.cpp | 46 ++++++++++++++-----
.../accessible/qaccessiblewidgetfactory.cpp | 9 ++++
tests/auto/gui/kernel/qwindow/tst_qwindow.cpp | 13 ++++++
.../widgets/qlineedit/tst_qlineedit.cpp | 3 +-
5 files changed, 61 insertions(+), 12 deletions(-)
diff --git a/src/gui/kernel/qplatformwindow.cpp b/src/gui/kernel/qplatformwindow.cpp
index aea029b7f5c..fd3ce403426 100644
--- a/src/gui/kernel/qplatformwindow.cpp
+++ b/src/gui/kernel/qplatformwindow.cpp
@@ -62,6 +62,8 @@ QPlatformWindow::QPlatformWindow(QWindow *window)
*/
QPlatformWindow::~QPlatformWindow()
{
+ // We don't flush window system events here, as the event will be
+ // delivered with a platform window that is half-way destroyed.
}
/*!
diff --git a/src/gui/kernel/qwindow.cpp b/src/gui/kernel/qwindow.cpp
index 21734f16198..11ae3aa5c79 100644
--- a/src/gui/kernel/qwindow.cpp
+++ b/src/gui/kernel/qwindow.cpp
@@ -1648,15 +1648,6 @@ void QWindow::destroy()
}
}
- if (QGuiApplicationPrivate::focus_window == this)
- QGuiApplicationPrivate::focus_window = parent();
- if (QGuiApplicationPrivate::currentMouseWindow == this)
- QGuiApplicationPrivate::currentMouseWindow = parent();
- if (QGuiApplicationPrivate::currentMousePressWindow == this)
- QGuiApplicationPrivate::currentMousePressWindow = parent();
- if (QGuiApplicationPrivate::tabletPressTarget == this)
- QGuiApplicationPrivate::tabletPressTarget = parent();
-
bool wasVisible = isVisible();
d->visibilityOnDestroy = wasVisible && d->platformWindow;
@@ -1665,11 +1656,44 @@ void QWindow::destroy()
QPlatformSurfaceEvent e(QPlatformSurfaceEvent::SurfaceAboutToBeDestroyed);
QGuiApplication::sendEvent(this, &e);
- delete d->platformWindow;
+ QPlatformWindow *platformWindow = d->platformWindow;
+ d->platformWindow = 0;
+
+ // We flush before deleting the platform window so that any pending activation
+ // events for the window will be delivered.
+ QWindowSystemInterface::flushWindowSystemEvents();
+
+ delete platformWindow;
+
+ // Then we flush again so that if the platform plugin sent any deactivation
+ // events as a result of being destroyed, we can pick that up by looking at
+ // QGuiApplicationPrivate::focus_window, which will be up to date.
+ QWindowSystemInterface::flushWindowSystemEvents();
+
d->resizeEventPending = true;
d->receivedExpose = false;
d->exposed = false;
- d->platformWindow = 0;
+
+ // Ensure Qt doesn't refer to a destroyed QWindow if the platform plugin
+ // didn't reset the window activation or other states as part of setVisible
+ // or its destruction. We make a best guess of transferring to the parent
+ // window, as this is what most window managers will do. We go through the
+ // QWindowSystemInterface so that the proper signals and events are sent
+ // as a result of the reset.
+ if (QGuiApplicationPrivate::focus_window == this)
+ QWindowSystemInterface::handleWindowActivated(parent());
+ if (QGuiApplicationPrivate::currentMouseWindow == this)
+ QWindowSystemInterface::handleEnterLeaveEvent(parent(), this);
+
+ // FIXME: Handle these two though QPA like the others. Unfortunately both
+ // processMouseEvent and processTabletEvent in QGuiApplication have conditions
+ // that make sending the event though QPA not feasable right now.
+ if (QGuiApplicationPrivate::currentMousePressWindow == this)
+ QGuiApplicationPrivate::currentMousePressWindow = parent();
+ if (QGuiApplicationPrivate::tabletPressTarget == this)
+ QGuiApplicationPrivate::tabletPressTarget = parent();
+
+ QWindowSystemInterface::flushWindowSystemEvents();
if (wasVisible)
d->maybeQuitOnLastWindowClosed();
diff --git a/src/widgets/accessible/qaccessiblewidgetfactory.cpp b/src/widgets/accessible/qaccessiblewidgetfactory.cpp
index 4fa7c894826..00e21da34d4 100644
--- a/src/widgets/accessible/qaccessiblewidgetfactory.cpp
+++ b/src/widgets/accessible/qaccessiblewidgetfactory.cpp
@@ -43,6 +43,7 @@
#include <qtreeview.h>
#include <qvariant.h>
#include <qaccessible.h>
+#include <private/qwidget_p.h>
#ifndef QT_NO_ACCESSIBILITY
@@ -53,7 +54,15 @@ QAccessibleInterface *qAccessibleFactory(const QString &classname, QObject *obje
QAccessibleInterface *iface = 0;
if (!object || !object->isWidgetType())
return iface;
+
+ // QWidget emits destroyed() from its destructor instead of letting the QObject
+ // destructor do it, which means the QWidget is unregistered from the accessibillity
+ // cache. But QWidget destruction also emits enter and leave events, which may end
+ // up here, so we have to ensure that we don't fill the cache with an entry of
+ // a widget that is going away.
QWidget *widget = static_cast<QWidget*>(object);
+ if (QWidgetPrivate::get(widget)->data.in_destructor)
+ return iface;
if (false) {
#ifndef QT_NO_LINEEDIT
diff --git a/tests/auto/gui/kernel/qwindow/tst_qwindow.cpp b/tests/auto/gui/kernel/qwindow/tst_qwindow.cpp
index a89f0da4d27..794d549b57a 100644
--- a/tests/auto/gui/kernel/qwindow/tst_qwindow.cpp
+++ b/tests/auto/gui/kernel/qwindow/tst_qwindow.cpp
@@ -94,6 +94,7 @@ private slots:
void modalWindowPosition();
void windowsTransientChildren();
void requestUpdate();
+ void destroyResetsFocusWindow();
void initTestCase();
void stateChange_data();
void stateChange();
@@ -1804,6 +1805,18 @@ void tst_QWindow::requestUpdate()
QTRY_COMPARE(window.received(QEvent::UpdateRequest), 2);
}
+void tst_QWindow::destroyResetsFocusWindow()
+{
+ QWindow window;
+ window.showNormal();
+ window.requestActivate();
+ QVERIFY(QTest::qWaitForWindowActive(&window));
+ QCOMPARE(qGuiApp->focusWindow(), &window);
+
+ window.destroy();
+ QVERIFY(!qGuiApp->focusWindow());
+}
+
#include <tst_qwindow.moc>
QTEST_MAIN(tst_QWindow)
diff --git a/tests/auto/widgets/widgets/qlineedit/tst_qlineedit.cpp b/tests/auto/widgets/widgets/qlineedit/tst_qlineedit.cpp
index b46609c371e..bf176258038 100644
--- a/tests/auto/widgets/widgets/qlineedit/tst_qlineedit.cpp
+++ b/tests/auto/widgets/widgets/qlineedit/tst_qlineedit.cpp
@@ -3570,7 +3570,7 @@ void tst_QLineEdit::task174640_editingFinished()
QVERIFY(QTest::qWaitForWindowExposed(testMenu1));
QTest::qWait(20);
mw.activateWindow();
-
+ qApp->processEvents();
delete testMenu1;
QCOMPARE(editingFinishedSpy.count(), 0);
QTRY_VERIFY(le1->hasFocus());
@@ -3582,6 +3582,7 @@ void tst_QLineEdit::task174640_editingFinished()
QVERIFY(QTest::qWaitForWindowExposed(testMenu2));
QTest::qWait(20);
mw.activateWindow();
+ qApp->processEvents();
delete testMenu2;
QCOMPARE(editingFinishedSpy.count(), 1);
}
--
GitLab