Bug 813125 - ibus should not generate empty preedit strings on focus switch
ibus should not generate empty preedit strings on focus switch
Status: CLOSED ERRATA
Product: Fedora
Classification: Fedora
Component: ibus (Show other bugs)
15
Unspecified Unspecified
unspecified Severity unspecified
: ---
: ---
Assigned To: fujiwara
Fedora Extras Quality Assurance
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2012-04-16 21:59 EDT by Luke Hutchison
Modified: 2012-05-10 10:32 EDT (History)
3 users (show)

See Also:
Fixed In Version: ibus-1.4.1-2.fc15
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2012-05-10 10:25:34 EDT
Type: Bug
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)

  None (edit)
Description Luke Hutchison 2012-04-16 21:59:49 EDT
Description of problem:
ibus generates a pre-edit-changed signal on focus switch, with an empty preedit string. It appears the purpose of this is to clear preedit context when you switch windows. Unfortunately this can cause some bad bugs, which various projects have had to work around (e.g. OpenOffice), and even data loss -- see the Gnumeric bug here: https://bugzilla.gnome.org/show_bug.cgi?id=663371  Andreas states in this bug that it needs to be fixed in ibus, not in apps that are broken by this behavior.

How reproducible:
Every time.

Steps to Reproduce:
See the Gnumeric bug linked above.
Comment 1 fujiwara 2012-04-17 02:25:05 EDT
(In reply to comment #0)
> How reproducible:
> Every time.

I don't understand your problem. I don't see any problems.

Do you mean you showed a preedit text on a sheet cell and switch the windows with Alt+Tab?
I don't understand why you need to enable input method on a cell during the switching windows.
I don't see any zero length preedits.

I checked cb_gnm_pane_preedit_changed() and it's called only when the preedit is opened.
I don't understand which codes are problem in cb_gnm_pane_preedit_changed().

Which ibus engines do you use?

If you use ibus-anthy, it provides a property to choose the behavior on focus out with /usr/libexec/ibus-setup-anthy .
Comment 2 Luke Hutchison 2012-04-17 11:13:03 EDT
The behavior manifests with Alt+Tab, but the simplest way to see it is when Gnumeric is opened -- Gnumeric gets a preedit_changed signal with zero-length pre-edit string, and this puts Gnumeric into edit mode, as if F2 had been pushed. This is dangerous because the zero-length preedit string replaces whatever was in the cell if the user does anything other than pressing Esc. So basically if the cursor is over meaningful content, whenever you open a doc in Gnumeric, you *must* press Esc after opening every document.

Please see the screencast here: http://web.mit.edu/~luke_h/www/gnumeric.ogv

I don't have any special ibus engines in use. In the panel applet, I only see "English (Dvorak)" and "English (US)".

cb_gnm_pane_preedit_changed() is what I traced the problem to when hitting Alt-Tab (i.e. the code was definitely called in that situation -- on that computer I also had Pinyin and Hangeul input methods set up as options, but was only using Dvorak at the time). I assume that the same procedure is called when gnumeric is opened, the symptoms are the same -- although Gnumeric going into edit mode right after starting is a new symptom in recent Fedora updates.

Please let me know what other information I can provide so you can duplicate the problem.
Comment 3 fujiwara 2012-04-18 07:15:31 EDT
OK, I can reproduce your problem when open an existent sheet but could not reproduce when I use Alt+Tab.
I think this is not reproduced in Fedora 17 or later.

I agree it's an ibus bug. The ibus gtk client sends the preedit signal when the disable signal is received from ibus-daemon.
The disable signal is added to notify the switching engines to ibus engines but it's not needed when the initial engine is assigned.

The following is the tentative patch for ibus-daemon:
https://github.com/fujiwarat/ibus/commit/9c63ecc4e36479242b2b763da8c66fa171bcc23e

I created the test rpms:
http://fujiwara.fedorapeople.org/ibus/20120418/4001269/
Could you try the ibus-1.4.1-2.fc16.x86_64.rpm if the problem is fixed?
Comment 4 Luke Hutchison 2012-04-18 21:31:04 EDT
On the computer I'm on right now, I can't duplicate the problem of Gnumeric going into edit mode right on document open, but I can duplicate the problem on Alt+Tab, even after installing your patched RPMs, killing every process containing the string "ibus", and re-starting ibus.

With the Alt+Tab case, you have to actually have multiple keyboard layouts installed, and switch layouts once, before Alt+Tab causes Gnumeric to enter edit mode with an empty preedit string.

I also found a new case that causes the problem: switching input methods while the cursor is over a cell in Gnumeric also causes Gnumeric to go into edit mode with an empty preedit string.

Here are some specific instructions on how to reproduce:

1) Create a Gnumeric doc with "x" in A1. Make sure the cursor is in A1, then save the doc and close it.

2) Start iBus and make sure you have at least 2 input methods, e.g. US and Dvorak keyboard layouts. (This computer also has Hangeul and Pinyin, but I don't think those are needed.)  Maybe restart iBus for good measure after adding a second method, I don't know if it needs it.

3) Switch to another keyboard method from the one you started with, e.g. US -> Dvorak layout. It seems that this is needed to trigger the bug, at least with all the latest Fedora updates, and with your patched RPMs.

4) Open a terminal window so you have something else to switch to with Alt+Tab in step 6. Make sure the terminal window is not going to overlap the area of the screen where A1 in your Gnumeric window will be shown, so that you can see the area of the screen containing A1 even when the terminal window is switched to the top with Alt+Tab.

5) Hit Alt+F2 and type "gnumeric Book1.gnumeric" or similar to re-open your doc. Cursor should be in A1 and presumably with your current patch, the cursor should not be in edit mode (although as I say, on the computer I'm currently on, I can't duplicate the "edit mode on open" behavior without your patches -- I can test at home later).

6) Switch to the terminal with Alt+Tab. This should bring the terminal to the top, and the Gnumeric window should enter edit mode. (=> bug #1)

7) Switch back to Gnumeric and hit Esc to take Gnumeric out of edit mode. Now switch input methods using the iBus panel applet. Again Gnumeric will enter edit mode. (=> bug #2)

Please let me know if you can duplicate.  Thanks!
Comment 5 fujiwara 2012-04-19 00:06:25 EDT
Hmm.., your explanation is a bit complicated.
E.g. When you launch a Gnumeric doc and which input focus is changed the layout to 'Dvorak layout' and which is kept the default 'US layout'.
Could you provide the ogv file again?
I understand you prepare the two keymaps so you don't have to show the setup in the ogv.
In my env, the Gnumeric cell works fine with Alt+Tab.
Comment 6 Luke Hutchison 2012-04-19 01:50:52 EDT
OK, I figured it out:

(1) Gnumeric enters edit mode when the input method switches (see the very first part of the video linked below). Just click on a cell with text in in Gnumeric, and then use the iBus panel applet to switch input methods.

(2) Gnumeric enters edit mode when you Alt+Tab to another window, but *only* if the iBus preference "Share the same input method among all applications" is *checked*. This is actually counter-intuitive relative to (1) listed above, because this should mean that the input method does *not* switch on Alt+Tab.

Full video demonstration here:
http://web.mit.edu/~luke_h/www/gnumeric-problem-2.ogv

Thanks for fixing this!
Comment 7 fujiwara 2012-04-19 04:16:29 EDT
(In reply to comment #6)
> (2) Gnumeric enters edit mode when you Alt+Tab to another window, but *only* if
> the iBus preference "Share the same input method among all applications" is
> *checked*. This is actually counter-intuitive relative to (1) listed above,
> because this should mean that the input method does *not* switch on Alt+Tab.

OK, your problem depends on the global input method and I can reproduce it.

I added the check about if preedit is visible:
https://github.com/fujiwarat/ibus/commit/6136a9b1650da562d544b18e41514c9ba26a048a

I created the test rpms:
http://fujiwara.fedorapeople.org/ibus/20120418/4004692/
Could you try ibus-1.4.1-2.fc16.x86_64.rpm and ibus-gtk2-1.4.1-2.fc16.x86_64.rpm if the problem is fixed?
Comment 8 Luke Hutchison 2012-04-19 18:02:00 EDT
You said "your problem depends on..." but you only quoted (2) -- were your patches supposed to fix (1) too?

Those updated packages unfortunately don't fix either problem for me. I tried "Restart" in the iBus panel applet menu after installing the RPMs over the top of the old ones, and when that didn't work, I killed all ibus processes and restarted the daemon. Still didn't fix it.

General question: is there always a 1:1 correspondence between preedit_visible being false and the preedit string being empty? Could it be good to simply catch the sending of an empty preedit string, and prevent this from being sent, in case bugs like this creep in again in future?
Comment 9 fujiwara 2012-04-19 22:47:37 EDT
(In reply to comment #8)
> You said "your problem depends on..." but you only quoted (2) -- were your
> patches supposed to fix (1) too?

Actually I don't understand the case (1). Probably it would be good to point out the actual code in gnumeric. But I think the problem in the first ogv is already fixed.

> Those updated packages unfortunately don't fix either problem for me. I tried
> "Restart" in the iBus panel applet menu after installing the RPMs over the top
> of the old ones, and when that didn't work, I killed all ibus processes and
> restarted the daemon. Still didn't fix it.

Hmm.., curious.
In my env, The problem in your second ogv is fixed in the yesterday's RPM.
You also need to restart gnumeric after ibus-gtk2 is installed.

% strace gnumeric a.gnumeric 2>&1 | grep im-ibus
open("/usr/lib64/gtk-2.0/2.10.0/immodules/im-ibus.so", O_RDONLY|O_CLOEXEC) = 8

The default preedit_visible is false. I'm interested in when preedit_visible is changed. Maybe someone calls 'show-preedit-text' signal.

> General question: is there always a 1:1 correspondence between preedit_visible
> being false and the preedit string being empty? Could it be good to simply
> catch the sending of an empty preedit string, and prevent this from being sent,
> in case bugs like this creep in again in future?

AFAIK, any documents does not say "preedit-changed" signal should not be sent in case that just preedit_visible is changed for zero length. I mind another regression if "preedit-changed" is sent only for non-zero length.
If you could see the ibus codes, the "preedit-changed" signal is called in many parts.
I'd think if the preedit_visible only could resolve your problem to minimize the changes.
If the length would be also checked, I think other parts should check the length besides the problem part.
You could get the Fedora SRPM with 'yumdownloader --source ibus' and rpmbuild could build the Fedora RPMs.
Comment 10 Luke Hutchison 2012-04-20 01:40:26 EDT
For (1), just select a cell in Gnumeric that contains text, then go to the iBus panel applet and switch to another input method while the keyboard focus is still on Gnumeric. Gnumeric will enter edit mode with empty preedit text again. (Same symptom, different trigger.)

The RPMs you posted yesterday do fix all three preedit problems on my home computer (even though they didn't fix my work computer). I'll try again on the work computer tomorrow.  The home computer strace only produces the following two messages on gnumeric startup, and nothing subsequently on IM switch etc.:

stat("/usr/lib64/gtk-2.0/2.10.0/immodules/im-ibus.so", {st_mode=S_IFREG|0755, st_size=27520, ...}) = 0
open("/usr/lib64/gtk-2.0/2.10.0/immodules/im-ibus.so", O_RDONLY|O_CLOEXEC) = 8

Since the "preedit-changed" signal is called from many places in ibus, is it possible there may be other cases that are calling it at incorrect times, or do you think you probably caught them all?
Comment 11 Luke Hutchison 2012-04-20 01:49:32 EDT
Hmm, two more cases that generate empty preedit strings: clicking "Restart" or "Quit" on the ibus panel applet while Gnumeric is focused.
Comment 12 fujiwara 2012-04-20 04:16:24 EDT
(In reply to comment #10)
> For (1), just select a cell in Gnumeric that contains text, then go to the iBus
> panel applet and switch to another input method while the keyboard focus is
> still on Gnumeric. Gnumeric will enter edit mode with empty preedit text again.
> (Same symptom, different trigger.)
> 
> The RPMs you posted yesterday do fix all three preedit problems on my home
> computer (even though they didn't fix my work computer). I'll try again on the

OK, I cannot reproduce your problem with yesterdays' RPMs.

> work computer tomorrow.  The home computer strace only produces the following
> two messages on gnumeric startup, and nothing subsequently on IM switch etc.:
> 
> stat("/usr/lib64/gtk-2.0/2.10.0/immodules/im-ibus.so", {st_mode=S_IFREG|0755,
> st_size=27520, ...}) = 0
> open("/usr/lib64/gtk-2.0/2.10.0/immodules/im-ibus.so", O_RDONLY|O_CLOEXEC) = 8

Yes, the loading .so files are happened at the first access only.
I asked it because you still saw your problem with the updated RPM and that im-ibus.so has the fix.

> 
> Since the "preedit-changed" signal is called from many places in ibus, is it
> possible there may be other cases that are calling it at incorrect times, or do
> you think you probably caught them all?

Almost places emit the "preedit-changed" signal when the preedit_visible is changed so I think you don't see that problem.


(In reply to comment #11)
> Hmm, two more cases that generate empty preedit strings: clicking "Restart" or
> "Quit" on the ibus panel applet while Gnumeric is focused.

Good catch.
I added the check for _ibus_context_destroy_cb() besides _ibus_context_disabled_cb():
https://github.com/fujiwarat/ibus/commit/5fb8bb0b35549c5ac22382418241ef594e94037b

I created the test RPMs:
http://fujiwara.fedorapeople.org/ibus/20120418/4008321/
Could you try ibus-1.4.1-2.fc16.x86_64.rpm and ibus-gtk2-1.4.1-2.fc16.x86_64.rpm ?
Comment 13 Luke Hutchison 2012-04-20 20:37:49 EDT
OK, it looks like these RPMs fix all the problems. Thanks for your patience and for fixing these bugs! (Go ahead and close the Google Code bug too, please.)

(I noticed one inconsistent bit of behavior -- (offtopic for this bug, but it's small so it's probably not worth creating a whole new bug for): upon switching away from the Pinyin input method to another method, the preedit text is cleared. Upon switching away from the Hangeul input method, the preedit text is committed. Not sure if you want to fix that or not while you're at it.)
Comment 14 fujiwara 2012-04-22 22:17:17 EDT
(In reply to comment #13)
> OK, it looks like these RPMs fix all the problems. Thanks for your patience and
> for fixing these bugs! (Go ahead and close the Google Code bug too, please.)

OK, I will update Fedora ibus later after I investigate other bugs.
Yes, I will close the upstream bug later.

> 
> (I noticed one inconsistent bit of behavior -- (offtopic for this bug, but it's
> small so it's probably not worth creating a whole new bug for): upon switching
> away from the Pinyin input method to another method, the preedit text is
> cleared. Upon switching away from the Hangeul input method, the preedit text is
> committed. Not sure if you want to fix that or not while you're at it.)

It depends on the ibus engines.
IBus Hangul is supposed no differents between the preedit text and committed text by Korean users so it likes to commit every preedit text when the engine is switched.
But other engines is not by the users. So it's not a bug.
Comment 16 Fedora Update System 2012-04-28 04:00:56 EDT
ibus-1.4.1-2.fc16 has been submitted as an update for Fedora 16.
https://admin.fedoraproject.org/updates/ibus-1.4.1-2.fc16
Comment 17 Fedora Update System 2012-04-28 04:01:16 EDT
ibus-1.4.1-2.fc15 has been submitted as an update for Fedora 15.
https://admin.fedoraproject.org/updates/ibus-1.4.1-2.fc15
Comment 18 Fedora Update System 2012-04-28 20:21:39 EDT
Package ibus-1.4.1-2.fc15:
* should fix your issue,
* was pushed to the Fedora 15 testing repository,
* should be available at your local mirror within two days.
Update it with:
# su -c 'yum update --enablerepo=updates-testing ibus-1.4.1-2.fc15'
as soon as you are able to.
Please go to the following url:
https://admin.fedoraproject.org/updates/FEDORA-2012-6902/ibus-1.4.1-2.fc15
then log in and leave karma (feedback).
Comment 19 Fedora Update System 2012-05-10 10:25:34 EDT
ibus-1.4.1-2.fc16 has been pushed to the Fedora 16 stable repository.  If problems still persist, please make note of it in this bug report.
Comment 20 Fedora Update System 2012-05-10 10:32:03 EDT
ibus-1.4.1-2.fc15 has been pushed to the Fedora 15 stable repository.  If problems still persist, please make note of it in this bug report.

Note You need to log in before you can comment on or make changes to this bug.