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 <davidedmundson@kde.org>
This commit is contained in:
Vlad Zahorodnii 2023-11-14 13:31:28 +02:00 committed by David Edmundson
parent fa1658f8bf
commit 061340e9c6
4 changed files with 23 additions and 22 deletions

View File

@ -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<QWaylandXdgSurface *>(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);

View File

@ -171,7 +171,6 @@ private:
QScopedPointer<QWaylandXdgDecorationManagerV1> m_xdgDecorationManager;
QScopedPointer<QWaylandXdgActivationV1> m_xdgActivation;
QScopedPointer<QWaylandXdgExporterV2> m_xdgExporter;
QWaylandXdgSurface::Popup *m_topmostGrabbingPopup = nullptr;
friend class QWaylandXdgSurface;
};

View File

@ -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

View File

@ -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;
};