diff --git a/src/vocalinux/text_injection/ibus_engine.py b/src/vocalinux/text_injection/ibus_engine.py index 76e3d78..887ade3 100644 --- a/src/vocalinux/text_injection/ibus_engine.py +++ b/src/vocalinux/text_injection/ibus_engine.py @@ -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 ` 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" diff --git a/src/vocalinux/ui/keyboard_backends/__init__.py b/src/vocalinux/ui/keyboard_backends/__init__.py index 9e42c57..4585691 100644 --- a/src/vocalinux/ui/keyboard_backends/__init__.py +++ b/src/vocalinux/ui/keyboard_backends/__init__.py @@ -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") diff --git a/tests/test_ibus_engine.py b/tests/test_ibus_engine.py index 87a3cbe..a9c3e2b 100644 --- a/tests/test_ibus_engine.py +++ b/tests/test_ibus_engine.py @@ -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 ` 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 diff --git a/tests/test_ibus_engine_core.py b/tests/test_ibus_engine_core.py index 342014d..b2df534 100644 --- a/tests/test_ibus_engine_core.py +++ b/tests/test_ibus_engine_core.py @@ -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 diff --git a/tests/test_ibus_engine_utils.py b/tests/test_ibus_engine_utils.py index 30ef29a..4bc7b53 100644 --- a/tests/test_ibus_engine_utils.py +++ b/tests/test_ibus_engine_utils.py @@ -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 diff --git a/tests/test_keyboard_init_ext.py b/tests/test_keyboard_init_ext.py index 292827c..fe78607 100644 --- a/tests/test_keyboard_init_ext.py +++ b/tests/test_keyboard_init_ext.py @@ -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):