Merge pull request #467 from cmeissl/feature/popup_grab

Add support for popup grabs to desktop abstraction

CHANGELOG.md

@@ -13,6 +13,7 @@
 - `PointerButtonEvent::button` now returns an `Option<MouseButton>`.
 - `MouseButton` is now non-exhaustive.
 - Remove `Other` and add `Forward` and `Back` variants to `MouseButton`. Use the new `PointerButtonEvent::button_code` in place of `Other`.
+- `GrabStartData` has been renamed to `PointerGrabStartData`
 
 #### Backends
 
@@ -49,6 +50,7 @@
 - Support for `xdg_wm_base` protocol version 3
 - Added the option to initialize the dmabuf global with a client filter
 - `wayland::output::Output` now has user data attached to it and more functions to query its properties
+- Added a `KeyboardGrab` similar to the existing `PointerGrab`
 
 #### Backends
 

anvil/src/shell.rs

@@ -19,7 +19,7 @@ use smithay::{
             compositor_init, is_sync_subsurface, with_states, with_surface_tree_upward, BufferAssignment,
             SurfaceAttributes, TraversalAction,
         },
-        seat::{AxisFrame, GrabStartData, PointerGrab, PointerInnerHandle, Seat},
+        seat::{AxisFrame, PointerGrab, PointerGrabStartData, PointerInnerHandle, Seat},
         shell::{
             legacy::{wl_shell_init, ShellRequest, ShellState as WlShellState, ShellSurfaceKind},
             wlr_layer::{LayerShellRequest, LayerSurfaceAttributes},
@@ -39,7 +39,7 @@ use crate::{
 };
 
 struct MoveSurfaceGrab {
-    start_data: GrabStartData,
+    start_data: PointerGrabStartData,
     window_map: Rc<RefCell<WindowMap>>,
     toplevel: SurfaceKind,
     initial_window_location: Point<i32, Logical>,
@@ -82,7 +82,7 @@ impl PointerGrab for MoveSurfaceGrab {
         handle.axis(details)
     }
 
-    fn start_data(&self) -> &GrabStartData {
+    fn start_data(&self) -> &PointerGrabStartData {
         &self.start_data
     }
 }
@@ -130,7 +130,7 @@ impl From<ResizeEdge> for xdg_toplevel::ResizeEdge {
 }
 
 struct ResizeSurfaceGrab {
-    start_data: GrabStartData,
+    start_data: PointerGrabStartData,
     toplevel: SurfaceKind,
     edges: ResizeEdge,
     initial_window_size: Size<i32, Logical>,
@@ -280,7 +280,7 @@ impl PointerGrab for ResizeSurfaceGrab {
         handle.axis(details)
     }
 
-    fn start_data(&self) -> &GrabStartData {
+    fn start_data(&self) -> &PointerGrabStartData {
         &self.start_data
     }
 }

src/desktop/layer.rs

@@ -21,6 +21,8 @@ use std::{
     sync::{Arc, Mutex, Weak},
 };
 
+use super::WindowSurfaceType;
+
 crate::utils::ids::id_gen!(next_layer_id, LAYER_ID, LAYER_IDS);
 
 /// Map of [`LayerSurface`]s on an [`Output`]
@@ -489,6 +491,7 @@ impl LayerSurface {
     pub fn surface_under<P: Into<Point<f64, Logical>>>(
         &self,
         point: P,
+        surface_type: WindowSurfaceType,
     ) -> Option<(WlSurface, Point<i32, Logical>)> {
         let point = point.into();
         if let Some(surface) = self.get_surface() {
@@ -499,13 +502,13 @@ impl LayerSurface {
             {
                 if let Some(result) = popup
                     .get_surface()
-                    .and_then(|surface| under_from_surface_tree(surface, point, location))
+                    .and_then(|surface| under_from_surface_tree(surface, point, location, surface_type))
                 {
                     return Some(result);
                 }
             }
 
-            under_from_surface_tree(surface, point, (0, 0))
+            under_from_surface_tree(surface, point, (0, 0), surface_type)
         } else {
             None
         }

src/desktop/popup/grab.rs

@@ -0,0 +1,501 @@
+use std::sync::{Arc, Mutex};
+
+use wayland_server::protocol::{wl_keyboard::KeyState, wl_pointer::ButtonState, wl_surface::WlSurface};
+
+use crate::{
+    utils::{DeadResource, Logical, Point},
+    wayland::{
+        compositor::get_role,
+        seat::{
+            AxisFrame, KeyboardGrab, KeyboardGrabStartData, KeyboardHandle, KeyboardInnerHandle, PointerGrab,
+            PointerGrabStartData, PointerInnerHandle,
+        },
+        shell::xdg::XDG_POPUP_ROLE,
+        Serial,
+    },
+};
+
+use thiserror::Error;
+
+use super::{PopupKind, PopupManager};
+
+/// Defines the possible errors that
+/// can be returned from [`PopupManager::grab_popup`]
+#[derive(Debug, Error)]
+pub enum PopupGrabError {
+    /// This resource has been destroyed and can no longer be used.
+    #[error(transparent)]
+    DeadResource(#[from] DeadResource),
+    /// The client tried to grab a popup after it's parent has been dismissed
+    #[error("the parent of the popup has been already dismissed")]
+    ParentDismissed,
+    /// The client tried to grab a popup after it has been mapped
+    #[error("tried to grab after being mapped")]
+    InvalidGrab,
+    /// The client tried to grab a popup which is not the topmost
+    #[error("popup was not created on the topmost popup")]
+    NotTheTopmostPopup,
+}
+
+/// Defines the possibly strategies
+/// for the [`PopupGrab::ungrab`] operation
+#[derive(Debug, Copy, Clone)]
+pub enum PopupUngrabStrategy {
+    /// Only ungrab the topmost popup
+    Topmost,
+    /// Ungrab all active popups
+    All,
+}
+
+#[derive(Debug, Default)]
+struct PopupGrabInternal {
+    serial: Option<Serial>,
+    active_grabs: Vec<(WlSurface, PopupKind)>,
+    dismissed_grabs: Vec<(WlSurface, PopupKind)>,
+}
+
+impl PopupGrabInternal {
+    fn alive(&self) -> bool {
+        !self.active_grabs.is_empty() || !self.dismissed_grabs.is_empty()
+    }
+
+    fn current_grab(&self) -> Option<&WlSurface> {
+        self.active_grabs
+            .iter()
+            .rev()
+            .find(|(_, p)| p.alive())
+            .map(|(s, _)| s)
+    }
+
+    fn is_dismissed(&self, surface: &WlSurface) -> bool {
+        self.dismissed_grabs.iter().any(|(s, _)| s == surface)
+    }
+
+    fn append_grab(&mut self, popup: &PopupKind) -> Result<(), DeadResource> {
+        let surface = popup.get_surface().ok_or(DeadResource)?;
+        self.active_grabs.push((surface.clone(), popup.clone()));
+        Ok(())
+    }
+
+    fn cleanup(&mut self) {
+        let mut i = 0;
+        while i < self.active_grabs.len() {
+            if !self.active_grabs[i].1.alive() {
+                let grab = self.active_grabs.remove(i);
+                self.dismissed_grabs.push(grab);
+            } else {
+                i += 1;
+            }
+        }
+
+        self.dismissed_grabs.retain(|(s, _)| s.as_ref().is_alive());
+    }
+}
+
+#[derive(Debug, Clone, Default)]
+pub(super) struct PopupGrabInner {
+    internal: Arc<Mutex<PopupGrabInternal>>,
+}
+
+impl PopupGrabInner {
+    pub(super) fn alive(&self) -> bool {
+        let guard = self.internal.lock().unwrap();
+        guard.alive()
+    }
+
+    fn current_grab(&self) -> Option<WlSurface> {
+        let guard = self.internal.lock().unwrap();
+        guard
+            .active_grabs
+            .iter()
+            .rev()
+            .find(|(_, p)| p.alive())
+            .map(|(s, _)| s)
+            .cloned()
+    }
+
+    pub(super) fn cleanup(&self) {
+        let mut guard = self.internal.lock().unwrap();
+        guard.cleanup();
+    }
+
+    pub(super) fn grab(&self, popup: &PopupKind, serial: Serial) -> Result<Option<Serial>, PopupGrabError> {
+        let parent = popup.parent().ok_or(DeadResource)?;
+        let parent_role = get_role(&parent);
+
+        self.cleanup();
+
+        let mut guard = self.internal.lock().unwrap();
+
+        match guard.current_grab() {
+            Some(grab) => {
+                if grab != &parent {
+                    // If the parent is a grabbing popup which has already been dismissed, this popup will be immediately dismissed.
+                    if guard.is_dismissed(&parent) {
+                        return Err(PopupGrabError::ParentDismissed);
+                    }
+
+                    // If the parent is a popup that did not take an explicit grab, an error will be raised.
+                    return Err(PopupGrabError::NotTheTopmostPopup);
+                }
+            }
+            None => {
+                if parent_role == Some(XDG_POPUP_ROLE) {
+                    return Err(PopupGrabError::NotTheTopmostPopup);
+                }
+            }
+        }
+
+        guard.append_grab(popup)?;
+
+        Ok(guard.serial.replace(serial))
+    }
+
+    fn ungrab(&self, root: &WlSurface, strategy: PopupUngrabStrategy) -> Option<WlSurface> {
+        let mut guard = self.internal.lock().unwrap();
+        let dismissed = match strategy {
+            PopupUngrabStrategy::Topmost => {
+                if let Some(grab) = guard.active_grabs.pop() {
+                    let dismissed = PopupManager::dismiss_popup(root, &grab.1);
+
+                    if dismissed.is_ok() {
+                        guard.dismissed_grabs.push(grab);
+                    }
+
+                    dismissed
+                } else {
+                    Ok(())
+                }
+            }
+            PopupUngrabStrategy::All => {
+                let grabs = guard.active_grabs.drain(..).collect::<Vec<_>>();
+
+                if let Some(grab) = grabs.first() {
+                    let dismissed = PopupManager::dismiss_popup(root, &grab.1);
+
+                    if dismissed.is_ok() {
+                        guard.dismissed_grabs.push(grab.clone());
+                        guard.dismissed_grabs.extend(grabs);
+                    }
+
+                    dismissed
+                } else {
+                    Ok(())
+                }
+            }
+        };
+
+        if dismissed.is_err() {
+            // If dismiss_popup returns Err(DeadResource) there is not much what
+            // can do about it here, we just remove all our grabs as they are dead now
+            // anyway. The pointer/keyboard grab will be unset automatically so we
+            // should be fine.
+            guard.active_grabs.drain(..);
+        }
+
+        guard.current_grab().cloned()
+    }
+}
+
+/// Represents the explicit grab a client requested for a popup
+///
+/// An explicit grab can be used by a client to redirect all keyboard
+/// input to a single popup. The focus of the keyboard will stay on
+/// the popup for as long as the grab is valid, that is as long as the
+/// compositor did not call [`ungrab`](PopupGrab::ungrab) or the client
+/// did not destroy the popup. A grab can be nested by requesting a grab
+/// on a popup who's parent is the currently grabbed popup. The grab will
+/// be returned to the parent after the popup has been dismissed.
+///
+/// This module also provides default implementations for [`KeyboardGrab`] and
+/// [`PointerGrab`] that implement the behavior described in the [`xdg-shell`](https://wayland.app/protocols/xdg-shell#xdg_popup:request:grab)
+/// specification. See [`PopupKeyboardGrab`] and [`PopupPointerGrab`] for more
+/// information on the default implementations.
+///
+/// In case the implemented behavior is not suited for your use-case the grab can be
+/// either decorated or a custom [`KeyboardGrab`]/[`PointerGrab`] can use the methods
+/// on the [`PopupGrab`] to implement a custom behavior.
+///
+/// One example would be to use a timer to automatically dismiss the popup after some
+/// timeout.
+///
+/// The grab is obtained by calling [`PopupManager::grap_popup`](super::PopupManager::grab_popup).
+#[derive(Debug, Clone)]
+pub struct PopupGrab {
+    root: WlSurface,
+    serial: Serial,
+    previous_serial: Option<Serial>,
+    toplevel_grab: PopupGrabInner,
+    keyboard_handle: Option<KeyboardHandle>,
+    keyboard_grab_start_data: KeyboardGrabStartData,
+    pointer_grab_start_data: PointerGrabStartData,
+}
+
+impl PopupGrab {
+    pub(super) fn new(
+        toplevel_popups: PopupGrabInner,
+        root: WlSurface,
+        serial: Serial,
+        previous_serial: Option<Serial>,
+        keyboard_handle: Option<KeyboardHandle>,
+    ) -> Self {
+        PopupGrab {
+            root: root.clone(),
+            serial,
+            previous_serial,
+            toplevel_grab: toplevel_popups,
+            keyboard_handle,
+            keyboard_grab_start_data: KeyboardGrabStartData {
+                // We set the focus to root as this will make
+                // sure the grab will stay alive until the
+                // toplevel is destroyed or the grab is unset
+                focus: Some(root.clone()),
+            },
+            pointer_grab_start_data: PointerGrabStartData {
+                button: 0,
+                // We set the focus to root as this will make
+                // sure the grab will stay alive until the
+                // toplevel is destroyed or the grab is unset
+                focus: Some((root, (0, 0).into())),
+                location: (0f64, 0f64).into(),
+            },
+        }
+    }
+
+    /// Returns the serial that was used to grab the popup
+    pub fn serial(&self) -> Serial {
+        self.serial
+    }
+
+    /// Returns the previous serial that was used to grab
+    /// the parent popup in case of nested grabs
+    pub fn previous_serial(&self) -> Option<Serial> {
+        self.previous_serial
+    }
+
+    /// Check if this grab has ended
+    ///
+    /// A grab has ended if either all popups
+    /// associated with the grab have been dismissed
+    /// by the server with [`PopupGrab::ungrab`] or by the client
+    /// by destroying the popup.
+    ///
+    /// This will also return [`false`] if the root
+    /// of the grab has been destroyed.
+    pub fn has_ended(&self) -> bool {
+        !self.root.as_ref().is_alive() || !self.toplevel_grab.alive()
+    }
+
+    /// Returns the current grabbed [`WlSurface`].
+    ///
+    /// If the grab has ended this will return the root surface
+    /// so that the client expected focus can be restored
+    pub fn current_grab(&self) -> Option<WlSurface> {
+        self.toplevel_grab
+            .current_grab()
+            .or_else(|| Some(self.root.clone()))
+    }
+
+    /// Ungrab and dismiss a popup
+    ///
+    /// This will dismiss either the topmost or all popups
+    /// according to the specified [`PopupUngrabStrategy`]
+    ///
+    /// Returns the new topmost popup in case of nested popups
+    /// or if the grab has ended the root surface
+    pub fn ungrab(&mut self, strategy: PopupUngrabStrategy) -> Option<WlSurface> {
+        self.toplevel_grab
+            .ungrab(&self.root, strategy)
+            .or_else(|| Some(self.root.clone()))
+    }
+
+    /// Convenience method for getting a [`KeyboardGrabStartData`] for this grab.
+    ///
+    /// The focus of the [`KeyboardGrabStartData`] will always be the root
+    /// of the popup grab, e.g. the surface of the toplevel, to make sure
+    /// the grab is not automatically unset.
+    pub fn keyboard_grab_start_data(&self) -> &KeyboardGrabStartData {
+        &self.keyboard_grab_start_data
+    }
+
+    /// Convenience method for getting a [`PointerGrabStartData`] for this grab.
+    ///
+    /// The focus of the [`PointerGrabStartData`] will always be the root
+    /// of the popup grab, e.g. the surface of the toplevel, to make sure
+    /// the grab is not automatically unset.
+    pub fn pointer_grab_start_data(&self) -> &PointerGrabStartData {
+        &self.pointer_grab_start_data
+    }
+
+    fn unset_keyboard_grab(&self, serial: Serial) {
+        if let Some(keyboard) = self.keyboard_handle.as_ref() {
+            if keyboard.is_grabbed()
+                && (keyboard.has_grab(self.serial)
+                    || keyboard.has_grab(self.previous_serial.unwrap_or(self.serial)))
+            {
+                keyboard.unset_grab();
+                keyboard.set_focus(Some(&self.root), serial);
+            }
+        }
+    }
+}
+
+/// Default implementation of a [`KeyboardGrab`] for [`PopupGrab`]
+///
+/// The [`PopupKeyboardGrab`] will keep the focus of the keyboard
+/// on the topmost popup until the grab has ended. If the
+/// grab has ended it will restore the focus on the root of the grab
+/// and unset the [`KeyboardGrab`]
+#[derive(Debug)]
+pub struct PopupKeyboardGrab {
+    popup_grab: PopupGrab,
+}
+
+impl PopupKeyboardGrab {
+    /// Create a [`PopupKeyboardGrab`] for the provided [`PopupGrab`]
+    pub fn new(popup_grab: &PopupGrab) -> Self {
+        PopupKeyboardGrab {
+            popup_grab: popup_grab.clone(),
+        }
+    }
+}
+
+impl KeyboardGrab for PopupKeyboardGrab {
+    fn input(
+        &mut self,
+        handle: &mut KeyboardInnerHandle<'_>,
+        keycode: u32,
+        key_state: KeyState,
+        modifiers: Option<(u32, u32, u32, u32)>,
+        serial: Serial,
+        time: u32,
+    ) {
+        // Check if the grab changed and update the focus
+        // If the grab has ended this will return the root
+        // surface to restore the client expected focus.
+        if let Some(surface) = self.popup_grab.current_grab() {
+            handle.set_focus(Some(&surface), serial);
+        }
+
+        if self.popup_grab.has_ended() {
+            handle.unset_grab(serial, false);
+        }
+
+        handle.input(keycode, key_state, modifiers, serial, time)
+    }
+
+    fn set_focus(&mut self, handle: &mut KeyboardInnerHandle<'_>, focus: Option<&WlSurface>, serial: Serial) {
+        // Ignore focus changes unless the grab has ended
+        if self.popup_grab.has_ended() {
+            handle.set_focus(focus, serial);
+            handle.unset_grab(serial, false);
+            return;
+        }
+
+        // Allow to set the focus to the current grab, this can
+        // happen if the user initially sets the focus to
+        // popup instead of relying on the grab behavior
+        if self.popup_grab.current_grab().as_ref() == focus {
+            handle.set_focus(focus, serial);
+        }
+    }
+
+    fn start_data(&self) -> &KeyboardGrabStartData {
+        self.popup_grab.keyboard_grab_start_data()
+    }
+}
+
+/// Default implementation of a [`PointerGrab`] for [`PopupGrab`]
+///
+/// The [`PopupPointerGrab`] will make sure that the pointer focus
+/// stays on the same client as the grabbed popup (similar to an
+/// "owner-events" grab in X11 parlance). If an input event happens
+/// outside of the grabbed [`WlSurface`] the popup will be dismissed
+/// and the grab ends. In case of a nested grab all parent grabs will
+/// also be dismissed.
+///
+/// If the grab has ended the pointer focus is restored and the
+/// [`PointerGrab`] is unset. Additional it will unset an active
+/// [`KeyboardGrab`] that matches the [`Serial`] of this grab and
+/// restore the keyboard focus like described in [`PopupKeyboardGrab`]
+#[derive(Debug)]
+pub struct PopupPointerGrab {
+    popup_grab: PopupGrab,
+}
+
+impl PopupPointerGrab {
+    /// Create a [`PopupPointerGrab`] for the provided [`PopupGrab`]
+    pub fn new(popup_grab: &PopupGrab) -> Self {
+        PopupPointerGrab {
+            popup_grab: popup_grab.clone(),
+        }
+    }
+}
+
+impl PointerGrab for PopupPointerGrab {
+    fn motion(
+        &mut self,
+        handle: &mut PointerInnerHandle<'_>,
+        location: Point<f64, Logical>,
+        focus: Option<(WlSurface, Point<i32, Logical>)>,
+        serial: Serial,
+        time: u32,
+    ) {
+        if self.popup_grab.has_ended() {
+            handle.unset_grab(serial, time);
+            self.popup_grab.unset_keyboard_grab(serial);
+            return;
+        }
+
+        // Check that the focus is of the same client as the grab
+        // If yes allow it, if not unset the focus.
+        let same_client = focus.as_ref().map(|(surface, _)| surface.as_ref().client())
+            == self
+                .popup_grab
+                .current_grab()
+                .map(|surface| surface.as_ref().client());
+
+        if same_client {
+            handle.motion(location, focus, serial, time);
+        } else {
+            handle.motion(location, None, serial, time);
+        }
+    }
+
+    fn button(
+        &mut self,
+        handle: &mut PointerInnerHandle<'_>,
+        button: u32,
+        state: ButtonState,
+        serial: Serial,
+        time: u32,
+    ) {
+        if self.popup_grab.has_ended() {
+            handle.unset_grab(serial, time);
+            handle.button(button, state, serial, time);
+            self.popup_grab.unset_keyboard_grab(serial);
+            return;
+        }
+
+        // Check if the the focused surface is still on the current grabbed surface,
+        // if not the popup will be dismissed
+        if state == ButtonState::Pressed
+            && handle.current_focus().map(|(surface, _)| surface) != self.popup_grab.current_grab().as_ref()
+        {
+            let _ = self.popup_grab.ungrab(PopupUngrabStrategy::All);
+            handle.unset_grab(serial, time);
+            handle.button(button, state, serial, time);
+            self.popup_grab.unset_keyboard_grab(serial);
+        }
+
+        handle.button(button, state, serial, time);
+    }
+
+    fn axis(&mut self, handle: &mut PointerInnerHandle<'_>, details: AxisFrame) {
+        handle.axis(details);
+    }
+
+    fn start_data(&self) -> &PointerGrabStartData {
+        self.popup_grab.pointer_grab_start_data()
+    }
+}

src/desktop/popup/manager.rs

@@ -1,18 +1,23 @@
 use crate::{
-    utils::{DeadResource, Logical, Point, Rectangle},
+    utils::{DeadResource, Logical, Point},
     wayland::{
         compositor::{get_role, with_states},
-        shell::xdg::{PopupSurface, SurfaceCachedState, XdgPopupSurfaceRoleAttributes, XDG_POPUP_ROLE},
+        seat::Seat,
+        shell::xdg::{XdgPopupSurfaceRoleAttributes, XDG_POPUP_ROLE},
+        Serial,
     },
 };
 use std::sync::{Arc, Mutex};
 use wayland_server::protocol::wl_surface::WlSurface;
 
+use super::{PopupGrab, PopupGrabError, PopupGrabInner, PopupKind};
+
 /// Helper to track popups.
 #[derive(Debug)]
 pub struct PopupManager {
     unmapped_popups: Vec<PopupKind>,
     popup_trees: Vec<PopupTree>,
+    popup_grabs: Vec<PopupGrabInner>,
     logger: ::slog::Logger,
 }
 
@@ -22,6 +27,7 @@ impl PopupManager {
         PopupManager {
             unmapped_popups: Vec::new(),
             popup_trees: Vec::new(),
+            popup_grabs: Vec::new(),
             logger: crate::slog_or_fallback(logger),
         }
     }
@@ -54,24 +60,89 @@ impl PopupManager {
         }
     }
 
-    fn add_popup(&mut self, popup: PopupKind) -> Result<(), DeadResource> {
-        let mut parent = popup.parent().unwrap();
-        while get_role(&parent) == Some(XDG_POPUP_ROLE) {
-            parent = with_states(&parent, |states| {
-                states
-                    .data_map
-                    .get::<Mutex<XdgPopupSurfaceRoleAttributes>>()
-                    .unwrap()
-                    .lock()
-                    .unwrap()
-                    .parent
-                    .as_ref()
-                    .cloned()
-                    .unwrap()
-            })?;
+    /// Take an explicit grab for the provided [`PopupKind`]
+    ///
+    /// Returns a [`PopupGrab`] on success or an [`PopupGrabError`]
+    /// if the grab has been denied.
+    pub fn grab_popup(
+        &mut self,
+        popup: PopupKind,
+        seat: &Seat,
+        serial: Serial,
+    ) -> Result<PopupGrab, PopupGrabError> {
+        let surface = popup.get_surface().ok_or(DeadResource)?;
+        let root = find_popup_root_surface(&popup)?;
+
+        match popup {
+            PopupKind::Xdg(ref xdg) => {
+                let surface = xdg.get_surface().ok_or(DeadResource)?;
+                let committed = with_states(surface, |states| {
+                    states
+                        .data_map
+                        .get::<Mutex<XdgPopupSurfaceRoleAttributes>>()
+                        .unwrap()
+                        .lock()
+                        .unwrap()
+                        .committed
+                })?;
+
+                if committed {
+                    surface.as_ref().post_error(
+                        wayland_protocols::xdg_shell::server::xdg_popup::Error::InvalidGrab as u32,
+                        "xdg_popup already is mapped".to_string(),
+                    );
+                    return Err(PopupGrabError::InvalidGrab);
+                }
+            }
         }
 
-        with_states(&parent, |states| {
+        // The primary store for the grab is the seat, additional we store it
+        // in the popupmanager for active cleanup
+        seat.user_data().insert_if_missing(PopupGrabInner::default);
+        let toplevel_popups = seat.user_data().get::<PopupGrabInner>().unwrap().clone();
+
+        // It the popup grab is not alive it is likely
+        // that it either is new and have never been
+        // added to the popupmanager or that it has been
+        // cleaned up.
+        if !toplevel_popups.alive() {
+            self.popup_grabs.push(toplevel_popups.clone());
+        }
+
+        let previous_serial = match toplevel_popups.grab(&popup, serial) {
+            Ok(serial) => serial,
+            Err(err) => {
+                match err {
+                    PopupGrabError::ParentDismissed => {
+                        let _ = PopupManager::dismiss_popup(&root, &popup);
+                    }
+                    PopupGrabError::NotTheTopmostPopup => {
+                        surface.as_ref().post_error(
+                            wayland_protocols::xdg_shell::server::xdg_wm_base::Error::NotTheTopmostPopup
+                                as u32,
+                            "xdg_popup was not created on the topmost popup".to_string(),
+                        );
+                    }
+                    _ => {}
+                }
+
+                return Err(err);
+            }
+        };
+
+        Ok(PopupGrab::new(
+            toplevel_popups,
+            root,
+            serial,
+            previous_serial,
+            seat.get_keyboard(),
+        ))
+    }
+
+    fn add_popup(&mut self, popup: PopupKind) -> Result<(), DeadResource> {
+        let root = find_popup_root_surface(&popup)?;
+
+        with_states(&root, |states| {
             let tree = PopupTree::default();
             if states.data_map.insert_if_missing(|| tree.clone()) {
                 self.popup_trees.push(tree);
@@ -81,7 +152,7 @@ impl PopupManager {
                 // if it previously had no popups, we likely removed it from our list already
                 self.popup_trees.push(tree.clone());
             }
-            slog::trace!(self.logger, "Adding popup {:?} to parent {:?}", popup, parent);
+            slog::trace!(self.logger, "Adding popup {:?} to root {:?}", popup, root);
             tree.insert(popup);
         })
     }
@@ -116,16 +187,47 @@ impl PopupManager {
         })
     }
 
+    pub(crate) fn dismiss_popup(surface: &WlSurface, popup: &PopupKind) -> Result<(), DeadResource> {
+        with_states(surface, |states| {
+            let tree = states.data_map.get::<PopupTree>();
+
+            if let Some(tree) = tree {
+                tree.dismiss_popup(popup);
+            }
+        })
+    }
+
     /// Needs to be called periodically (but not necessarily frequently)
     /// to cleanup internal resources.
     pub fn cleanup(&mut self) {
         // retain_mut is sadly still unstable
+        self.popup_grabs.iter_mut().for_each(|grabs| grabs.cleanup());
+        self.popup_grabs.retain(|grabs| grabs.alive());
         self.popup_trees.iter_mut().for_each(|tree| tree.cleanup());
         self.popup_trees.retain(|tree| tree.alive());
         self.unmapped_popups.retain(|surf| surf.alive());
     }
 }
 
+fn find_popup_root_surface(popup: &PopupKind) -> Result<WlSurface, DeadResource> {
+    let mut parent = popup.parent().ok_or(DeadResource)?;
+    while get_role(&parent) == Some(XDG_POPUP_ROLE) {
+        parent = with_states(&parent, |states| {
+            states
+                .data_map
+                .get::<Mutex<XdgPopupSurfaceRoleAttributes>>()
+                .unwrap()
+                .lock()
+                .unwrap()
+                .parent
+                .as_ref()
+                .cloned()
+                .unwrap()
+        })?;
+    }
+    Ok(parent)
+}
+
 #[derive(Debug, Default, Clone)]
 struct PopupTree(Arc<Mutex<Vec<PopupNode>>>);
 
@@ -157,6 +259,22 @@ impl PopupTree {
         children.push(PopupNode::new(popup));
     }
 
+    fn dismiss_popup(&self, popup: &PopupKind) {
+        let mut children = self.0.lock().unwrap();
+
+        let mut i = 0;
+        while i < children.len() {
+            let child = &mut children[i];
+
+            if child.dismiss_popup(popup) {
+                let _ = children.remove(i);
+                break;
+            } else {
+                i += 1;
+            }
+        }
+    }
+
     fn cleanup(&mut self) {
         let mut children = self.0.lock().unwrap();
         for child in children.iter_mut() {
@@ -209,80 +327,48 @@ impl PopupNode {
         }
     }
 
+    fn send_done(&self) {
+        for child in self.children.iter().rev() {
+            child.send_done();
+        }
+
+        self.surface.send_done();
+    }
+
+    fn dismiss_popup(&mut self, popup: &PopupKind) -> bool {
+        if self.surface.get_surface() == popup.get_surface() {
+            self.send_done();
+            return true;
+        }
+
+        let mut i = 0;
+        while i < self.children.len() {
+            let child = &mut self.children[i];
+
+            if child.dismiss_popup(popup) {
+                let _ = self.children.remove(i);
+                return false;
+            } else {
+                i += 1;
+            }
+        }
+
+        false
+    }
+
     fn cleanup(&mut self) {
         for child in &mut self.children {
             child.cleanup();
         }
+
+        if !self.surface.alive() && !self.children.is_empty() {
+            // TODO: The client destroyed a popup before
+            // destroying all children, this is a protocol
+            // error. As the surface is no longer alive we
+            // can not retrieve the client here to send
+            // the error.
+        }
+
         self.children.retain(|n| n.surface.alive());
     }
 }
-
-/// Represents a popup surface
-#[derive(Debug, Clone)]
-pub enum PopupKind {
-    /// xdg-shell [`PopupSurface`]
-    Xdg(PopupSurface),
-}
-
-impl PopupKind {
-    fn alive(&self) -> bool {
-        match *self {
-            PopupKind::Xdg(ref t) => t.alive(),
-        }
-    }
-
-    /// Retrieves the underlying [`WlSurface`]
-    pub fn get_surface(&self) -> Option<&WlSurface> {
-        match *self {
-            PopupKind::Xdg(ref t) => t.get_surface(),
-        }
-    }
-
-    fn parent(&self) -> Option<WlSurface> {
-        match *self {
-            PopupKind::Xdg(ref t) => t.get_parent_surface(),
-        }
-    }
-
-    /// Returns the surface geometry as set by the client using `xdg_surface::set_window_geometry`
-    pub fn geometry(&self) -> Rectangle<i32, Logical> {
-        let wl_surface = match self.get_surface() {
-            Some(s) => s,
-            None => return Rectangle::from_loc_and_size((0, 0), (0, 0)),
-        };
-
-        with_states(wl_surface, |states| {
-            states
-                .cached_state
-                .current::<SurfaceCachedState>()
-                .geometry
-                .unwrap_or_default()
-        })
-        .unwrap()
-    }
-
-    fn location(&self) -> Point<i32, Logical> {
-        let wl_surface = match self.get_surface() {
-            Some(s) => s,
-            None => return (0, 0).into(),
-        };
-        with_states(wl_surface, |states| {
-            states
-                .data_map
-                .get::<Mutex<XdgPopupSurfaceRoleAttributes>>()
-                .unwrap()
-                .lock()
-                .unwrap()
-                .current
-                .geometry
-        })
-        .unwrap_or_default()
-        .loc
-    }
-}
-
-impl From<PopupSurface> for PopupKind {
-    fn from(p: PopupSurface) -> PopupKind {
-        PopupKind::Xdg(p)
-    }
-}

src/desktop/popup/mod.rs

@@ -0,0 +1,96 @@
+mod grab;
+mod manager;
+
+use std::sync::Mutex;
+
+pub use grab::*;
+pub use manager::*;
+use wayland_server::protocol::wl_surface::WlSurface;
+
+use crate::{
+    utils::{Logical, Point, Rectangle},
+    wayland::{
+        compositor::with_states,
+        shell::xdg::{PopupSurface, SurfaceCachedState, XdgPopupSurfaceRoleAttributes},
+    },
+};
+
+/// Represents a popup surface
+#[derive(Debug, Clone)]
+pub enum PopupKind {
+    /// xdg-shell [`PopupSurface`]
+    Xdg(PopupSurface),
+}
+
+impl PopupKind {
+    fn alive(&self) -> bool {
+        match *self {
+            PopupKind::Xdg(ref t) => t.alive(),
+        }
+    }
+
+    /// Retrieves the underlying [`WlSurface`]
+    pub fn get_surface(&self) -> Option<&WlSurface> {
+        match *self {
+            PopupKind::Xdg(ref t) => t.get_surface(),
+        }
+    }
+
+    fn parent(&self) -> Option<WlSurface> {
+        match *self {
+            PopupKind::Xdg(ref t) => t.get_parent_surface(),
+        }
+    }
+
+    /// Returns the surface geometry as set by the client using `xdg_surface::set_window_geometry`
+    pub fn geometry(&self) -> Rectangle<i32, Logical> {
+        let wl_surface = match self.get_surface() {
+            Some(s) => s,
+            None => return Rectangle::from_loc_and_size((0, 0), (0, 0)),
+        };
+
+        with_states(wl_surface, |states| {
+            states
+                .cached_state
+                .current::<SurfaceCachedState>()
+                .geometry
+                .unwrap_or_default()
+        })
+        .unwrap()
+    }
+
+    fn send_done(&self) {
+        if !self.alive() {
+            return;
+        }
+
+        match *self {
+            PopupKind::Xdg(ref t) => t.send_popup_done(),
+        }
+    }
+
+    fn location(&self) -> Point<i32, Logical> {
+        let wl_surface = match self.get_surface() {
+            Some(s) => s,
+            None => return (0, 0).into(),
+        };
+        with_states(wl_surface, |states| {
+            states
+                .data_map
+                .get::<Mutex<XdgPopupSurfaceRoleAttributes>>()
+                .unwrap()
+                .lock()
+                .unwrap()
+                .current
+                .geometry
+        })
+        .unwrap_or_default()
+        .loc
+    }
+}
+
+impl From<PopupSurface> for PopupKind {
+    fn from(p: PopupSurface) -> PopupKind {
+        PopupKind::Xdg(p)
+    }
+}

src/desktop/utils.rs

@@ -16,6 +16,8 @@ use wayland_server::protocol::wl_surface;
 
 use std::cell::RefCell;
 
+use super::WindowSurfaceType;
+
 impl SurfaceState {
     fn contains_point<P: Into<Point<f64, Logical>>>(&self, attrs: &SurfaceAttributes, point: P) -> bool {
         let point = point.into();
@@ -172,6 +174,7 @@ pub fn under_from_surface_tree<P>(
     surface: &wl_surface::WlSurface,
     point: Point<f64, Logical>,
     location: P,
+    surface_type: WindowSurfaceType,
 ) -> Option<(wl_surface::WlSurface, Point<i32, Logical>)>
 where
     P: Into<Point<i32, Logical>>,
@@ -189,17 +192,23 @@ where
                 location += current.location;
             }
 
-            let contains_the_point = data
-                .map(|data| {
-                    data.borrow()
-                        .contains_point(&*states.cached_state.current(), point - location.to_f64())
-                })
-                .unwrap_or(false);
-            if contains_the_point {
-                *found.borrow_mut() = Some((wl_surface.clone(), location));
+            if states.role == Some("subsurface") || surface_type.contains(WindowSurfaceType::TOPLEVEL) {
+                let contains_the_point = data
+                    .map(|data| {
+                        data.borrow()
+                            .contains_point(&*states.cached_state.current(), point - location.to_f64())
+                    })
+                    .unwrap_or(false);
+                if contains_the_point {
+                    *found.borrow_mut() = Some((wl_surface.clone(), location));
+                }
             }
 
-            TraversalAction::DoChildren(location)
+            if surface_type.contains(WindowSurfaceType::SUBSURFACE) {
+                TraversalAction::DoChildren(location)
+            } else {
+                TraversalAction::SkipChildren
+            }
         },
         |_, _, _| {},
         |_, _, _| {

src/desktop/window.rs

@@ -109,6 +109,21 @@ impl Hash for Window {
     }
 }
 
+bitflags::bitflags! {
+    /// Defines the surface types that can be
+    /// queried with [`Window::surface_under`]
+    pub struct WindowSurfaceType: u32 {
+        /// Include the toplevel surface
+        const TOPLEVEL = 1;
+        /// Include all subsurfaces
+        const SUBSURFACE = 2;
+        /// Include all popup surfaces
+        const POPUP = 4;
+        /// Query all surfaces
+        const ALL = Self::TOPLEVEL.bits | Self::SUBSURFACE.bits | Self::POPUP.bits;
+    }
+}
+
 impl Window {
     /// Construct a new [`Window`] from a given compatible toplevel surface
     pub fn new(toplevel: Kind) -> Window {
@@ -222,24 +237,27 @@ impl Window {
     pub fn surface_under<P: Into<Point<f64, Logical>>>(
         &self,
         point: P,
+        surface_type: WindowSurfaceType,
     ) -> Option<(wl_surface::WlSurface, Point<i32, Logical>)> {
         let point = point.into();
         if let Some(surface) = self.0.toplevel.get_surface() {
-            for (popup, location) in PopupManager::popups_for_surface(surface)
-                .ok()
-                .into_iter()
-                .flatten()
-            {
-                let offset = self.geometry().loc + location - popup.geometry().loc;
-                if let Some(result) = popup
-                    .get_surface()
-                    .and_then(|surface| under_from_surface_tree(surface, point, offset))
+            if surface_type.contains(WindowSurfaceType::POPUP) {
+                for (popup, location) in PopupManager::popups_for_surface(surface)
+                    .ok()
+                    .into_iter()
+                    .flatten()
                 {
-                    return Some(result);
+                    let offset = self.geometry().loc + location - popup.geometry().loc;
+                    if let Some(result) = popup
+                        .get_surface()
+                        .and_then(|surface| under_from_surface_tree(surface, point, offset, surface_type))
+                    {
+                        return Some(result);
+                    }
                 }
             }
 
-            under_from_surface_tree(surface, point, (0, 0))
+            under_from_surface_tree(surface, point, (0, 0), surface_type)
         } else {
             None
         }

src/wayland/data_device/dnd_grab.rs

@@ -8,7 +8,7 @@ use wayland_server::{
 use crate::{
     utils::{Logical, Point},
     wayland::{
-        seat::{AxisFrame, GrabStartData, PointerGrab, PointerInnerHandle, Seat},
+        seat::{AxisFrame, PointerGrab, PointerGrabStartData, PointerInnerHandle, Seat},
         Serial,
     },
 };
@@ -16,7 +16,7 @@ use crate::{
 use super::{with_source_metadata, DataDeviceData, SeatData};
 
 pub(crate) struct DnDGrab {
-    start_data: GrabStartData,
+    start_data: PointerGrabStartData,
     data_source: Option<wl_data_source::WlDataSource>,
     current_focus: Option<wl_surface::WlSurface>,
     pending_offers: Vec<wl_data_offer::WlDataOffer>,
@@ -29,7 +29,7 @@ pub(crate) struct DnDGrab {
 
 impl DnDGrab {
     pub(crate) fn new(
-        start_data: GrabStartData,
+        start_data: PointerGrabStartData,
         source: Option<wl_data_source::WlDataSource>,
         origin: wl_surface::WlSurface,
         seat: Seat,
@@ -222,7 +222,7 @@ impl PointerGrab for DnDGrab {
         handle.axis(details);
     }
 
-    fn start_data(&self) -> &GrabStartData {
+    fn start_data(&self) -> &PointerGrabStartData {
         &self.start_data
     }
 }

src/wayland/data_device/mod.rs

@@ -60,7 +60,7 @@ use slog::{debug, error, o};
 
 use crate::wayland::{
     compositor,
-    seat::{GrabStartData, Seat},
+    seat::{PointerGrabStartData, Seat},
     Serial,
 };
 
@@ -335,7 +335,7 @@ pub fn set_data_device_selection(seat: &Seat, mime_types: Vec<String>) {
 pub fn start_dnd<C>(
     seat: &Seat,
     serial: Serial,
-    start_data: GrabStartData,
+    start_data: PointerGrabStartData,
     metadata: SourceMetadata,
     callback: C,
 ) where

src/wayland/data_device/server_dnd_grab.rs

@@ -8,7 +8,7 @@ use wayland_server::{
 use crate::{
     utils::{Logical, Point},
     wayland::{
-        seat::{AxisFrame, GrabStartData, PointerGrab, PointerInnerHandle, Seat},
+        seat::{AxisFrame, PointerGrab, PointerGrabStartData, PointerInnerHandle, Seat},
         Serial,
     },
 };
@@ -42,7 +42,7 @@ pub enum ServerDndEvent {
 }
 
 pub(crate) struct ServerDnDGrab<C: 'static> {
-    start_data: GrabStartData,
+    start_data: PointerGrabStartData,
     metadata: super::SourceMetadata,
     current_focus: Option<wl_surface::WlSurface>,
     pending_offers: Vec<wl_data_offer::WlDataOffer>,
@@ -53,7 +53,7 @@ pub(crate) struct ServerDnDGrab<C: 'static> {
 
 impl<C: 'static> ServerDnDGrab<C> {
     pub(crate) fn new(
-        start_data: GrabStartData,
+        start_data: PointerGrabStartData,
         metadata: super::SourceMetadata,
         seat: Seat,
         callback: Rc<RefCell<C>>,
@@ -222,7 +222,7 @@ where
         handle.axis(details);
     }
 
-    fn start_data(&self) -> &GrabStartData {
+    fn start_data(&self) -> &PointerGrabStartData {
         &self.start_data
     }
 }

src/wayland/seat/keyboard.rs

@@ -86,9 +86,16 @@ pub struct XkbConfig<'a> {
     pub options: Option<String>,
 }
 
+enum GrabStatus {
+    None,
+    Active(Serial, Box<dyn KeyboardGrab>),
+    Borrowed,
+}
+
 struct KbdInternal {
     known_kbds: Vec<WlKeyboard>,
     focus: Option<WlSurface>,
+    pending_focus: Option<WlSurface>,
     pressed_keys: Vec<u32>,
     mods_state: ModifiersState,
     keymap: xkb::Keymap,
@@ -96,6 +103,7 @@ struct KbdInternal {
     repeat_rate: i32,
     repeat_delay: i32,
     focus_hook: Box<dyn FnMut(Option<&WlSurface>)>,
+    grab: GrabStatus,
 }
 
 // focus_hook does not implement debug, so we have to impl Debug manually
@@ -147,6 +155,7 @@ impl KbdInternal {
         Ok(KbdInternal {
             known_kbds: Vec::new(),
             focus: None,
+            pending_focus: None,
             pressed_keys: Vec::new(),
             mods_state: ModifiersState::default(),
             keymap,
@@ -154,6 +163,7 @@ impl KbdInternal {
             repeat_rate,
             repeat_delay,
             focus_hook,
+            grab: GrabStatus::None,
         })
     }
 
@@ -215,6 +225,35 @@ impl KbdInternal {
             }
         }
     }
+
+    fn with_grab<F>(&mut self, f: F, logger: ::slog::Logger)
+    where
+        F: FnOnce(KeyboardInnerHandle<'_>, &mut dyn KeyboardGrab),
+    {
+        let mut grab = ::std::mem::replace(&mut self.grab, GrabStatus::Borrowed);
+        match grab {
+            GrabStatus::Borrowed => panic!("Accessed a keyboard grab from within a keyboard grab access."),
+            GrabStatus::Active(_, ref mut handler) => {
+                // If this grab is associated with a surface that is no longer alive, discard it
+                if let Some(ref surface) = handler.start_data().focus {
+                    if !surface.as_ref().is_alive() {
+                        self.grab = GrabStatus::None;
+                        f(KeyboardInnerHandle { inner: self, logger }, &mut DefaultGrab);
+                        return;
+                    }
+                }
+                f(KeyboardInnerHandle { inner: self, logger }, &mut **handler);
+            }
+            GrabStatus::None => {
+                f(KeyboardInnerHandle { inner: self, logger }, &mut DefaultGrab);
+            }
+        }
+
+        if let GrabStatus::Borrowed = self.grab {
+            // the grab has not been ended nor replaced, put it back in place
+            self.grab = grab;
+        }
+    }
 }
 
 /// Errors that can be encountered when creating a keyboard handler
@@ -320,6 +359,47 @@ pub enum FilterResult<T> {
     Intercept(T),
 }
 
+/// Data about the event that started the grab.
+#[derive(Debug, Clone)]
+pub struct GrabStartData {
+    /// The focused surface, if any, at the start of the grab.
+    pub focus: Option<WlSurface>,
+}
+
+/// A trait to implement a keyboard grab
+///
+/// In some context, it is necessary to temporarily change the behavior of the keyboard. This is
+/// typically known as a keyboard grab. A example would be, during a popup grab the keyboard focus
+/// will not be changed and stay on the grabbed popup.
+///
+/// This trait is the interface to intercept regular keyboard events and change them as needed, its
+/// interface mimics the [`KeyboardHandle`] interface.
+///
+/// If your logic decides that the grab should end, both [`KeyboardInnerHandle`] and [`KeyboardHandle`] have
+/// a method to change it.
+///
+/// When your grab ends (either as you requested it or if it was forcefully cancelled by the server),
+/// the struct implementing this trait will be dropped. As such you should put clean-up logic in the destructor,
+/// rather than trying to guess when the grab will end.
+pub trait KeyboardGrab {
+    /// An input was reported
+    fn input(
+        &mut self,
+        handle: &mut KeyboardInnerHandle<'_>,
+        keycode: u32,
+        key_state: WlKeyState,
+        modifiers: Option<(u32, u32, u32, u32)>,
+        serial: Serial,
+        time: u32,
+    );
+
+    /// A focus change was requested
+    fn set_focus(&mut self, handle: &mut KeyboardInnerHandle<'_>, focus: Option<&WlSurface>, serial: Serial);
+
+    /// The data about the event that started the grab.
+    fn start_data(&self) -> &GrabStartData;
+}
+
 /// An handle to a keyboard handler
 ///
 /// It can be cloned and all clones manipulate the same internal state.
@@ -337,6 +417,42 @@ pub struct KeyboardHandle {
 }
 
 impl KeyboardHandle {
+    /// Change the current grab on this keyboard to the provided grab
+    ///
+    /// Overwrites any current grab.
+    pub fn set_grab<G: KeyboardGrab + 'static>(&self, grab: G, serial: Serial) {
+        self.arc.internal.borrow_mut().grab = GrabStatus::Active(serial, Box::new(grab));
+    }
+
+    /// Remove any current grab on this keyboard, resetting it to the default behavior
+    pub fn unset_grab(&self) {
+        self.arc.internal.borrow_mut().grab = GrabStatus::None;
+    }
+
+    /// Check if this keyboard is currently grabbed with this serial
+    pub fn has_grab(&self, serial: Serial) -> bool {
+        let guard = self.arc.internal.borrow_mut();
+        match guard.grab {
+            GrabStatus::Active(s, _) => s == serial,
+            _ => false,
+        }
+    }
+
+    /// Check if this keyboard is currently being grabbed
+    pub fn is_grabbed(&self) -> bool {
+        let guard = self.arc.internal.borrow_mut();
+        !matches!(guard.grab, GrabStatus::None)
+    }
+
+    /// Returns the start data for the grab, if any.
+    pub fn grab_start_data(&self) -> Option<GrabStartData> {
+        let guard = self.arc.internal.borrow();
+        match &guard.grab {
+            GrabStatus::Active(_, g) => Some(g.start_data().clone()),
+            _ => None,
+        }
+    }
+
     /// Handle a keystroke
     ///
     /// All keystrokes from the input backend should be fed _in order_ to this method of the
@@ -392,14 +508,12 @@ impl KeyboardHandle {
             KeyState::Pressed => WlKeyState::Pressed,
             KeyState::Released => WlKeyState::Released,
         };
-        guard.with_focused_kbds(|kbd, _| {
-            // key event must be sent before modifers event for libxkbcommon
-            // to process them correctly
-            kbd.key(serial.into(), time, keycode, wl_state);
-            if let Some((dep, la, lo, gr)) = modifiers {
-                kbd.modifiers(serial.into(), dep, la, lo, gr);
-            }
-        });
+        guard.with_grab(
+            move |mut handle, grab| {
+                grab.input(&mut handle, keycode, wl_state, modifiers, serial, time);
+            },
+            self.arc.logger.clone(),
+        );
         if guard.focus.is_some() {
             trace!(self.arc.logger, "Input forwarded to client");
         } else {
@@ -417,44 +531,13 @@ impl KeyboardHandle {
     /// a [`wl_keyboard::Event::Enter`](wayland_server::protocol::wl_keyboard::Event::Enter) event will be sent.
     pub fn set_focus(&self, focus: Option<&WlSurface>, serial: Serial) {
         let mut guard = self.arc.internal.borrow_mut();
-
-        let same = guard
-            .focus
-            .as_ref()
-            .and_then(|f| focus.map(|s| s.as_ref().equals(f.as_ref())))
-            .unwrap_or(false);
-
-        if !same {
-            // unset old focus
-            guard.with_focused_kbds(|kbd, s| {
-                kbd.leave(serial.into(), s);
-            });
-
-            // set new focus
-            guard.focus = focus.cloned();
-            let (dep, la, lo, gr) = guard.serialize_modifiers();
-            let keys = guard.serialize_pressed_keys();
-            guard.with_focused_kbds(|kbd, surface| {
-                kbd.enter(serial.into(), surface, keys.clone());
-                // Modifiers must be send after enter event.
-                kbd.modifiers(serial.into(), dep, la, lo, gr);
-            });
-            {
-                let KbdInternal {
-                    ref focus,
-                    ref mut focus_hook,
-                    ..
-                } = *guard;
-                focus_hook(focus.as_ref());
-            }
-            if guard.focus.is_some() {
-                trace!(self.arc.logger, "Focus set to new surface");
-            } else {
-                trace!(self.arc.logger, "Focus unset");
-            }
-        } else {
-            trace!(self.arc.logger, "Focus unchanged");
-        }
+        guard.pending_focus = focus.cloned();
+        guard.with_grab(
+            move |mut handle, grab| {
+                grab.set_focus(&mut handle, focus, serial);
+            },
+            self.arc.logger.clone(),
+        );
     }
 
     /// Check if given client currently has keyboard focus
@@ -543,3 +626,129 @@ pub(crate) fn implement_keyboard(keyboard: Main<WlKeyboard>, handle: Option<&Key
 
     keyboard.deref().clone()
 }
+
+/// This inner handle is accessed from inside a keyboard grab logic, and directly
+/// sends event to the client
+#[derive(Debug)]
+pub struct KeyboardInnerHandle<'a> {
+    inner: &'a mut KbdInternal,
+    logger: ::slog::Logger,
+}
+
+impl<'a> KeyboardInnerHandle<'a> {
+    /// Change the current grab on this keyboard to the provided grab
+    ///
+    /// Overwrites any current grab.
+    pub fn set_grab<G: KeyboardGrab + 'static>(&mut self, serial: Serial, grab: G) {
+        self.inner.grab = GrabStatus::Active(serial, Box::new(grab));
+    }
+
+    /// Remove any current grab on this keyboard, resetting it to the default behavior
+    ///
+    /// This will also restore the focus of the underlying keyboard if restore_focus
+    /// is [`true`]
+    pub fn unset_grab(&mut self, serial: Serial, restore_focus: bool) {
+        self.inner.grab = GrabStatus::None;
+        // restore the focus
+        if restore_focus {
+            let focus = self.inner.pending_focus.clone();
+            self.set_focus(focus.as_ref(), serial);
+        }
+    }
+
+    /// Access the current focus of this keyboard
+    pub fn current_focus(&self) -> Option<&WlSurface> {
+        self.inner.focus.as_ref()
+    }
+
+    /// Send the input to the focused keyboards
+    pub fn input(
+        &mut self,
+        keycode: u32,
+        key_state: WlKeyState,
+        modifiers: Option<(u32, u32, u32, u32)>,
+        serial: Serial,
+        time: u32,
+    ) {
+        self.inner.with_focused_kbds(|kbd, _| {
+            // key event must be sent before modifers event for libxkbcommon
+            // to process them correctly
+            kbd.key(serial.into(), time, keycode, key_state);
+            if let Some((dep, la, lo, gr)) = modifiers {
+                kbd.modifiers(serial.into(), dep, la, lo, gr);
+            }
+        });
+    }
+
+    /// Set the current focus of this keyboard
+    ///
+    /// If the new focus is different from the previous one, any previous focus
+    /// will be sent a [`wl_keyboard::Event::Leave`](wayland_server::protocol::wl_keyboard::Event::Leave)
+    /// event, and if the new focus is not `None`,
+    /// a [`wl_keyboard::Event::Enter`](wayland_server::protocol::wl_keyboard::Event::Enter) event will be sent.
+    pub fn set_focus(&mut self, focus: Option<&WlSurface>, serial: Serial) {
+        let same = self
+            .inner
+            .focus
+            .as_ref()
+            .and_then(|f| focus.map(|s| s.as_ref().equals(f.as_ref())))
+            .unwrap_or(false);
+
+        if !same {
+            // unset old focus
+            self.inner.with_focused_kbds(|kbd, s| {
+                kbd.leave(serial.into(), s);
+            });
+
+            // set new focus
+            self.inner.focus = focus.cloned();
+            let (dep, la, lo, gr) = self.inner.serialize_modifiers();
+            let keys = self.inner.serialize_pressed_keys();
+            self.inner.with_focused_kbds(|kbd, surface| {
+                kbd.enter(serial.into(), surface, keys.clone());
+                // Modifiers must be send after enter event.
+                kbd.modifiers(serial.into(), dep, la, lo, gr);
+            });
+            {
+                let KbdInternal {
+                    ref focus,
+                    ref mut focus_hook,
+                    ..
+                } = *self.inner;
+                focus_hook(focus.as_ref());
+            }
+            if self.inner.focus.is_some() {
+                trace!(self.logger, "Focus set to new surface");
+            } else {
+                trace!(self.logger, "Focus unset");
+            }
+        } else {
+            trace!(self.logger, "Focus unchanged");
+        }
+    }
+}
+
+// The default grab, the behavior when no particular grab is in progress
+struct DefaultGrab;
+
+impl KeyboardGrab for DefaultGrab {
+    fn input(
+        &mut self,
+        handle: &mut KeyboardInnerHandle<'_>,
+        keycode: u32,
+        key_state: WlKeyState,
+        modifiers: Option<(u32, u32, u32, u32)>,
+        serial: Serial,
+        time: u32,
+    ) {
+        handle.input(keycode, key_state, modifiers, serial, time)
+    }
+
+    fn set_focus(&mut self, handle: &mut KeyboardInnerHandle<'_>, focus: Option<&WlSurface>, serial: Serial) {
+        handle.set_focus(focus, serial)
+    }
+
+    fn start_data(&self) -> &GrabStartData {
+        unreachable!()
+    }
+}

src/wayland/seat/mod.rs

@@ -41,12 +41,12 @@ mod pointer;
 
 pub use self::{
     keyboard::{
-        keysyms, Error as KeyboardError, FilterResult, KeyboardHandle, Keysym, KeysymHandle, ModifiersState,
-        XkbConfig,
+        keysyms, Error as KeyboardError, FilterResult, GrabStartData as KeyboardGrabStartData, KeyboardGrab,
+        KeyboardHandle, KeyboardInnerHandle, Keysym, KeysymHandle, ModifiersState, XkbConfig,
     },
     pointer::{
-        AxisFrame, CursorImageAttributes, CursorImageStatus, GrabStartData, PointerGrab, PointerHandle,
-        PointerInnerHandle,
+        AxisFrame, CursorImageAttributes, CursorImageStatus, GrabStartData as PointerGrabStartData,
+        PointerGrab, PointerHandle, PointerInnerHandle,
     },
 };
 

src/wayland/seat/pointer.rs

@@ -677,16 +677,18 @@ impl PointerGrab for DefaultGrab {
         time: u32,
     ) {
         handle.button(button, state, serial, time);
-        handle.set_grab(
-            serial,
-            ClickGrab {
-                start_data: GrabStartData {
-                    focus: handle.current_focus().cloned(),
-                    button,
-                    location: handle.current_location(),
+        if state == ButtonState::Pressed {
+            handle.set_grab(
+                serial,
+                ClickGrab {
+                    start_data: GrabStartData {
+                        focus: handle.current_focus().cloned(),
+                        button,
+                        location: handle.current_location(),
+                    },
                 },
-            },
-        );
+            );
+        }
     }
     fn axis(&mut self, handle: &mut PointerInnerHandle<'_>, details: AxisFrame) {
         handle.axis(details);

src/wayland/shell/xdg/mod.rs

@@ -383,6 +383,14 @@ xdg_role!(
         /// It is a protocol error to call commit on a wl_surface with
         /// the xdg_popup role when no parent is set.
         pub parent: Option<wl_surface::WlSurface>,
+
+        /// Defines if the surface has received at least one commit
+        ///
+        /// This can be used to check for protocol errors, like
+        /// checking if a popup requested a grab after it has been
+        /// mapped.
+        pub committed: bool,
+
         popup_handle: Option<xdg_popup::XdgPopup>
     }
 );
@@ -1399,6 +1407,7 @@ impl PopupSurface {
                 .unwrap()
                 .lock()
                 .unwrap();
+            attributes.committed = true;
             if attributes.initial_configure_sent {
                 if let Some(state) = attributes.last_acked {
                     if state != attributes.current {