keyboard: Send modifers after key

[ids1024]

Description

Fixes #2184.

be920fb reversed the order of these. But this doesn't appear to be correct.

I'm not sure exactly why the bug happens here, but sending modifiers after key seems to match Gnome.

@hojjatabdollahi what was this meant to fix? If there was some issue here, it may need a different solution.

Checklist

• I agree to smithay's Developer Certificate of Origin.

[YaLTeR]

https://wayland.app/protocols/wayland#wl_keyboard:event:key

If this event produces a change in modifiers, then the resulting wl_keyboard.modifiers event must be sent after this event.

(emph mine) seems this is right

[ids1024]

Right. So there may be some other issue this was meant to fix, but this wasn't the right solution, anyway.

[pgaskin]

Since this is in the shared keyboard code, it also affects #2159 (which makes virtual input use it). I was able to reproduce it for accented characters using ISO_Level5_Latch on a French layout.

It looks like in #2073, the order was swapped about halfway through.

"key event must be sent before modifiers event for libxkbcommon to process them correctly"

smithay/src/input/keyboard/mod.rs

Lines 1701 to 1744 in c952ea8

	/// Send the input to the focused keyboards
	pub fn input(
	&mut self,
	data: &mut D,
	keycode: Keycode,
	key_state: KeyState,
	modifiers: Option<ModifiersState>,
	serial: Serial,
	time: u32,
	) {
	let (focus, _) = match self.inner.focus.as_mut() {
	Some(focus) => focus,
	None => return,
	};
	#[cfg(feature = "wayland_frontend")]
	if let Some(keyboard_handle) = self.seat.get_keyboard() {
	match self.isolated.as_ref() {
	Some(iso) => {
	keyboard_handle.send_keymap(data, &Some(focus), iso.keymap_file(), iso.modifier_state());
	}
	None => {
	let keymap_file = keyboard_handle.arc.keymap.lock().unwrap();
	let mods = self.inner.mods_state;
	keyboard_handle.send_keymap(data, &Some(focus), &keymap_file, mods);
	}
	}
	}
	// key event must be sent before modifiers event for libxkbcommon
	// to process them correctly
	let key = KeysymHandle {
	xkb: match self.isolated.as_ref() {
	Some(iso) => &iso.xkb,
	None => &self.inner.xkb,
	},
	keycode,
	};
	focus.key(self.seat, data, key, key_state, serial, time);
	if let Some(mods) = modifiers {
	focus.modifiers(self.seat, data, mods, serial);
	}
	}

"Modifiers must be sent before the key event so the client resolves the key against the updated modifier state"

smithay/src/input/keyboard/mod.rs

Lines 1532 to 1565 in 804075b

	/// Send the input to the focused keyboards
	pub fn input(
	&mut self,
	data: &mut D,
	keycode: Keycode,
	key_state: KeyState,
	modifiers: Option<ModifiersState>,
	serial: Serial,
	time: u32,
	) {
	let (focus, _) = match self.inner.focus.as_mut() {
	Some(focus) => focus,
	None => return,
	};
	#[cfg(feature = "wayland_frontend")]
	if let Some(keyboard_handle) = self.seat.get_keyboard() {
	let keymap_file = keyboard_handle.arc.keymap.lock().unwrap();
	let mods = self.inner.mods_state;
	keyboard_handle.send_keymap(data, &Some(focus), &keymap_file, mods);
	}
	let key = KeysymHandle {
	xkb: &self.inner.xkb,
	keycode,
	};
	// Modifiers must be sent before the key event so the client resolves the key against the
	// updated modifier state.
	if let Some(mods) = modifiers {
	focus.modifiers(self.seat, data, mods, serial);
	}
	focus.key(self.seat, data, key, key_state, serial, time);
	}

I'm not entirely sure what this means overall though, it's a bit mind-twisting to reason about.

But yeah, this PR seems correct if I understand it correctly.