From 061340e9c6bc850132381600be18d0dc3dbb17e6 Mon Sep 17 00:00:00 2001 From: Vlad Zahorodnii Date: Tue, 14 Nov 2023 13:31:28 +0200 Subject: [PATCH] Client: Move topmost grabbing popup tracking to QWaylandWindow If the effective transient parent is different from QWaylandWindow::transientParent(), then the popups may be closed in wrong order and producing an xdg-shell protocol error. This change lifts topmost popup tracking from the xdg-shell plugin to QWaylandWindow so it can guess the correct transient parent and the xdg-shell plugin doesn't have to pick a different parent behind our back. Fixes: QTBUG-119110 Pick-to: 6.6 Change-Id: I7c5f780b7bd4c3362aa7b22762ff336ae908ff70 Reviewed-by: David Edmundson --- .../xdg-shell/qwaylandxdgshell.cpp | 18 --------------- .../xdg-shell/qwaylandxdgshell_p.h | 1 - .../platforms/wayland/qwaylandwindow.cpp | 23 +++++++++++++++++-- .../platforms/wayland/qwaylandwindow_p.h | 3 ++- 4 files changed, 23 insertions(+), 22 deletions(-) diff --git a/src/plugins/platforms/wayland/plugins/shellintegration/xdg-shell/qwaylandxdgshell.cpp b/src/plugins/platforms/wayland/plugins/shellintegration/xdg-shell/qwaylandxdgshell.cpp index 454fa243b9b..e6cade0818f 100644 --- a/src/plugins/platforms/wayland/plugins/shellintegration/xdg-shell/qwaylandxdgshell.cpp +++ b/src/plugins/platforms/wayland/plugins/shellintegration/xdg-shell/qwaylandxdgshell.cpp @@ -227,9 +227,6 @@ QWaylandXdgSurface::Popup::~Popup() destroy(); if (m_grabbing) { - auto *shell = m_xdgSurface->m_shell; - Q_ASSERT(shell->m_topmostGrabbingPopup == this); - shell->m_topmostGrabbingPopup = m_parentXdgSurface ? m_parentXdgSurface->m_popup : nullptr; m_grabbing = false; // Synthesize Qt enter/leave events for popup @@ -261,7 +258,6 @@ void QWaylandXdgSurface::Popup::resetConfiguration() void QWaylandXdgSurface::Popup::grab(QWaylandInputDevice *seat, uint serial) { - m_xdgSurface->m_shell->m_topmostGrabbingPopup = this; xdg_popup::grab(seat->wl_seat(), serial); m_grabbing = true; } @@ -574,20 +570,6 @@ void QWaylandXdgSurface::setPopup(QWaylandWindow *parent) void QWaylandXdgSurface::setGrabPopup(QWaylandWindow *parent, QWaylandInputDevice *device, int serial) { - auto parentXdgSurface = qobject_cast(parent->shellSurface()); - auto *top = m_shell->m_topmostGrabbingPopup; - - if (top && top->m_xdgSurface != parentXdgSurface) { - qCWarning(lcQpaWayland) << "setGrabPopup called with a parent," << parentXdgSurface - << "which does not match the current topmost grabbing popup," - << top->m_xdgSurface << "According to the xdg-shell protocol, this" - << "is not allowed. The wayland QPA plugin is currently handling" - << "it by setting the parent to the topmost grabbing popup." - << "Note, however, that this may cause positioning errors and" - << "popups closing unxpectedly because xdg-shell mandate that child" - << "popups close before parents"; - parent = top->m_xdgSurface->m_window; - } setPopup(parent); m_popup->grab(device, serial); diff --git a/src/plugins/platforms/wayland/plugins/shellintegration/xdg-shell/qwaylandxdgshell_p.h b/src/plugins/platforms/wayland/plugins/shellintegration/xdg-shell/qwaylandxdgshell_p.h index c5e6a98960e..0310706fd94 100644 --- a/src/plugins/platforms/wayland/plugins/shellintegration/xdg-shell/qwaylandxdgshell_p.h +++ b/src/plugins/platforms/wayland/plugins/shellintegration/xdg-shell/qwaylandxdgshell_p.h @@ -171,7 +171,6 @@ private: QScopedPointer m_xdgDecorationManager; QScopedPointer m_xdgActivation; QScopedPointer m_xdgExporter; - QWaylandXdgSurface::Popup *m_topmostGrabbingPopup = nullptr; friend class QWaylandXdgSurface; }; diff --git a/src/plugins/platforms/wayland/qwaylandwindow.cpp b/src/plugins/platforms/wayland/qwaylandwindow.cpp index a2af701ddd7..b547d056b98 100644 --- a/src/plugins/platforms/wayland/qwaylandwindow.cpp +++ b/src/plugins/platforms/wayland/qwaylandwindow.cpp @@ -41,6 +41,7 @@ namespace QtWaylandClient { Q_LOGGING_CATEGORY(lcWaylandBackingstore, "qt.qpa.wayland.backingstore") QWaylandWindow *QWaylandWindow::mMouseGrab = nullptr; +QWaylandWindow *QWaylandWindow::mTopPopup = nullptr; QWaylandWindow::QWaylandWindow(QWindow *window, QWaylandDisplay *display) : QPlatformWindow(window) @@ -122,7 +123,22 @@ void QWaylandWindow::initWindow() } else if (shouldCreateShellSurface()) { Q_ASSERT(!mShellSurface); Q_ASSERT(mShellIntegration); - mTransientParent = closestTransientParent(); + mTransientParent = guessTransientParent(); + if (mTransientParent) { + if (window()->type() == Qt::Popup) { + if (mTopPopup && mTopPopup != mTransientParent) { + qCWarning(lcQpaWayland) << "Creating a popup with a parent," << mTransientParent->window() + << "which does not match the current topmost grabbing popup," + << mTopPopup->window() << "With some shell surface protocols, this" + << "is not allowed. The wayland QPA plugin is currently handling" + << "it by setting the parent to the topmost grabbing popup." + << "Note, however, that this may cause positioning errors and" + << "popups closing unxpectedly. Please fix the transient parent of the popup."; + mTransientParent = mTopPopup; + } + mTopPopup = this; + } + } mShellSurface = mShellIntegration->createShellSurface(this); if (mShellSurface) { @@ -261,6 +277,9 @@ void QWaylandWindow::reset() { closeChildPopups(); + if (mTopPopup == this) + mTopPopup = mTransientParent && (mTransientParent->window()->type() == Qt::Popup) ? mTransientParent : nullptr; + if (mSurface) { emit wlSurfaceDestroyed(); QWriteLocker lock(&mSurfaceLock); @@ -1129,7 +1148,7 @@ QWaylandWindow *QWaylandWindow::transientParent() const return mTransientParent; } -QWaylandWindow *QWaylandWindow::closestTransientParent() const +QWaylandWindow *QWaylandWindow::guessTransientParent() const { // Take the closest window with a shell surface, since the transient parent may be a // QWidgetWindow or some other window without a shell surface, which is then not able to diff --git a/src/plugins/platforms/wayland/qwaylandwindow_p.h b/src/plugins/platforms/wayland/qwaylandwindow_p.h index 2e3f322fb38..aa125e6eab4 100644 --- a/src/plugins/platforms/wayland/qwaylandwindow_p.h +++ b/src/plugins/platforms/wayland/qwaylandwindow_p.h @@ -355,7 +355,7 @@ private: void setScale(qreal newScale); void sendRecursiveExposeEvent(); - QWaylandWindow *closestTransientParent() const; + QWaylandWindow *guessTransientParent() const; void addChildPopup(QWaylandWindow *child); void removeChildPopup(QWaylandWindow *child); @@ -367,6 +367,7 @@ private: void handleFrameCallback(struct ::wl_callback* callback); static QWaylandWindow *mMouseGrab; + static QWaylandWindow *mTopPopup; friend class QWaylandSubSurface; };