server: Unmap popup on popup_done event

Seems some applications don't expect their popups to be unmapped without
their consent, so they make act strangely, but at least it doesn't
crash.
Fixes #117

src/lib.rs

@@ -29,6 +29,7 @@ pub trait XConnection: Sized + 'static {
         data: Self::ExtraData,
     );
     fn close_window(&mut self, window: x::Window, data: Self::ExtraData);
+    fn unmap_window(&mut self, window: x::Window);
     fn raise_to_top(&mut self, window: x::Window);
 }
 

src/server/event.rs

@@ -256,7 +256,7 @@ impl SurfaceData {
         }
     }
 
-    fn popup_event<C: XConnection>(&mut self, event: xdg_popup::Event, _: &mut ServerState<C>) {
+    fn popup_event<C: XConnection>(&mut self, event: xdg_popup::Event, state: &mut ServerState<C>) {
         match event {
             xdg_popup::Event::Configure {
                 x,
@@ -273,6 +273,13 @@ impl SurfaceData {
                 });
             }
             xdg_popup::Event::Repositioned { .. } => {}
+            xdg_popup::Event::PopupDone => {
+                state
+                    .connection
+                    .as_mut()
+                    .unwrap()
+                    .unmap_window(self.window.unwrap());
+            }
             other => todo!("{other:?}"),
         }
     }

src/server/tests.rs

@@ -247,6 +247,10 @@ impl super::XConnection for FakeXConnection {
             "Unknown window: {window:?}"
         );
     }
+
+    fn unmap_window(&mut self, _: x::Window) {
+        todo!()
+    }
 }
 
 type FakeServerState = ServerState<FakeXConnection>;

src/xstate/mod.rs

@@ -1024,6 +1024,12 @@ impl XConnection for RealConnection {
         }
     }
 
+    fn unmap_window(&mut self, window: x::Window) {
+        unwrap_or_skip_bad_window!(self
+            .connection
+            .send_and_check_request(&x::UnmapWindow { window }));
+    }
+
     fn raise_to_top(&mut self, window: x::Window) {
         unwrap_or_skip_bad_window!(self.connection.send_and_check_request(&x::ConfigureWindow {
             window,

tests/integration.rs

@@ -211,6 +211,42 @@ impl Fixture {
         surface
     }
 
+    #[track_caller]
+    fn map_as_popup(
+        &mut self,
+        connection: &mut Connection,
+        window: x::Window,
+        x: i16,
+        y: i16,
+        width: u16,
+        height: u16,
+    ) -> testwl::SurfaceId {
+        connection.map_window(window);
+        self.wait_and_dispatch();
+        let surface = self
+            .testwl
+            .last_created_surface_id()
+            .expect("No surface created");
+        let data = self.testwl.get_surface_data(surface).unwrap();
+        assert!(
+            matches!(data.role, Some(testwl::SurfaceRole::Popup(_))),
+            "surface role was wrong: {:?}",
+            data.role
+        );
+        self.testwl.configure_popup(surface);
+        self.wait_and_dispatch();
+        let geometry = connection.get_reply(&x::GetGeometry {
+            drawable: x::Drawable::Window(window),
+        });
+
+        assert_eq!(geometry.x(), x);
+        assert_eq!(geometry.y(), y);
+        assert_eq!(geometry.width(), width);
+        assert_eq!(geometry.height(), height);
+
+        surface
+    }
+
     /// Triggers a Wayland side toplevel Close event and processes the corresponding
     /// X11 side WM_DELETE_WINDOW client message
     fn wm_delete_window(
@@ -1362,3 +1398,23 @@ fn fake_selection_targets() {
         std::str::from_utf8(data).unwrap()
     );
 }
+
+#[test]
+fn popup_done() {
+    let mut f = Fixture::new();
+    let mut conn = Connection::new(&f.display);
+    let toplevel = conn.new_window(conn.root, 0, 0, 20, 20, false);
+    f.map_as_toplevel(&mut conn, toplevel);
+
+    let popup = conn.new_window(conn.root, 0, 0, 20, 20, true);
+    let surface = f.map_as_popup(&mut conn, popup, 0, 0, 20, 20);
+
+    f.testwl.popup_done(surface);
+    f.wait_and_dispatch();
+
+    let reply = conn
+        .wait_for_reply(conn.send_request(&x::GetWindowAttributes { window: popup }))
+        .expect("Couldn't get window attributes");
+
+    assert_eq!(reply.map_state(), x::MapState::Unmapped);
+}

testwl/src/lib.rs

@@ -268,7 +268,7 @@ impl State {
     }
 
     #[track_caller]
-    pub fn configure_popup(&mut self, surface_id: SurfaceId) {
+    fn configure_popup(&mut self, surface_id: SurfaceId) {
         let surface = self.surfaces.get_mut(&surface_id).unwrap();
         let Some(SurfaceRole::Popup(p)) = &mut surface.role else {
             panic!("Surface does not have popup role: {:?}", surface.role);
@@ -291,6 +291,15 @@ impl State {
             other => panic!("Surface does not have toplevel role: {:?}", other),
         }
     }
+
+    #[track_caller]
+    fn popup_done(&mut self, surface_id: SurfaceId) {
+        let surface = self.surfaces.get_mut(&surface_id).unwrap();
+        let Some(SurfaceRole::Popup(p)) = &mut surface.role else {
+            panic!("Surface does not have popup role: {:?}", surface.role);
+        };
+        p.popup.popup_done();
+    }
 }
 
 macro_rules! simple_global_dispatch {
@@ -492,6 +501,12 @@ impl Server {
         self.display.flush_clients().unwrap();
     }
 
+    #[track_caller]
+    pub fn popup_done(&mut self, surface_id: SurfaceId) {
+        self.state.popup_done(surface_id);
+        self.display.flush_clients().unwrap();
+    }
+
     #[track_caller]
     pub fn close_toplevel(&mut self, surface_id: SurfaceId) {
         let toplevel = self.state.get_toplevel(surface_id);