Revoke fullscreen stacking exceptions when window parents change

Clear the old exception when a dialog is unparented or reparented, and remove child references when a parent is destroyed. Preserve floating placement while rebuilding the stacking order.

Fixes #55

compositor/src/handlers/xdg_shell.rs

@@ -60,10 +60,7 @@ impl XdgShellHandler for Ferese {
     fn parent_changed(&mut self, surface: ToplevelSurface) {
         self.refresh_capture_privacy();
         self.refresh_idle_inhibition();
-        let Some(parent) = surface.parent() else {
+        let parent = surface.parent().and_then(|parent| self.windows.id_for_surface(&parent));
-            return;
-        };
-        let parent = self.windows.id_for_surface(&parent);
         let child = self
             .windows
             .id_for_surface(surface.wl_surface())
@@ -77,8 +74,8 @@ impl XdgShellHandler for Ferese {
             })
             .cloned();
 
-        if let (Some(parent), Some(child)) = (parent, child) {
+        if let Some(child) = child {
-            self.make_window_transient(&child, parent);
+            self.update_window_parent(&child, parent);
         }
     }
 

compositor/src/state/window_lifecycle_tests.rs

@@ -1334,3 +1334,70 @@ fn shell_can_move_a_window_to_the_current_workspace_on_another_output() {
     assert_eq!(state.workspaces.workspace_for_window(id), Some(destination));
     assert!(state.move_managed_window_to_workspace(id, destination));
 }
+
+fn change_transient_parent(state: &mut Ferese, child: &Window, parent: Option<&Window>) {
+    use smithay::wayland::{
+        compositor::with_states,
+        shell::xdg::{XdgShellHandler, XdgToplevelSurfaceData},
+    };
+    let toplevel = child.toplevel().unwrap();
+    with_states(toplevel.wl_surface(), |states| {
+        states
+            .data_map
+            .get::<XdgToplevelSurfaceData>()
+            .unwrap()
+            .lock()
+            .unwrap()
+            .parent = parent.map(|parent| parent.toplevel().unwrap().wl_surface().clone());
+    });
+    state.parent_changed(toplevel.clone());
+}
+
+#[test]
+fn transient_fullscreen_exception_follows_parent_changes() {
+    if !crate::startup_tests::private_runtime(
+        "state::window_lifecycle_tests::transient_fullscreen_exception_follows_parent_changes",
+    ) {
+        return;
+    }
+    let (mut events, mut state, _output) = fixture();
+    let (parent, _parent_wire) = window(&mut state, &mut events, 0xff112233);
+    let (other, _other_wire) = window(&mut state, &mut events, 0xff223344);
+    let (child, _child_wire) = window(&mut state, &mut events, 0xff334455);
+    let parent_id = state.windows.ids()[&parent];
+    let other_id = state.windows.ids()[&other];
+    let child_id = state.windows.ids()[&child];
+    state.set_window_fullscreen(parent_id, true);
+    change_transient_parent(&mut state, &child, Some(&parent));
+    assert_eq!(state.floating_above_fullscreen.get(&child_id), Some(&parent_id));
+    change_transient_parent(&mut state, &child, None);
+    assert!(
+        !state.floating_above_fullscreen.contains_key(&child_id),
+        "unparenting must revoke fullscreen priority"
+    );
+    assert!(
+        matches!(
+            state.workspaces.placement(child_id),
+            Some(WindowPlacement::Floating { .. })
+        ),
+        "unparenting must not retile the dialog"
+    );
+    change_transient_parent(&mut state, &child, Some(&parent));
+    change_transient_parent(&mut state, &child, Some(&other));
+    assert!(
+        !state.floating_above_fullscreen.contains_key(&child_id),
+        "an ordinary parent must not retain the old fullscreen exception"
+    );
+    state.set_window_fullscreen(other_id, true);
+    change_transient_parent(&mut state, &child, Some(&other));
+    assert_eq!(state.floating_above_fullscreen.get(&child_id), Some(&other_id));
+    state.remove_tiled_window(&other);
+    assert!(
+        state
+            .floating_above_fullscreen
+            .values()
+            .all(|parent| *parent != other_id),
+        "removing a parent must revoke its children's exceptions"
+    );
+    assert!(state.windows.window(child_id).is_some());
+}

compositor/src/state/windows.rs

@@ -415,17 +415,27 @@ impl Ferese {
         self.restore_keyboard_focus();
     }
 
-    pub fn make_window_transient(&mut self, window: &Window, parent: WindowId) {
+    pub fn update_window_parent(&mut self, window: &Window, parent: Option<WindowId>) {
         let Some(id) = self.windows.ids().get(window).copied() else {
             return;
         };
-
+        // A stacking exception belongs to the current relationship. Revoke it
-        if self.workspaces.workspace_for_window(id) != self.workspaces.workspace_for_window(parent) {
+        // even when the replacement parent is absent or cannot be placed here.
-            tracing::warn!(?id, ?parent, "ignored transient parent on another workspace");
+        let removed = self.floating_above_fullscreen.remove(&id).is_some();
-            return;
+        if removed {
+            self.invalidate_window_stacking();
         }
-
+        let placement = parent.and_then(|parent| {
-        let Some(bounds) = self.floating_bounds_for_window(parent) else {
+            if self.workspaces.workspace_for_window(id) != self.workspaces.workspace_for_window(parent) {
+                tracing::warn!(?id, ?parent, "ignored transient parent on another workspace");
+                return None;
+            }
+            self.floating_bounds_for_window(parent).map(|bounds| (parent, bounds))
+        });
+        let Some((parent, bounds)) = placement else {
+            if removed {
+                self.relayout();
+            }
             return;
         };
 
@@ -441,11 +451,19 @@ impl Ferese {
                 .toggle_floating(id, rect, Axis::Horizontal, 0.5)
                 .map(|_| ()),
             Some(WindowPlacement::Floating { .. }) => self.workspaces.set_floating_rect(id, rect),
-            None => return,
+            None => {
+                if removed {
+                    self.relayout();
+                }
+                return;
+            }
         };
 
         if let Err(error) = result {
             tracing::warn!(%error, ?id, ?parent, "failed to apply transient placement");
+            if removed {
+                self.relayout();
+            }
             return;
         }
 
@@ -488,7 +506,8 @@ impl Ferese {
         self.render.remove_window(id);
         self.capture_render.remove_window(id);
         self.window_stack.remove(id);
-        self.floating_above_fullscreen.remove(&id);
+        self.floating_above_fullscreen
+            .retain(|child, parent| *child != id && *parent != id);
         if let Err(error) = self.workspaces.remove_window(id) {
             tracing::error!(%error, ?id, "failed to remove window from layout");
         }