fix(ibus): wait for FocusIn before commit on scoped injection (#533)
* fix(ibus): wait for FocusIn before commit on scoped injection Cold first activation of the vocalinux engine on GNOME Wayland could commit before mutter bound a client context, so logs reported success while the first dictation of a session was dropped. Fixes #523 * refactor(ibus): clear focus event directly in do_destroy Drop the clear_focus callback on the destroy helper; one local branch is enough. * test(ibus): cover FocusIn gate helpers for patch coverage Extract PING/focus-wait helpers and exercise them so codecov/patch clears the 80% target on the #523 fix.
This commit is contained in:
@@ -599,6 +599,39 @@ def _handle_engine_destroy(
|
||||
return next_active_instance
|
||||
|
||||
|
||||
def _ibus_ping_status(active_instance: object, focus_event: threading.Event) -> bytes:
|
||||
"""Readiness bytes for the inject socket PING probe (#523).
|
||||
|
||||
FOCUSED: bound to a client context (safe to commit).
|
||||
OK: engine instance exists but FocusIn has not arrived yet.
|
||||
NO_ENGINE: no active instance.
|
||||
"""
|
||||
# Use identity checks: VocalinuxEngine may subclass MagicMock in tests,
|
||||
# so truthiness on the instance is unreliable.
|
||||
if active_instance is None:
|
||||
return b"NO_ENGINE"
|
||||
if focus_event.is_set():
|
||||
return b"FOCUSED"
|
||||
return b"OK"
|
||||
|
||||
|
||||
def _ibus_ensure_client_focus(
|
||||
active_instance: object,
|
||||
focus_event: threading.Event,
|
||||
timeout: float,
|
||||
) -> bool:
|
||||
"""Wait for FocusIn before commit_text when needed (#523).
|
||||
|
||||
Returns True when a client context is focused (or already was).
|
||||
Returns False when there is no instance or FocusIn times out.
|
||||
"""
|
||||
if active_instance is None:
|
||||
return False
|
||||
if focus_event.is_set():
|
||||
return True
|
||||
return focus_event.wait(timeout=timeout)
|
||||
|
||||
|
||||
def switch_engine(engine_name: str) -> bool:
|
||||
"""Switch to the specified IBus engine."""
|
||||
import time
|
||||
@@ -787,9 +820,14 @@ class VocalinuxEngine(IBus.Engine if IBUS_AVAILABLE else object):
|
||||
|
||||
# Class-level reference to active engine instance
|
||||
_active_instance: Optional["VocalinuxEngine"] = None
|
||||
# Set only after do_focus_in binds the engine to a client input context.
|
||||
# do_enable alone is not enough for commit_text delivery (#523).
|
||||
_focus_event: threading.Event = threading.Event()
|
||||
_socket_server: Optional[threading.Thread] = None
|
||||
_server_socket: Optional[socket.socket] = None
|
||||
_server_running: bool = False
|
||||
# Max wait for FocusIn before committing text (cold first activation).
|
||||
_FOCUS_WAIT_TIMEOUT_S: float = 1.0
|
||||
|
||||
def __init__(self):
|
||||
"""Initialize the Vocalinux IBus engine."""
|
||||
@@ -801,6 +839,8 @@ class VocalinuxEngine(IBus.Engine if IBUS_AVAILABLE else object):
|
||||
"""Called when the engine is enabled/selected."""
|
||||
logger.info("VocalinuxEngine enabled - ready for text injection")
|
||||
VocalinuxEngine._active_instance = self
|
||||
# do_enable does not imply a focused client context; wait for do_focus_in
|
||||
# before commit_text (#523).
|
||||
|
||||
# Start socket server if not already running
|
||||
if VocalinuxEngine._socket_server is None:
|
||||
@@ -820,21 +860,26 @@ class VocalinuxEngine(IBus.Engine if IBUS_AVAILABLE else object):
|
||||
super_destroy: Optional[Callable[[], None]] = None
|
||||
if IBUS_AVAILABLE:
|
||||
super_destroy = super().do_destroy
|
||||
was_active = VocalinuxEngine._active_instance is self
|
||||
VocalinuxEngine._active_instance = _handle_engine_destroy(
|
||||
VocalinuxEngine._active_instance,
|
||||
self,
|
||||
IBUS_AVAILABLE,
|
||||
super_destroy,
|
||||
)
|
||||
if was_active:
|
||||
VocalinuxEngine._focus_event.clear()
|
||||
|
||||
def do_focus_in(self) -> None:
|
||||
"""Called when the engine gains focus."""
|
||||
"""Called when the engine gains focus on a client input context."""
|
||||
logger.debug("VocalinuxEngine focus in")
|
||||
VocalinuxEngine._active_instance = self
|
||||
VocalinuxEngine._focus_event.set()
|
||||
|
||||
def do_focus_out(self) -> None:
|
||||
"""Called when the engine loses focus."""
|
||||
"""Called when the engine loses focus on a client input context."""
|
||||
logger.debug("VocalinuxEngine focus out")
|
||||
VocalinuxEngine._focus_event.clear()
|
||||
|
||||
def do_process_key_event(self, keyval: int, keycode: int, state: int) -> bool:
|
||||
"""
|
||||
@@ -899,14 +944,31 @@ class VocalinuxEngine(IBus.Engine if IBUS_AVAILABLE else object):
|
||||
data = conn.recv(65536) # Max text size
|
||||
if data:
|
||||
if data == b"\x00PING":
|
||||
if cls._active_instance:
|
||||
conn.sendall(b"OK")
|
||||
else:
|
||||
conn.sendall(b"NO_ENGINE")
|
||||
conn.sendall(
|
||||
_ibus_ping_status(cls._active_instance, cls._focus_event)
|
||||
)
|
||||
continue
|
||||
|
||||
text = data.decode("utf-8")
|
||||
# Snapshot active instance to avoid TOCTOU race with do_destroy
|
||||
# Wait for FocusIn before commit. On cold first
|
||||
# activation, do_enable can set _active_instance
|
||||
# before mutter binds the client context; committing
|
||||
# then reports success while text is dropped (#523).
|
||||
if (
|
||||
cls._active_instance is not None
|
||||
and not _ibus_ensure_client_focus(
|
||||
cls._active_instance,
|
||||
cls._focus_event,
|
||||
cls._FOCUS_WAIT_TIMEOUT_S,
|
||||
)
|
||||
):
|
||||
logger.warning(
|
||||
"Timed out waiting for IBus FocusIn " "before text commit"
|
||||
)
|
||||
conn.sendall(b"NO_FOCUS")
|
||||
continue
|
||||
|
||||
# Snapshot after focus wait (FocusIn may set a new instance)
|
||||
instance = cls._active_instance
|
||||
if instance:
|
||||
done = threading.Event()
|
||||
@@ -1154,7 +1216,8 @@ class IBusTextInjector:
|
||||
sock.sendall(b"\x00PING")
|
||||
response = sock.recv(64).decode("utf-8")
|
||||
|
||||
if response == "OK":
|
||||
# FOCUSED = client context ready; OK = instance exists (not yet focused).
|
||||
if response in ("OK", "FOCUSED"):
|
||||
logger.debug(f"IBus engine readiness probe succeeded on attempt {attempt + 1}")
|
||||
return
|
||||
|
||||
@@ -1162,7 +1225,7 @@ class IBusTextInjector:
|
||||
logger.debug("IBus engine socket is ready; active engine instance not required")
|
||||
return
|
||||
|
||||
if response != "NO_ENGINE":
|
||||
if response not in ("NO_ENGINE", "OK", "FOCUSED"):
|
||||
logger.warning(f"Unexpected readiness probe response: {response}")
|
||||
except (socket.timeout, FileNotFoundError, ConnectionRefusedError, OSError) as e:
|
||||
logger.debug(f"IBus readiness probe attempt {attempt + 1} failed: {e}")
|
||||
@@ -1285,10 +1348,14 @@ class IBusTextInjector:
|
||||
if response == "OK":
|
||||
logger.debug("Text injection successful")
|
||||
return True
|
||||
elif response == "NO_ENGINE" and attempt < max_attempts - 1:
|
||||
# Engine instance was destroyed (layout switch).
|
||||
# Re-activate to create a new instance and retry.
|
||||
logger.info("Engine instance not active, re-activating and retrying...")
|
||||
elif response in ("NO_ENGINE", "NO_FOCUS") and attempt < max_attempts - 1:
|
||||
# NO_ENGINE: instance destroyed (layout switch).
|
||||
# NO_FOCUS: selected but FocusIn never arrived (#523).
|
||||
# Re-activate and retry.
|
||||
logger.info(
|
||||
f"Engine not ready for commit ({response}); "
|
||||
"re-activating and retrying..."
|
||||
)
|
||||
switch_engine(ENGINE_NAME)
|
||||
time.sleep(0.3)
|
||||
continue
|
||||
|
||||
@@ -12,6 +12,7 @@ import subprocess
|
||||
import sys
|
||||
import tempfile
|
||||
import threading
|
||||
import time
|
||||
import unittest
|
||||
from pathlib import Path
|
||||
from unittest.mock import MagicMock, Mock, call, patch
|
||||
@@ -323,6 +324,7 @@ class TestVocalinuxEngine(unittest.TestCase):
|
||||
from vocalinux.text_injection.ibus_engine import VocalinuxEngine
|
||||
|
||||
VocalinuxEngine._active_instance = None
|
||||
VocalinuxEngine._focus_event.clear()
|
||||
VocalinuxEngine._socket_server = None
|
||||
VocalinuxEngine._server_socket = None
|
||||
VocalinuxEngine._server_running = False
|
||||
@@ -368,6 +370,7 @@ class TestVocalinuxEngine(unittest.TestCase):
|
||||
engine = VocalinuxEngine()
|
||||
engine.do_focus_in()
|
||||
self.assertEqual(VocalinuxEngine._active_instance, engine)
|
||||
self.assertTrue(VocalinuxEngine._focus_event.is_set())
|
||||
|
||||
def test_vocalinux_engine_do_focus_out(self):
|
||||
"""Test engine focus out signal handler."""
|
||||
@@ -375,9 +378,84 @@ class TestVocalinuxEngine(unittest.TestCase):
|
||||
|
||||
engine = VocalinuxEngine()
|
||||
VocalinuxEngine._active_instance = engine
|
||||
engine.do_focus_in()
|
||||
engine.do_focus_out()
|
||||
# Focus out should not clear the active instance
|
||||
# Focus out should not clear the active instance, only client focus
|
||||
self.assertEqual(VocalinuxEngine._active_instance, engine)
|
||||
self.assertFalse(VocalinuxEngine._focus_event.is_set())
|
||||
|
||||
def test_do_enable_does_not_mark_client_focused(self):
|
||||
"""Enable alone must not claim FocusIn readiness (#523)."""
|
||||
from vocalinux.text_injection.ibus_engine import VocalinuxEngine
|
||||
|
||||
engine = VocalinuxEngine()
|
||||
with patch.object(engine, "_start_socket_server"):
|
||||
engine.do_enable()
|
||||
self.assertEqual(VocalinuxEngine._active_instance, engine)
|
||||
self.assertFalse(VocalinuxEngine._focus_event.is_set())
|
||||
|
||||
def test_ping_status_matrix(self):
|
||||
"""PING status distinguishes no-engine / enabled / focused (#523)."""
|
||||
from vocalinux.text_injection.ibus_engine import VocalinuxEngine, _ibus_ping_status
|
||||
|
||||
focus = threading.Event()
|
||||
self.assertEqual(_ibus_ping_status(None, focus), b"NO_ENGINE")
|
||||
|
||||
engine = VocalinuxEngine()
|
||||
self.assertEqual(_ibus_ping_status(engine, focus), b"OK")
|
||||
|
||||
focus.set()
|
||||
self.assertEqual(_ibus_ping_status(engine, focus), b"FOCUSED")
|
||||
|
||||
def test_ensure_client_focus_true_when_already_focused(self):
|
||||
"""Already-focused engine does not wait before commit."""
|
||||
from vocalinux.text_injection.ibus_engine import _ibus_ensure_client_focus
|
||||
|
||||
focus = threading.Event()
|
||||
focus.set()
|
||||
self.assertTrue(_ibus_ensure_client_focus(object(), focus, timeout=0.05))
|
||||
|
||||
def test_ensure_client_focus_false_without_instance(self):
|
||||
"""No active instance is not ready to commit."""
|
||||
from vocalinux.text_injection.ibus_engine import _ibus_ensure_client_focus
|
||||
|
||||
focus = threading.Event()
|
||||
self.assertFalse(_ibus_ensure_client_focus(None, focus, timeout=0.05))
|
||||
|
||||
def test_ensure_client_focus_waits_then_succeeds(self):
|
||||
"""Cold path: FocusIn arriving after enable unblocks commit (#523)."""
|
||||
from vocalinux.text_injection.ibus_engine import _ibus_ensure_client_focus
|
||||
|
||||
focus = threading.Event()
|
||||
|
||||
def set_focus_soon():
|
||||
time.sleep(0.05)
|
||||
focus.set()
|
||||
|
||||
t = threading.Thread(target=set_focus_soon, daemon=True)
|
||||
t.start()
|
||||
self.assertTrue(_ibus_ensure_client_focus(object(), focus, timeout=0.5))
|
||||
t.join(timeout=1.0)
|
||||
self.assertTrue(focus.is_set())
|
||||
|
||||
def test_ensure_client_focus_times_out(self):
|
||||
"""Timeout without FocusIn returns False so caller can send NO_FOCUS."""
|
||||
from vocalinux.text_injection.ibus_engine import _ibus_ensure_client_focus
|
||||
|
||||
focus = threading.Event()
|
||||
self.assertFalse(_ibus_ensure_client_focus(object(), focus, timeout=0.05))
|
||||
|
||||
def test_do_destroy_clears_focus_when_active(self):
|
||||
"""Destroy of the active instance clears FocusIn readiness."""
|
||||
from vocalinux.text_injection.ibus_engine import VocalinuxEngine
|
||||
|
||||
engine = VocalinuxEngine()
|
||||
engine.do_focus_in()
|
||||
self.assertTrue(VocalinuxEngine._focus_event.is_set())
|
||||
with patch("vocalinux.text_injection.ibus_engine.IBUS_AVAILABLE", False):
|
||||
engine.do_destroy()
|
||||
self.assertIsNone(VocalinuxEngine._active_instance)
|
||||
self.assertFalse(VocalinuxEngine._focus_event.is_set())
|
||||
|
||||
def test_vocalinux_engine_do_process_key_event(self):
|
||||
"""Test engine key event processing."""
|
||||
@@ -1148,6 +1226,32 @@ class TestIBusTextInjectorRetry(unittest.TestCase):
|
||||
# 3 max_attempts: first 2 get NO_ENGINE and retry, 3rd gets NO_ENGINE and fails
|
||||
self.assertEqual(mock_sock.recv.call_count, 3)
|
||||
|
||||
@patch("vocalinux.text_injection.ibus_engine.ensure_ibus_dir")
|
||||
@patch("vocalinux.text_injection.ibus_engine.switch_engine", return_value=True)
|
||||
@patch("vocalinux.text_injection.ibus_engine.is_engine_active", return_value=True)
|
||||
@patch("socket.socket")
|
||||
@patch("vocalinux.text_injection.ibus_engine.SOCKET_PATH")
|
||||
@patch("vocalinux.text_injection.ibus_engine.time")
|
||||
def test_retry_on_no_focus_then_succeeds(
|
||||
self, mock_time, mock_socket_path, mock_socket_cls, mock_active, mock_switch, mock_ensure
|
||||
):
|
||||
"""NO_FOCUS (FocusIn timeout) should re-activate and succeed on retry (#523)."""
|
||||
from vocalinux.text_injection.ibus_engine import IBusTextInjector
|
||||
|
||||
mock_socket_path.exists.return_value = True
|
||||
mock_sock = MagicMock()
|
||||
mock_sock.__enter__.return_value = mock_sock
|
||||
mock_sock.__exit__.return_value = None
|
||||
mock_sock.recv.side_effect = [b"NO_FOCUS", b"OK"]
|
||||
mock_socket_cls.return_value = mock_sock
|
||||
|
||||
injector = IBusTextInjector(auto_activate=False)
|
||||
result = injector.inject_text("hello")
|
||||
|
||||
self.assertTrue(result)
|
||||
self.assertEqual(mock_sock.recv.call_count, 2)
|
||||
self.assertGreaterEqual(mock_switch.call_count, 1)
|
||||
|
||||
|
||||
class TestIBusEngineStartupReadiness(unittest.TestCase):
|
||||
"""Test startup readiness probe behavior in setup flow."""
|
||||
@@ -1224,6 +1328,27 @@ class TestIBusEngineStartupReadiness(unittest.TestCase):
|
||||
|
||||
mock_sock.sendall.assert_called_once_with(b"\x00PING")
|
||||
|
||||
@patch("vocalinux.text_injection.ibus_engine.ensure_ibus_dir")
|
||||
@patch("socket.socket")
|
||||
@patch("vocalinux.text_injection.ibus_engine.SOCKET_PATH")
|
||||
def test_wait_for_engine_ready_accepts_focused(
|
||||
self, mock_socket_path, mock_socket_cls, mock_ensure_dir
|
||||
):
|
||||
"""Readiness probe treats FOCUSED as success (#523)."""
|
||||
from vocalinux.text_injection.ibus_engine import IBusTextInjector
|
||||
|
||||
mock_socket_path.exists.return_value = True
|
||||
mock_sock = MagicMock()
|
||||
mock_sock.__enter__.return_value = mock_sock
|
||||
mock_sock.__exit__.return_value = None
|
||||
mock_sock.recv.return_value = b"FOCUSED"
|
||||
mock_socket_cls.return_value = mock_sock
|
||||
|
||||
injector = IBusTextInjector(auto_activate=False)
|
||||
injector._wait_for_engine_ready(max_attempts=1)
|
||||
|
||||
mock_sock.sendall.assert_called_once_with(b"\x00PING")
|
||||
|
||||
@patch("vocalinux.text_injection.ibus_engine.ensure_ibus_dir")
|
||||
@patch("vocalinux.text_injection.ibus_engine.time")
|
||||
@patch("socket.socket")
|
||||
|
||||
Reference in New Issue
Block a user