fix(hotkeys): prefer evdev on GNOME X11 and skip redundant IBus engine restore (#963)

Fixes #954.

GNOME X11: mutter never delivers physical key events to the XRecord extension pynput relies on, so the listener started cleanly but shortcuts stayed dead. Prefer the evdev backend there (same path as Wayland), falling back to pynput when /dev/input is unusable.

Shutdown: `ibus engine <name>` rewrites the whole X server XKB map, and the unconditional restore collapsed GNOME's multi-layout map and broke layout switching. The daemon check now uses _query_active_engine() (no GNOME gsettings fallback) so a cleared engine reads as None; the restore write is skipped when a real engine is live or the target is an xkb:* source GNOME owns, and still runs for real IM engines (#558).
This commit is contained in:
devin-ai-integration[bot]
2026-10-16 09:48:28 -07:00
committed by GitHub
parent ed0ed8ba49
commit e866cb85e4
6 changed files with 244 additions and 17 deletions
+48 -9
View File
@@ -553,6 +553,28 @@ def get_current_engine() -> Optional[str]:
return None
def _query_active_engine() -> Optional[str]:
"""
Query the engine ibus-daemon actually reports, with no GNOME fallbacks.
Unlike :func:`get_current_engine`, this never consults gsettings, so it
can distinguish a really-active engine from a cleared global engine —
``None`` means either "No global engine" or that the query failed.
"""
try:
result = subprocess.run(
["ibus", "engine"], capture_output=True, text=True, timeout=5, env=host_env()
)
stderr_lower = result.stderr.lower() if result.stderr else ""
if "no global engine" in stderr_lower:
return None
if result.returncode == 0:
return result.stdout.strip() or None
except (subprocess.SubprocessError, FileNotFoundError):
pass
return None
def _ibus_im_env_configured() -> bool:
"""True if the process env says IBus is the session input method."""
gtk_im = os.environ.get("GTK_IM_MODULE", "").lower()
@@ -1588,15 +1610,32 @@ class IBusTextInjector:
stop_engine_process()
if restore_engine:
logger.info(f"Restoring previous engine after teardown: {restore_engine}")
restored = False
for attempt in range(3):
if switch_engine(restore_engine):
restored = True
break
time.sleep(0.15 * (attempt + 1))
if not restored:
logger.error(f"Failed to restore IBus engine after teardown: {restore_engine}")
# `ibus engine <name>` applies that engine's XKB map server-wide.
# Re-check the daemon (not the GNOME fallback) after teardown and
# skip the write when a real engine is already active — rewriting
# it collapses GNOME's multi-layout map (#954). The same skip
# applies when teardown left the engine cleared but the restore
# target is an xkb:* source on GNOME: gnome-shell owns that map
# and the write would only collapse it. A cleared global engine
# for a real IM engine still gets restored (#558).
live = _query_active_engine()
if live and live != ENGINE_NAME:
logger.info(f"Skipping engine restore — live engine already active: {live}")
elif live is None and _is_gnome_session() and restore_engine.startswith("xkb:"):
logger.info(
"Skipping engine restore — engine cleared and GNOME owns "
f"the XKB map; not rewriting {restore_engine}"
)
else:
logger.info(f"Restoring previous engine after teardown: {restore_engine}")
restored = False
for attempt in range(3):
if switch_engine(restore_engine):
restored = True
break
time.sleep(0.15 * (attempt + 1))
if not restored:
logger.error(f"Failed to restore IBus engine after teardown: {restore_engine}")
else:
logger.info(
"Skipping engine restoration — no restorable engine was available at shutdown"
@@ -82,6 +82,11 @@ class DesktopEnvironment:
return DesktopEnvironment.UNKNOWN
def _is_gnome_session() -> bool:
"""Check if the current session is running under GNOME."""
return "GNOME" in os.environ.get("XDG_CURRENT_DESKTOP", "").upper()
def create_backend(
preferred_backend: Optional[str] = None,
shortcut: str = DEFAULT_SHORTCUT,
@@ -152,6 +157,21 @@ def create_backend(
)
return None
# On GNOME's X11 session, mutter does not deliver physical key events
# through the XRecord extension that pynput's Listener relies on, so the
# listener starts cleanly but shortcuts stay dead (#954). evdev reads the
# kernel input devices directly, so prefer it here — but fall back to
# pynput when /dev/input is unusable rather than leaving shortcuts dead.
if env == DesktopEnvironment.X11 and _is_gnome_session() and EVDEV_AVAILABLE:
backend = EvdevKeyboardBackend(shortcut=shortcut, mode=mode) # type: ignore
if backend.is_available():
logger.info("Using evdev backend for global shortcuts (GNOME X11)")
return backend
hint = backend.get_permission_hint()
if hint:
logger.warning(f"Evdev backend not usable on GNOME X11: {hint}")
logger.warning("Falling back to pynput; global shortcuts may not respond on GNOME X11")
# Default to pynput for native X11 (or unknown)
if PYNPUT_AVAILABLE:
logger.info("Using pynput backend")
+118 -5
View File
@@ -751,13 +751,14 @@ class TestIBusTextInjector(unittest.TestCase):
self.assertEqual(sent["data"], b"\n")
@patch("vocalinux.text_injection.ibus_engine._query_active_engine", return_value=None)
@patch("vocalinux.text_injection.ibus_engine.stop_engine_process")
@patch("vocalinux.text_injection.ibus_engine.get_current_engine", return_value=None)
@patch("vocalinux.text_injection.ibus_engine.IBUS_AVAILABLE", True)
@patch("vocalinux.text_injection.ibus_engine.ensure_ibus_dir")
@patch("vocalinux.text_injection.ibus_engine.switch_engine", return_value=True)
def test_stop_restores_engine(
self, mock_switch, mock_ensure_dir, mock_get_current, mock_stop_proc
self, mock_switch, mock_ensure_dir, mock_get_current, mock_stop_proc, mock_live
):
"""Test stop() restores cached engine when live engine is unavailable (#558)."""
from vocalinux.text_injection.ibus_engine import IBusTextInjector
@@ -776,13 +777,14 @@ class TestIBusTextInjector(unittest.TestCase):
self.assertEqual([c[0] for c in parent.mock_calls], ["stop_proc", "switch"])
self.assertIsNone(injector._previous_engine)
@patch("vocalinux.text_injection.ibus_engine._query_active_engine", return_value=None)
@patch("vocalinux.text_injection.ibus_engine.stop_engine_process")
@patch("vocalinux.text_injection.ibus_engine.get_current_engine", return_value="xkb:de::deu")
@patch("vocalinux.text_injection.ibus_engine.switch_engine", return_value=True)
@patch("vocalinux.text_injection.ibus_engine.IBUS_AVAILABLE", True)
@patch("vocalinux.text_injection.ibus_engine.ensure_ibus_dir")
def test_stop_prefers_live_engine_over_stale_warmup_cache(
self, mock_ensure_dir, mock_switch, mock_get_current, mock_stop_proc
self, mock_ensure_dir, mock_switch, mock_get_current, mock_stop_proc, mock_live
):
"""Quit must restore the live input source, not a stale warmup cache (#643 Bugbot)."""
from vocalinux.text_injection.ibus_engine import IBusTextInjector
@@ -797,13 +799,14 @@ class TestIBusTextInjector(unittest.TestCase):
mock_switch.assert_called_once_with("xkb:de::deu")
self.assertIsNone(injector._previous_engine)
@patch("vocalinux.text_injection.ibus_engine._query_active_engine", return_value=None)
@patch("vocalinux.text_injection.ibus_engine.stop_engine_process")
@patch("vocalinux.text_injection.ibus_engine.get_current_engine", return_value="xkb:es::spa")
@patch("vocalinux.text_injection.ibus_engine.switch_engine", return_value=True)
@patch("vocalinux.text_injection.ibus_engine.IBUS_AVAILABLE", True)
@patch("vocalinux.text_injection.ibus_engine.ensure_ibus_dir")
def test_stop_restores_current_engine_when_previous_unset(
self, mock_ensure_dir, mock_switch, mock_get_current, mock_stop_proc
self, mock_ensure_dir, mock_switch, mock_get_current, mock_stop_proc, mock_live
):
"""Warmup quit must restore the live engine after register_component teardown (#558)."""
from vocalinux.text_injection.ibus_engine import IBusTextInjector
@@ -822,6 +825,105 @@ class TestIBusTextInjector(unittest.TestCase):
mock_switch.assert_called_once_with("xkb:es::spa")
self.assertEqual([c[0] for c in parent.mock_calls], ["stop_proc", "switch"])
@patch("vocalinux.text_injection.ibus_engine.stop_engine_process")
@patch(
"vocalinux.text_injection.ibus_engine._query_active_engine",
return_value="xkb:us::eng",
)
@patch(
"vocalinux.text_injection.ibus_engine.get_current_engine",
return_value="xkb:us::eng",
)
@patch("vocalinux.text_injection.ibus_engine.switch_engine")
@patch("vocalinux.text_injection.ibus_engine.IBUS_AVAILABLE", True)
@patch("vocalinux.text_injection.ibus_engine.ensure_ibus_dir")
def test_stop_skips_engine_write_when_live_engine_healthy(
self, mock_ensure_dir, mock_switch, mock_get_current, mock_live, mock_stop_proc
):
"""A healthy live engine must not be rewritten at quit (#954).
`ibus engine <name>` applies the engine's XKB map server-wide; on
GNOME the redundant write collapses the user's multi-layout map and
breaks keyboard layout switching after exit.
"""
from vocalinux.text_injection.ibus_engine import IBusTextInjector
injector = IBusTextInjector(auto_activate=False)
injector._previous_engine = "xkb:us::eng"
injector.stop()
mock_get_current.assert_called()
mock_stop_proc.assert_called_once()
mock_switch.assert_not_called()
self.assertIsNone(injector._previous_engine)
@patch("vocalinux.text_injection.ibus_engine._is_gnome_session", return_value=True)
@patch("vocalinux.text_injection.ibus_engine._query_active_engine", return_value=None)
@patch("vocalinux.text_injection.ibus_engine.stop_engine_process")
@patch("vocalinux.text_injection.ibus_engine.get_current_engine", return_value=None)
@patch("vocalinux.text_injection.ibus_engine.switch_engine")
@patch("vocalinux.text_injection.ibus_engine.IBUS_AVAILABLE", True)
@patch("vocalinux.text_injection.ibus_engine.ensure_ibus_dir")
def test_stop_skips_xkb_restore_when_gnome_engine_cleared(
self,
mock_ensure_dir,
mock_switch,
mock_get_current,
mock_stop_proc,
mock_live,
mock_gnome,
):
"""On GNOME, a cleared engine with an xkb:* target must not be rewritten (#954).
gnome-shell owns the XKB map; restoring an xkb engine would only
collapse the user's multi-layout map.
"""
from vocalinux.text_injection.ibus_engine import IBusTextInjector
injector = IBusTextInjector(auto_activate=False)
injector._previous_engine = "xkb:us::eng"
injector.stop()
mock_stop_proc.assert_called_once()
mock_live.assert_called_once()
mock_switch.assert_not_called()
self.assertIsNone(injector._previous_engine)
@patch("vocalinux.text_injection.ibus_engine._is_gnome_session", return_value=True)
@patch("vocalinux.text_injection.ibus_engine._query_active_engine", return_value=None)
@patch("vocalinux.text_injection.ibus_engine.stop_engine_process")
@patch("vocalinux.text_injection.ibus_engine.get_current_engine", return_value=None)
@patch("vocalinux.text_injection.ibus_engine.switch_engine", return_value=True)
@patch("vocalinux.text_injection.ibus_engine.IBUS_AVAILABLE", True)
@patch("vocalinux.text_injection.ibus_engine.ensure_ibus_dir")
def test_stop_restores_im_engine_when_gnome_teardown_cleared(
self,
mock_ensure_dir,
mock_switch,
mock_get_current,
mock_stop_proc,
mock_live,
mock_gnome,
):
"""On GNOME, a cleared engine with a real IM target still restores (#558).
A libpinyin/mozc user left with no global engine would lose input
method support entirely, so the restore write must still run.
"""
from vocalinux.text_injection.ibus_engine import IBusTextInjector
injector = IBusTextInjector(auto_activate=False)
injector._previous_engine = "libpinyin"
injector.stop()
mock_stop_proc.assert_called_once()
mock_live.assert_called_once()
mock_switch.assert_called_once_with("libpinyin")
self.assertIsNone(injector._previous_engine)
@patch("vocalinux.text_injection.ibus_engine.stop_engine_process")
@patch("vocalinux.text_injection.ibus_engine.get_current_engine", return_value=None)
@patch("vocalinux.text_injection.ibus_engine.switch_engine")
@@ -840,6 +942,7 @@ class TestIBusTextInjector(unittest.TestCase):
mock_stop_proc.assert_called_once()
mock_switch.assert_not_called()
@patch("vocalinux.text_injection.ibus_engine._query_active_engine", return_value=None)
@patch("vocalinux.text_injection.ibus_engine.restore_xkb_layout")
@patch("vocalinux.text_injection.ibus_engine.stop_engine_process")
@patch("vocalinux.text_injection.ibus_engine.get_current_engine", return_value="xkb:es::spa")
@@ -847,7 +950,13 @@ class TestIBusTextInjector(unittest.TestCase):
@patch("vocalinux.text_injection.ibus_engine.IBUS_AVAILABLE", True)
@patch("vocalinux.text_injection.ibus_engine.ensure_ibus_dir")
def test_stop_restores_xkb_layout(
self, mock_ensure_dir, mock_switch, mock_get_current, mock_stop_proc, mock_restore_xkb
self,
mock_ensure_dir,
mock_switch,
mock_get_current,
mock_stop_proc,
mock_restore_xkb,
mock_live,
):
"""Test stop() restores XKB after post-teardown engine restore (#292/#558)."""
from vocalinux.text_injection.ibus_engine import IBusTextInjector
@@ -892,6 +1001,10 @@ class TestIBusTextInjector(unittest.TestCase):
mock_restore_xkb.assert_not_called()
mock_switch.assert_not_called()
@patch(
"vocalinux.text_injection.ibus_engine._query_active_engine",
return_value="vocalinux",
)
@patch("vocalinux.text_injection.ibus_engine.stop_engine_process")
@patch(
"vocalinux.text_injection.ibus_engine.get_current_engine",
@@ -901,7 +1014,7 @@ class TestIBusTextInjector(unittest.TestCase):
@patch("vocalinux.text_injection.ibus_engine.IBUS_AVAILABLE", True)
@patch("vocalinux.text_injection.ibus_engine.ensure_ibus_dir")
def test_stop_uses_cached_engine_when_stuck_on_vocalinux(
self, mock_ensure_dir, mock_switch, mock_get_current, mock_stop_proc
self, mock_ensure_dir, mock_switch, mock_get_current, mock_stop_proc, mock_live
):
"""If quit finds vocalinux selected, fall back to the warmup-captured engine (#558)."""
from vocalinux.text_injection.ibus_engine import IBusTextInjector
+6 -2
View File
@@ -627,9 +627,13 @@ class TestIBusTextInjector(unittest.TestCase):
@patch("vocalinux.text_injection.ibus_engine.ensure_ibus_dir")
@patch("vocalinux.text_injection.ibus_engine.stop_engine_process")
@patch("vocalinux.text_injection.ibus_engine.switch_engine")
@patch("vocalinux.text_injection.ibus_engine.get_current_engine", return_value="other-engine")
@patch(
"vocalinux.text_injection.ibus_engine.get_current_engine",
return_value="other-engine",
)
@patch("vocalinux.text_injection.ibus_engine._query_active_engine", return_value=None)
def test_ibus_text_injector_stop(
self, mock_current_engine, mock_switch, mock_stop_proc, mock_ensure_dir
self, mock_live, mock_current_engine, mock_switch, mock_stop_proc, mock_ensure_dir
):
"""Test IBusTextInjector stop restores previous engine after teardown."""
from vocalinux.text_injection.ibus_engine import IBusTextInjector
+2 -1
View File
@@ -428,6 +428,7 @@ class TestStopEngineProcess(unittest.TestCase):
class TestStopRestoreRetry(unittest.TestCase):
"""Coverage for stop() restore retry after teardown."""
@patch("vocalinux.text_injection.ibus_engine._query_active_engine", return_value=None)
@patch("vocalinux.text_injection.ibus_engine.stop_engine_process")
@patch("vocalinux.text_injection.ibus_engine.get_current_engine", return_value="xkb:es::spa")
@patch("vocalinux.text_injection.ibus_engine.switch_engine", return_value=False)
@@ -435,7 +436,7 @@ class TestStopRestoreRetry(unittest.TestCase):
@patch("vocalinux.text_injection.ibus_engine.IBUS_AVAILABLE", True)
@patch("vocalinux.text_injection.ibus_engine.ensure_ibus_dir")
def test_stop_retries_failed_engine_restore(
self, mock_ensure_dir, mock_sleep, mock_switch, mock_get_current, mock_stop_proc
self, mock_ensure_dir, mock_sleep, mock_switch, mock_get_current, mock_stop_proc, mock_live
):
"""stop() retries switch_engine after teardown when the first attempts fail."""
from vocalinux.text_injection.ibus_engine import IBusTextInjector
+50
View File
@@ -115,6 +115,56 @@ class TestDesktopEnvironmentDetect:
mock_pynput.assert_called_once()
mock_evdev.assert_not_called()
def test_create_backend_gnome_x11_prefers_evdev(self) -> None:
"""GNOME X11 must prefer evdev: mutter's XRecord drops physical key events (#954)."""
from vocalinux.ui.keyboard_backends import create_backend
fake = MagicMock()
fake.is_available.return_value = True
env = {
"XDG_SESSION_TYPE": "x11",
"XDG_CURRENT_DESKTOP": "GNOME",
"DISPLAY": ":0",
}
with patch.dict(os.environ, env, clear=True):
with (
patch.object(kb_module, "EVDEV_AVAILABLE", True),
patch.object(kb_module, "EvdevKeyboardBackend", return_value=fake) as mock_evdev,
patch.object(kb_module, "PYNPUT_AVAILABLE", True),
patch.object(kb_module, "PynputKeyboardBackend") as mock_pynput,
):
backend = create_backend()
assert backend is fake
mock_evdev.assert_called_once()
mock_pynput.assert_not_called()
def test_create_backend_gnome_x11_falls_back_to_pynput(self) -> None:
"""GNOME X11 keeps pynput when evdev cannot read /dev/input (#954)."""
from vocalinux.ui.keyboard_backends import create_backend
fake = MagicMock()
fake.is_available.return_value = False
fake.get_permission_hint.return_value = "not in input group"
env = {
"XDG_SESSION_TYPE": "x11",
"XDG_CURRENT_DESKTOP": "ubuntu:GNOME",
"DISPLAY": ":0",
}
with patch.dict(os.environ, env, clear=True):
with (
patch.object(kb_module, "EVDEV_AVAILABLE", True),
patch.object(kb_module, "EvdevKeyboardBackend", return_value=fake),
patch.object(kb_module, "PYNPUT_AVAILABLE", True),
patch.object(
kb_module, "PynputKeyboardBackend", return_value=MagicMock()
) as mock_pynput,
):
backend = create_backend()
assert backend is mock_pynput.return_value
mock_pynput.assert_called_once()
def test_detect_xdg_session_type_case_insensitive(self):
"""Test that XDG_SESSION_TYPE detection is case-insensitive."""
with patch.dict(os.environ, {"XDG_SESSION_TYPE": "WAYLAND"}, clear=False):