[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#1019453: ibus: IBus doesn't work correctly in apps that don't support surrounding text



Attached is a patch that matches the upstream PR.

When first introduced IBUS_CAP_SURROUNDING_TEXT was a runtime-flag that allowed
ibus engines to detect whether or not a client application supports
surrounding text. This was determined by the return value of the `retrieve-surrounding`
signal. If the emit of the signal returned false the conclusion was that
the client app doesn't support surrounding text and the IBUS_CAP_SURROUNDING_TEXT
flag was cleared.

Some time later it was discovered that Firefox (which implements surrounding text) sometimes returns `false` from the signal handler for the first character, but succeeds for subsequent calls. The workaround to fix this bug was that instead
of clearing the IBUS_CAP_SURROUNDING_TEXT flag a warning was output.
This makes the IBUS_CAP_SURROUNDING_TEXT flag basically a compile time flag.

However, this workaround has the side effect that now it is impossible for engines to detect if the client implements surrounding text. This has the effect that
apps that don't implement surrounding text no longer work properly, e.g.
the backspace key no longer works properly.

This change tries to undo the negative side effects while still keeping the workaround for Firefox. It does this by checking the name of the client application. If the name is "firefox" we output the warning and don't touch the IBUS_CAP_SURROUNDING_TEXT
flag. For other apps we assume that they don't support surrounding text and
clear the flag.

The risk of this change is that there might be other apps besides Firefox
that implement surrounding text but fail on first call; those would fail again.
On the other side this change fixes things in apps that don't support
surrounding text like Chromium, Gnome Terminal, and others.

From db037f77f62b3510d04b08f5c0a771f9059e8076 Mon Sep 17 00:00:00 2001
From: Eberhard Beilharz <eb1@sil.org>
Date: Tue, 16 Aug 2022 09:24:26 +0200
Subject: [PATCH] Fix surrounding text
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

When first introduced IBUS_CAP_SURROUNDING_TEXT was a runtime-flag that allowed
ibus engines to detect whether or not a client application supports
surrounding text. This was determined by the return value of the `retrieve-surrounding`
signal. If the emit of the signal returned false the conclusion was that
the client app doesn't support surrounding text and the IBUS_CAP_SURROUNDING_TEXT
flag was cleared.

Some time later it was discovered that Firefox (which implements surrounding text)
sometimes returns `false` from the signal handler for the first character, but
succeeds for subsequent calls (https://github.com/ibus/ibus/issues/2054).
The workaround to fix this bug was that instead
of clearing the IBUS_CAP_SURROUNDING_TEXT flag a warning was output.
This makes the IBUS_CAP_SURROUNDING_TEXT flag basically a compile time flag
which tells whether or not ibus can deal with surrounding text.

However, this workaround has the side effect that now it is impossible for engines
to detect if the client implements surrounding text. This has the effect that
apps that don't implement surrounding text no longer work properly, e.g.
the backspace key no longer works. Having ibus support surrounding text is
only half of what's needed because the application (e.g. Chrome) also needs
to support surrounding text. Currently ibus engines have no way to detect if the
application supports surrounding text because the capability is always set.

See #2354 for a scenario that is affected by this behavior: the user types 'n',
the engine outputs 'n'. Next the user types ';', the end result should be that
'n' gets replaced by 'ŋ'.
If the application supports surrounding text (e.g. gedit) the engine can use
`ibus_engine_delete_surrounding_text()` to delete 'n'.
However, if the application doesn't support surrounding text (e.g. Chrome,
gnome-terminal) then the engine has to use `ibus_engine_forward_key_event()` to
send a backspace to delete the previous character. Which means the engine needs
a way to detect whether or not the application supports surrounding text.

An example where this behavior can be seen is in ibus-engine-keyman with the
[IPA (SIL)](https://keyman.com/keyboards/sil_ipa) keyboard.

This change tries to undo the negative side effects while still keeping the workaround
for Firefox. It does this by checking the name of the client application. If the name
is "firefox" we output the warning and don't touch the IBUS_CAP_SURROUNDING_TEXT
flag. For other apps we assume that they don't support surrounding text and
clear the flag.

The risk of this change is that there might be other apps besides Firefox
that implement surrounding text but fail on first call; those would fail again.
On the other side this change fixes things in apps that don't support
surrounding text like Chromium, Gnome Terminal, and others.

It turns out that before commit https://github.com/ibus/ibus/commit/7b3b8c8 the
IBUS_CAP_SURROUNDING_TEXT capability could be used to detect if the application supports
surrounding text.
The changes in commit https://github.com/ibus/ibus/commit/7b3b8c8 modified the behavior so that
instead of removing the IBUS_CAP_SURROUNDING_TEXT capability if the `retrieve-surrounding` signal
returns 0 a warning is output. This works around a bug in Firefox where the
first call sometimes failed but subsequent calls work
(https://github.com/ibus/ibus/issues/2054). However, because
the IBUS_CAP_SURROUNDING_TEXT now is always set it breaks backspace in apps that
don't support surrounding text. This can be seen e.g in Chrome/Chromium as well
as in gnome-terminal. When using the _IPA (SIL)_ keyboard, backspace won't work:
switch to _IPA (SIL)_ keyboard, type a few characters then type backspace.
Notice that this does nothing in Chrome/Chromium and gnome-terminal. See
https://github.com/ibus/ibus/issues/2354 for more examples where things fail.

BUG=https://github.com/ibus/ibus/issues/2054
BUG=https://github.com/ibus/ibus/issues/2354
---
 client/gtk2/ibusimcontext.c | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/client/gtk2/ibusimcontext.c b/client/gtk2/ibusimcontext.c
index bc14df00..ca1ce023 100644
--- a/client/gtk2/ibusimcontext.c
+++ b/client/gtk2/ibusimcontext.c
@@ -576,12 +576,17 @@ _request_surrounding_text (IBusIMContext *context)
         g_signal_emit (context, _signal_retrieve_surrounding_id, 0,
                        &return_value);
         if (!return_value) {
-            /* #2054 firefox::IMContextWrapper::GetCurrentParagraph() could
-             * fail with the first typing on firefox but it succeeds with
-             * the second typing.
-             */
-            g_warning ("%s has no capability of surrounding-text feature",
-                       g_get_prgname ());
+            if (strcmp(g_get_prgname(), "firefox")) {
+                context->caps &= ~IBUS_CAP_SURROUNDING_TEXT;
+                ibus_input_context_set_capabilities(context->ibuscontext, context->caps);
+            } else {
+                /* #2054 firefox::IMContextWrapper::GetCurrentParagraph() could
+                * fail with the first typing on firefox but it succeeds with
+                * the second typing, so don't disable surrounding text.
+                */
+                g_warning("%s has no capability of surrounding-text feature",
+                        g_get_prgname());
+            }
         }
     }
 }
--
2.37.3

Attachment: OpenPGP_0xE9140597606020D3.asc
Description: OpenPGP public key

Attachment: OpenPGP_signature
Description: OpenPGP digital signature


Reply to: