github/xrdp
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

chansrv: Fix regression from the previous commit breaking Model A entirely

Browse Source 
The "don't reclaim a deliberately-chosen engine" guard from the last
commit had a logic bug that broke ordinary Model A composition
completely, not just the edge case it was meant to fix.

ibus_bus_set_global_engine() sets the global engine *name*
synchronously, but the actual engine instance (g_engine) is created
asynchronously afterward. During that brief window, the global engine
name is already "XrdpIme" but g_engine is still NULL - the existing
fast path (name == "XrdpIme" && g_engine != NULL) correctly falls
through in that case, same as always. But the new guard only checked
whether the current name differed from last_input_name, and "XrdpIme"
is *always* different from last_input_name by construction (that's
the name of whatever we switched away from). So every commit that
landed in that async window got misclassified as "a different engine
took over" and silently dropped. Composing even one phrase sends
several characters in quick succession, so this was most of them -
not just the intended edge case.

Fixed by checking name == "XrdpIme" first, independent of g_engine's
readiness, before ever reaching the "is this some other engine"
guard - that window means "still connecting", not "something else
took over".

Verified against the exact failure condition: sent a 9-character
phrase rapid-fire immediately after xrdp_input_unicode_init() (no
settle time), landing correctly as a single unbroken phrase with zero
drops. Also re-ran all four trigger-happy scenarios from the previous
commit, plus a new check sending 5 characters rapid-fire immediately
after a from-original reassertion - all pass.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
Liyi Meng
2026-08-18 08:22:45 +00:00
parent 82b4eb6556
commit 55a4f81083
1 changed files with 34 additions and 20 deletions
Download Patch File Download Diff File Expand all files Collapse all files
sesman/chansrv/input_ibus.c
+34 -20
View File
@@ -196,29 +196,43 @@ xrdp_input_enable(void)
desc = ibus_bus_get_global_engine(local_bus);
name = desc != NULL ? ibus_engine_desc_get_name(desc) : NULL;
if (name != NULL && g_ascii_strcasecmp(name, "XrdpIme") == 0 &&
g_engine != NULL)
if (name != NULL && g_ascii_strcasecmp(name, "XrdpIme") == 0)
{
g_object_unref(desc);
g_object_unref(local_bus);
return TRUE;
if (g_engine != NULL)
{
g_object_unref(desc);
g_object_unref(local_bus);
return TRUE;
}
/* XrdpIme is already the global engine, but ibus_bus_set_global_
* engine() only sets the *name* synchronously - the actual
* engine instance (g_engine) is created asynchronously after.
* This is that brief in-flight window, NOT a case of some
* other engine having taken over, so it must not hit the
* "leave a deliberately different engine alone" guard below
* (name == "XrdpIme" is *always* != last_input_name by
* construction, so that guard would otherwise reject every
* commit that lands in this window - which, since composing
* even one phrase sends several characters in quick
* succession, was most of them). Fall through and let the
* queue/prepare-check mechanism wait for g_engine as usual. */
}
/* Only reclaim XrdpIme if the current engine is either unset, or is
* the original one we remembered before ever switching away from
* it (last_input_name) - that's ibus silently reverting on its
* own, which is exactly the scenario this reassertion exists to
* fix. If it's some OTHER named engine, that's a deliberate user
* action (e.g. manually switching to a native engine like
* libpinyin to compose directly in the remote session), possibly
* well before this call - forcibly reclaiming XrdpIme would yank
* that away mid-use for the sake of a single, possibly stale or
* deferred commit (e.g. macOS auto-flushing a composition that was
* left pending after the user moved on to something else). Drop
* this commit instead and leave the user's active choice alone. */
if (last_input_name != NULL && name != NULL &&
g_ascii_strcasecmp(name, last_input_name) != 0)
else if (last_input_name != NULL && name != NULL &&
g_ascii_strcasecmp(name, last_input_name) != 0)
{
/* Only reclaim XrdpIme if the current engine is either unset,
* or is the original one we remembered before ever switching
* away from it (last_input_name) - that's ibus silently
* reverting on its own, which is exactly the scenario this
* reassertion exists to fix. Anything else named is a
* deliberate user action (e.g. manually switching to a native
* engine like libpinyin to compose directly in the remote
* session), possibly well before this call - forcibly
* reclaiming XrdpIme would yank that away mid-use for the sake
* of a single, possibly stale or deferred commit (e.g. macOS
* auto-flushing a composition left pending after the user
* moved on to something else). Drop this commit instead and
* leave the user's active choice alone. */
LOG(LOG_LEVEL_WARNING,
"xrdp_input_enable: global engine is \"%s\", not XrdpIme or "
"the original \"%s\" - leaving it alone rather than "