From 5454cc47c3d578d6789c7906d35af2a166bdb58f Mon Sep 17 00:00:00 2001
From: Vlad Zahorodnii <vlad.zahorodnii@kde.org>
Date: Thu, 23 Mar 2023 10:19:12 +0200
Subject: [PATCH] Preserve stacking order constraints when closing a window

When closing a window, we may need to change parent-child relationship
between windows, but we need to preserve stacking order constraints so
dialogs are placed above their parents when they're closed.
---
 src/waylandwindow.cpp | 18 ++++++++++--------
 src/window.h          |  7 ++-----
 src/x11window.cpp     | 28 +++++++++++++++++++---------
 3 files changed, 31 insertions(+), 22 deletions(-)

diff --git a/src/waylandwindow.cpp b/src/waylandwindow.cpp
index 31c389a2ac..b9daa83a95 100644
--- a/src/waylandwindow.cpp
+++ b/src/waylandwindow.cpp
@@ -221,16 +221,18 @@ void WaylandWindow::doSetActive()
 
 void WaylandWindow::cleanGrouping()
 {
+    // We want to break parent-child relationships, but preserve stacking
+    // order constraints at the same time for window closing animations.
+
     if (transientFor()) {
-        transientFor()->removeTransient(this);
+        transientFor()->removeTransientFromList(this);
+        setTransientFor(nullptr);
     }
-    for (auto it = transients().constBegin(); it != transients().constEnd();) {
-        if ((*it)->transientFor() == this) {
-            removeTransient(*it);
-            it = transients().constBegin(); // restart, just in case something more has changed with the list
-        } else {
-            ++it;
-        }
+
+    const auto children = transients();
+    for (Window *transient : children) {
+        removeTransientFromList(transient);
+        transient->setTransientFor(nullptr);
     }
 }
 
diff --git a/src/window.h b/src/window.h
index 8eb30cab90..accb2c3ab3 100644
--- a/src/window.h
+++ b/src/window.h
@@ -925,6 +925,7 @@ public:
     virtual QRectF transientPlacement(const QRectF &bounds) const;
     const Window *transientFor() const;
     Window *transientFor();
+    void setTransientFor(Window *transientFor);
     /**
      * @returns @c true if transient is the transient_for window for this window,
      *  or recursively the transient_for window
@@ -934,6 +935,7 @@ public:
     const QList<Window *> &transients() const; // Is not indirect
     virtual void addTransient(Window *transient);
     virtual void removeTransient(Window *transient);
+    void removeTransientFromList(Window *cl);
     virtual QList<Window *> mainWindows() const; // Call once before loop , is not indirect
     QList<Window *> allMainWindows() const; // Call once before loop , is indirect
     /**
@@ -1623,11 +1625,6 @@ protected:
     void setupWindowManagementInterface();
     void updateColorScheme();
     void ensurePalette();
-    void setTransientFor(Window *transientFor);
-    /**
-     * Just removes the @p cl from the transients without any further checks.
-     */
-    void removeTransientFromList(Window *cl);
 
     virtual Layer belongsToLayer() const;
     virtual bool belongsToDesktop() const;
diff --git a/src/x11window.cpp b/src/x11window.cpp
index 5fe5665e27..c55f87abf2 100644
--- a/src/x11window.cpp
+++ b/src/x11window.cpp
@@ -3076,19 +3076,29 @@ void X11Window::removeFromMainClients()
 // related lists.
 void X11Window::cleanGrouping()
 {
-    removeFromMainClients();
-    group()->removeMember(this);
-    in_group = nullptr;
+    // We want to break parent-child relationships, but preserve stacking
+    // order constraints at the same time for window closing animations.
 
-    for (auto it = transients().constBegin(); it != transients().constEnd();) {
-        if ((*it)->transientFor() == this) {
-            removeTransient(*it);
-            it = transients().constBegin(); // restart, just in case something more has changed with the list
-        } else {
-            ++it;
+    if (transientFor()) {
+        transientFor()->removeTransientFromList(this);
+        setTransientFor(nullptr);
+    }
+
+    if (groupTransient()) {
+        const auto members = group()->members();
+        for (Window *member : members) {
+            member->removeTransientFromList(this);
         }
     }
 
+    const auto children = transients();
+    for (Window *transient : children) {
+        removeTransientFromList(transient);
+        transient->setTransientFor(nullptr);
+    }
+
+    group()->removeMember(this);
+    in_group = nullptr;
     m_transientForId = XCB_WINDOW_NONE;
 }