From 76d9d869072d22995fa2cc6cb5fec17c42230dc4 Mon Sep 17 00:00:00 2001 From: Richard Moe Gustavsen Date: Mon, 22 Jun 2020 13:15:12 +0200 Subject: [PATCH] qnsview: don't active QWindows inside NSWindows that are not key MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A QWindow should only become Active when it's inside an NSWindow that is Key. If the NSWindow is not key, we need to wait for it to be so, and handle window activation from QCocoaWindow::windowDidBecomeKey() instead. Otherwise Qt will report a QWindow as Active when, in reality, it is not. Change-Id: Ib7e63b374f26af527a668c7f7d863c4168a4446d Reviewed-by: Tor Arne Vestbø --- src/plugins/platforms/cocoa/qcocoawindow.mm | 14 ++++++++++--- src/plugins/platforms/cocoa/qnsview.mm | 21 ++++++++++++++++++- .../widgets/kernel/qwidget/tst_qwidget.cpp | 21 +++++++++++++++++++ .../qopenglwidget/tst_qopenglwidget.cpp | 10 +++++++++ 4 files changed, 62 insertions(+), 4 deletions(-) diff --git a/src/plugins/platforms/cocoa/qcocoawindow.mm b/src/plugins/platforms/cocoa/qcocoawindow.mm index 667db88a91..af490d49b5 100644 --- a/src/plugins/platforms/cocoa/qcocoawindow.mm +++ b/src/plugins/platforms/cocoa/qcocoawindow.mm @@ -1212,9 +1212,17 @@ void QCocoaWindow::windowDidBecomeKey() QWindowSystemInterface::handleEnterEvent(m_enterLeaveTargetWindow, windowPoint, screenPoint); } - if (!windowIsPopupType()) - QWindowSystemInterface::handleWindowActivated( - window(), Qt::ActiveWindowFocusReason); + QNSView *firstResponderView = qt_objc_cast(m_view.window.firstResponder); + if (!firstResponderView) + return; + + const QCocoaWindow *focusCocoaWindow = firstResponderView.platformWindow; + if (focusCocoaWindow->windowIsPopupType()) + return; + + // See also [QNSView becomeFirstResponder] + QWindowSystemInterface::handleWindowActivated( + focusCocoaWindow->window(), Qt::ActiveWindowFocusReason); } void QCocoaWindow::windowDidResignKey() diff --git a/src/plugins/platforms/cocoa/qnsview.mm b/src/plugins/platforms/cocoa/qnsview.mm index bd7e10e2c7..471fa368c3 100644 --- a/src/plugins/platforms/cocoa/qnsview.mm +++ b/src/plugins/platforms/cocoa/qnsview.mm @@ -310,8 +310,27 @@ QT_NAMESPACE_ALIAS_OBJC_CLASS(QNSViewMouseMoveHelper); return NO; if ([self isTransparentForUserInput]) return NO; - if (!m_platformWindow->windowIsPopupType()) + + if (!m_platformWindow->windowIsPopupType() + && (!self.window.canBecomeKeyWindow || self.window.keyWindow)) { + // Calling handleWindowActivated for a QWindow has two effects: first, it + // will set the QWindow (and all other QWindows in the same hierarchy) + // as Active. Being Active means that the window should appear active from + // a style perspective (according to QWindow::isActive()). The second + // effect is that it will set QQuiApplication::focusWindow() to point to + // the QWindow. The latter means that the QWindow should have keyboard + // focus. But those two are not necessarily the same; A tool window could e.g be + // rendered as Active while the parent window, which is also Active, has + // input focus. But we currently don't distiguish between that cleanly in Qt. + // Since we don't want a QWindow to be rendered as Active when the NSWindow + // it belongs to is not key, we skip calling handleWindowActivated when + // that is the case. Instead, we wait for the window to become key, and handle + // QWindow activation from QCocoaWindow::windowDidBecomeKey instead. The only + // exception is if the window can never become key, in which case we naturally + // cannot wait for that to happen. QWindowSystemInterface::handleWindowActivated([self topLevelWindow], Qt::ActiveWindowFocusReason); + } + return YES; } diff --git a/tests/auto/widgets/kernel/qwidget/tst_qwidget.cpp b/tests/auto/widgets/kernel/qwidget/tst_qwidget.cpp index 1ec296211f..6e304f2152 100644 --- a/tests/auto/widgets/kernel/qwidget/tst_qwidget.cpp +++ b/tests/auto/widgets/kernel/qwidget/tst_qwidget.cpp @@ -5685,6 +5685,16 @@ void tst_QWidget::moveChild() parent.showNormal(); QVERIFY(QTest::qWaitForWindowExposed(&parent)); + if (QGuiApplicationPrivate::platformIntegration()->hasCapability(QPlatformIntegration::WindowActivation)) { + // On some platforms (macOS), the palette will be different depending on if a + // window is active or not. And because of that, the whole window will be + // repainted when going from Inactive to Active. So wait for the window to be + // active before we continue, so the activation doesn't happen at a random + // time below. And call processEvents to have the paint events delivered right away. + QVERIFY(QTest::qWaitForWindowActive(&parent)); + qApp->processEvents(); + } + QTRY_COMPARE(parent.r, QRegion(parent.rect()) - child.geometry()); QTRY_COMPARE(child.r, QRegion(child.rect())); @@ -8029,6 +8039,17 @@ void tst_QWidget::hideOpaqueChildWhileHidden() w.show(); QVERIFY(QTest::qWaitForWindowExposed(&w)); + + if (QGuiApplicationPrivate::platformIntegration()->hasCapability(QPlatformIntegration::WindowActivation)) { + // On some platforms (macOS), the palette will be different depending on if a + // window is active or not. And because of that, the whole window will be + // repainted when going from Inactive to Active. So wait for the window to be + // active before we continue, so the activation doesn't happen at a random + // time below. And call processEvents to have the paint events delivered right away. + QVERIFY(QTest::qWaitForWindowActive(&w)); + qApp->processEvents(); + } + QTRY_COMPARE(child2.r, QRegion(child2.rect())); child.r = QRegion(); child2.r = QRegion(); diff --git a/tests/auto/widgets/widgets/qopenglwidget/tst_qopenglwidget.cpp b/tests/auto/widgets/widgets/qopenglwidget/tst_qopenglwidget.cpp index ccd8d0ec05..099a099c13 100644 --- a/tests/auto/widgets/widgets/qopenglwidget/tst_qopenglwidget.cpp +++ b/tests/auto/widgets/widgets/qopenglwidget/tst_qopenglwidget.cpp @@ -364,6 +364,16 @@ void tst_QOpenGLWidget::asViewport() widget.show(); QVERIFY(QTest::qWaitForWindowExposed(&widget)); + if (QGuiApplicationPrivate::platformIntegration()->hasCapability(QPlatformIntegration::WindowActivation)) { + // On some platforms (macOS), the palette will be different depending on if a + // window is active or not. And because of that, the whole window will be + // repainted when going from Inactive to Active. So wait for the window to be + // active before we continue, so the activation doesn't happen at a random + // time below. And call processEvents to have the paint events delivered right away. + QVERIFY(QTest::qWaitForWindowActive(&widget)); + qApp->processEvents(); + } + QVERIFY(view->paintCount() > 0); view->resetPaintCount();