XWaylandKeyboardGrab interferes with lock surface keyboard input

[ids1024]

This issue occurs with XWayland clients that use keyboard grabs, like virtualbox (pop-os/cosmic-comp#449). Or a minimal X client like this one:

use x11rb::{
    connection::Connection,
    protocol::{Event, xproto::*},
};
use xkeysym::Keysym;

fn main() -> Result<(), Box<dyn std::error::Error>> {
    let (conn, screen_num) = x11rb::connect(None).unwrap();
    let screen = &conn.setup().roots[screen_num];
    let win_id = conn.generate_id()?;
    conn.create_window(
        x11rb::COPY_DEPTH_FROM_PARENT,
        win_id,
        screen.root,
        0,
        0,
        100,
        100,
        0,
        WindowClass::INPUT_OUTPUT,
        screen.root_visual,
        &CreateWindowAux::new()
            .background_pixel(screen.white_pixel)
            .event_mask(EventMask::FOCUS_CHANGE | EventMask::KEY_RELEASE),
    )?;
    conn.map_window(win_id)?;
    conn.flush()?;
    let mut grabbed = false;
    println!("Press any key to grab keyboard. Press `esc` to release grab.");
    loop {
        match conn.wait_for_event()? {
            Event::FocusIn(_) => {
                println!("Focus in");
            }
            Event::FocusOut(_) => {
                println!("Focus out");
            }
            Event::KeyRelease(KeyPressEvent { detail, state, .. }) => {
                let level = if state.contains(KeyButMask::SHIFT) {
                    1
                } else {
                    0
                };
                let mapping = conn.get_keyboard_mapping(detail, 1)?.reply()?;
                let keysym = Keysym::from(mapping.keysyms[level]);
                if keysym == Keysym::NoSymbol {
                    continue;
                }
                println!("key: {:?}", keysym);
                if grabbed && keysym == Keysym::Escape {
                    conn.ungrab_keyboard(x11rb::CURRENT_TIME)?;
                    conn.flush()?;
                    println!("Ungrabbed keyboard");
                    grabbed = false;
                } else if !grabbed && keysym != Keysym::Escape {
                    let res = conn
                        .grab_keyboard(
                            false,
                            win_id,
                            x11rb::CURRENT_TIME,
                            GrabMode::ASYNC,
                            GrabMode::ASYNC,
                        )?
                        .reply()?;
                    conn.flush()?;
                    println!("Grabbed keyboard: {:?}", res.status);
                    grabbed = true;
                }
            }
            _ => {}
        }
    }
}

This can be tested with something like sleep 10 && loginctl lock-session & cargo run with a locker client like cosmic-greeter running.

This can't be tested on anvil (since it doesn't support session lock currently) or niri (because it doesn't support XWayland). But on cosmic-comp, when the XWayland keyboard grab is active, it remains active when the session is locked, and only the X client, and not the session lock surface, gets keyboard input. With this test client, esc can be used to release the grab and be able to interact with the lock surface.

On Sway, when the session lock is created, the XWayland window gets a FocusOut event, no longer intercepts key presses, then gets a FocusIn when the lock surface is destroyed, and it then goes back to intercepting key presses. This is presumably the correct behavior. If we released the grab, the protocol has no way to tell X11 that the grab is released and should be taken again.

This can potentially be fixed without changes in smithay (except maybe adding a Clone impl to XWaylandKeyboardGrab) by overriding the default XWaylandKeyboardGrabHandler::grab or otherwise changing things, but it would be good if possible if Smithay itself could help somehow.

I see a few ways to address this. Not sure what's best.

• Session lock doesn't use a Smithay KeyboardGrab. So this could also be changed to not use a grab like that, and be handled in the compositor input logic similarly to session lock. But with lower precedence.
• I guess a mechanism to have multiple keyboard grabs with different precedence levels would work (basically the opposite of the former approach), but that probably just makes things more complicated.
• The compositor could clear the keyboard grab in SessionLockHandler::lock. But if the grab is a session lock, it could downcast that to the concrete type, and save it to be restored.
  • Alternately instead of downcasting, XWaylandKeyboardGrabHandler::grab() could save a copy of the grab. Which still needs to be added as a grab somewhere when the session is unlocked.

None of these are obviously a way for Smithay to help handle this. And is there anything else an XWayland keyboard grab needs to not take precedence over? Probably locking should cancel all grabs. Is this the only one that should be restored?

[YaLTeR]

And is there anything else an XWayland keyboard grab needs to not take precedence over?

Presumably, compositor-side dialogs like screenshot interface or polkit auth prompt, etc.

[ids1024]

Good point. Currently on cosmic-comp the screenshot UI and polkit prompt are just layer shell surfaces, but it would be good for them to be privileged clients that are given special precedence over this kind of grab in some way.