Use wl_keyboard instead of toplevel state for focus

Rare TODO comment actually done.
Fixes #64

src/server/event.rs

@@ -253,15 +253,6 @@ impl SurfaceData {
                 states,
             } => {
                 debug!("configuring toplevel {width}x{height}, {states:?}");
-                let activated = states.contains(&(u32::from(xdg_toplevel::State::Activated) as u8));
-
-                if activated {
-                    // Technically this is wrong - activated doesn't necessarily mean focused
-                    // - but it works and no one's complained yet.
-                    // TODO: base focus on keyboard enter instead.
-                    state.to_focus = Some(self.window.unwrap());
-                }
-
                 if let Some(SurfaceRole::Toplevel(Some(toplevel))) = &mut self.role {
                     let prev_fs = toplevel.fullscreen;
                     toplevel.fullscreen =
@@ -539,9 +530,11 @@ impl HandleEvent for Keyboard {
                 surface,
                 keys,
             } => {
+                let key: ObjectKey = surface.data().copied().unwrap();
+                if let Some(data) = state.objects.get(key).map(|o| <_ as AsRef<SurfaceData>>::as_ref(o)) {
                     state.last_kb_serial = Some(serial);
-                if let Some(surface_data) = state.get_server_surface_from_client(surface) {
+                    state.to_focus = Some(data.window.unwrap());
-                    self.server.enter(serial, surface_data, keys);
+                    self.server.enter(serial, &data.server, keys);
                 }
             }
             _ => simple_event_shunt! {

src/server/tests.rs

@@ -92,7 +92,7 @@ struct Compositor {
     compositor: TestObject<WlCompositor>,
     shm: TestObject<WlShm>,
     shell: TestObject<XwaylandShellV1>,
-    seat: TestObject<WlSeat>
+    seat: TestObject<WlSeat>,
 }
 
 }
@@ -340,6 +340,12 @@ impl TestFixture {
             }
         }
 
+        // Activate keyboard for focus.
+        TestObject::<WlKeyboard>::from_request(
+            &ret.seat.as_ref().expect("Seat global missing").obj,
+            wl_seat::Request::GetKeyboard {},
+        );
+
         ret.into()
     }
 
@@ -566,6 +572,7 @@ impl TestFixture {
 
         self.testwl
             .configure_toplevel(id, 100, 100, vec![xdg_toplevel::State::Activated]);
+        self.testwl.focus_toplevel(id);
         self.run();
 
         {
@@ -1087,7 +1094,6 @@ fn window_group_properties() {
 #[test]
 fn copy_from_x11() {
     let (mut f, comp) = TestFixture::new_with_compositor();
-    TestObject::<WlKeyboard>::from_request(&comp.seat.obj, wl_seat::Request::GetKeyboard {});
     let win = unsafe { Window::new(1) };
     let (_surface, _id) = f.create_toplevel(&comp, win);
 
@@ -1303,7 +1309,8 @@ fn output_offset() {
         );
     }
     f.testwl
-        .configure_toplevel(t_id, 100, 100, vec![xdg_toplevel::State::Activated]);
+        .configure_toplevel(t_id, 100, 100, vec![]);
+    f.testwl.focus_toplevel(t_id);
     f.run();
 
     {

tests/integration.rs

@@ -179,6 +179,7 @@ impl Fixture {
 
         self.testwl
             .configure_toplevel(surface, 100, 100, vec![xdg_toplevel::State::Activated]);
+        self.testwl.focus_toplevel(surface);
         self.wait_and_dispatch();
         let geometry = connection
             .wait_for_reply(connection.send_request(&x::GetGeometry {

testwl/src/lib.rs

@@ -161,6 +161,11 @@ struct Output {
     xdg: Option<ZxdgOutputV1>,
 }
 
+struct KeyboardState {
+    keyboard: WlKeyboard,
+    current_focus: Option<SurfaceId>,
+}
+
 struct State {
     surfaces: HashMap<SurfaceId, SurfaceData>,
     outputs: HashMap<WlOutput, Output>,
@@ -171,7 +176,7 @@ struct State {
     last_output: Option<WlOutput>,
     callbacks: Vec<WlCallback>,
     pointer: Option<WlPointer>,
-    keyboard: Option<WlKeyboard>,
+    keyboard: Option<KeyboardState>,
     configure_serial: u32,
     selection: Option<WlDataSource>,
     data_device_man: Option<WlDataDeviceManager>,
@@ -209,15 +214,6 @@ impl State {
         states: Vec<xdg_toplevel::State>,
     ) {
         let last_serial = self.configure_serial;
-        if states.contains(&xdg_toplevel::State::Activated) {
-            if let Some(kb) = &self.keyboard {
-                kb.enter(
-                    last_serial,
-                    &self.surfaces[&surface_id].surface,
-                    Vec::default(),
-                );
-            }
-        }
         let toplevel = self.get_toplevel(surface_id);
         toplevel.states = states.clone();
         let states: Vec<u8> = states
@@ -229,6 +225,38 @@ impl State {
         self.configure_serial += 1;
     }
 
+    #[track_caller]
+    fn focus_toplevel(&mut self, surface_id: SurfaceId) {
+        let KeyboardState {
+            keyboard,
+            current_focus,
+        } = self.keyboard.as_mut().expect("Keyboard should be created");
+
+        if let Some(id) = current_focus {
+            keyboard.leave(self.configure_serial, &self.surfaces[id].surface);
+        }
+
+        keyboard.enter(
+            self.configure_serial,
+            &self.surfaces[&surface_id].surface,
+            Vec::default(),
+        );
+
+        *current_focus = Some(surface_id);
+    }
+
+    #[track_caller]
+    fn unfocus_toplevel(&mut self) {
+        let KeyboardState {
+            current_focus,
+            keyboard,
+        } = self.keyboard.as_mut().expect("Keyboard should be created");
+
+        if let Some(id) = current_focus.take() {
+            keyboard.leave(self.configure_serial, &self.surfaces[&id].surface);
+        }
+    }
+
     #[track_caller]
     pub fn configure_popup(&mut self, surface_id: SurfaceId) {
         let surface = self.surfaces.get_mut(&surface_id).unwrap();
@@ -433,6 +461,18 @@ impl Server {
         self.display.flush_clients().unwrap();
     }
 
+    #[track_caller]
+    pub fn focus_toplevel(&mut self, surface_id: SurfaceId) {
+        self.state.focus_toplevel(surface_id);
+        self.display.flush_clients().unwrap();
+    }
+
+    #[track_caller]
+    pub fn unfocus_toplevel(&mut self) {
+        self.state.unfocus_toplevel();
+        self.display.flush_clients().unwrap();
+    }
+
     #[track_caller]
     pub fn configure_popup(&mut self, surface_id: SurfaceId) {
         self.state.configure_popup(surface_id);
@@ -825,7 +865,10 @@ impl Dispatch<WlSeat, ()> for State {
                 state.pointer = Some(data_init.init(id, ()));
             }
             wl_seat::Request::GetKeyboard { id } => {
-                state.keyboard = Some(data_init.init(id, ()));
+                state.keyboard = Some(KeyboardState {
+                    keyboard: data_init.init(id, ()),
+                    current_focus: None,
+                });
             }
             wl_seat::Request::Release => {}
             other => todo!("unhandled request {other:?}"),
@@ -1317,9 +1360,16 @@ impl Dispatch<WlSurface, ()> for State {
             }
             Commit => {}
             Destroy => {
-                state
+                let id = SurfaceId(resource.id().protocol_id());
-                    .surfaces
+                if let Some(kb) = state
-                    .remove(&SurfaceId(resource.id().protocol_id()));
+                    .keyboard
+                    .as_mut()
+                    .filter(|kb| kb.current_focus == Some(id))
+                {
+                    kb.keyboard.leave(state.configure_serial, resource);
+                    kb.current_focus.take();
+                }
+                state.surfaces.remove(&id);
             }
             SetInputRegion { .. } => {}
             SetBufferScale { .. } => {}