1138159 - selecting text with ibus pinyin input method on Xubuntu immediately deletes it

Status: RESOLVED FIXED

(Core :: Widget: Gtk, defect)

Description

[georgezhu]

User Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:36.0) Gecko/20100101 Firefox/36.0
Build ID: 20150224134236

Steps to reproduce:

0. Install Xubuntu 14.10, IBus 1.5.5, IBus Pinyin 1.5.0, Firefox 36.0
1. Type some text into the URL bar or any web form(like at translate.google.com or bugzilla.mozilla.org)
2. Switch input method to "Chinese - Pinyin" from the apt package "ibus-pinyin"
3. Select the text you just wrote in any way(ctrl+a, mouse click and drag, shift+arrowkeys, anything)



Actual results:

The text you were trying to select is immediately deleted, and it's not possible to get it back by pressing ctrl+z. So if you just pressed ctrl+a after entering a large amount of Chinese text, good luck rewriting it all!

It happens to me every time and it's pretty excruciating as I'm trying to learn reading and writing Chinese. I've got to admit my setup is pretty specific though. I wonder if the same problem occurs on other Ubuntus.

I've actually been having this issue since around Firefox 33.0(when I started using the Pinyin) but only now did I get fed up about it enough to make an issue.


Expected results:

The text is just selected and nothing is deleted. This is what happens on every other program I've tested, including mousepad, thunar, guake, terminator, xfce-terminal, LibreOffice Writer, and Mozilla Thunderbird.

Comment 1

[georgezhu]

Interestingly, I noticed on Lubuntu 14.10 that the issue does not exist. The only difference I see is that IBus on Lubuntu is at version 1.5.8 instead of 1.5.5. Maybe if Xubuntu updated that it would fix this issue on Firefox.

Comment 3

[[e-mail]]

I think it is a bug of ibus-pinyin. Now I use ibus-sunpinyin and ibus-googlepinyin instead of ibus-pinyin to avoid this bug.

Comment 4

[[:Aleksej]]

It also affects Chrome
iBus ticket: https://code.google.com/p/ibus/issues/detail?id=1722
Chrome ticket: https://code.google.com/p/chromium/issues/detail?id=404005

Comment 5

[georgezhu]

(In reply to [e-mail] from comment #3)
> I think it is a bug of ibus-pinyin. Now I use ibus-sunpinyin and
> ibus-googlepinyin instead of ibus-pinyin to avoid this bug.

Thanks for suggesting those, openrpj! I also tried ibus-sunpinyin and ibus-googlepinyin and found that they indeed do not have this bug. I didn't like them as much though, because they're not as configurable. Most importantly they lack "Double Pinyin" mode, which I love. Luckily, there was another Pinyin input method called ibus-libpinyin, which seems mostly like ibus-pinyin, but which doesn't have this issue. I'll use that from now on. Only thing I have to get used to is that typing 爱 is now o+l(=ai) instead of a+l(=ai).

Should this bug be closed now as it's really an ibus-pinyin problem and you can work around it by using a different pinyin input package?

Comment 6

[[e-mail]]

(In reply to georgezhu from comment #5)
> Thanks for suggesting those, openrpj! I also tried ibus-sunpinyin and
> ibus-googlepinyin and found that they indeed do not have this bug. I didn't
> like them as much though, because they're not as configurable. Most
> importantly they lack "Double Pinyin" mode, which I love. Luckily, there was
> another Pinyin input method called ibus-libpinyin, which seems mostly like
> ibus-pinyin, but which doesn't have this issue. I'll use that from now on.
> Only thing I have to get used to is that typing 爱 is now o+l(=ai) instead of
> a+l(=ai).
> 
> Should this bug be closed now as it's really an ibus-pinyin problem and you
> can work around it by using a different pinyin input package?

I have tried ibus-libpinyin, it works perfect for me! Thanks! 非常感谢!
I agree with that this bug can be closed now.

Comment 7

[Makoto Kato [:m_kato]]

This may be a regression by bug 1065835.

georgezhu, does this issue occur from Firefox 35?

Comment 8

[Makoto Kato [:m_kato]]

When I test on Ubuntu 15.04 with ibus-pinyin.

- Firefox 34 works fine.
- Firefox 35 doesn't work.

When calling ResetIME on ibus-pinyin, selection is removed.  This seems to be regression by bug 1065835.

Comment 9

[Makoto Kato [:m_kato]]

Nakano-san, why calling ResetIME on OnSelectionChange?  ibus-pinyin seems to remove selected character even if no composition.

I would like know the situation that ResetIME is required.

Comment 10

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(sickish, maybe slower respose)]

The definition of gtk_im_context_reset() said:

https://developer.gnome.org/gtk3/unstable/GtkIMContext.html#gtk-im-context-reset
> Notify the input method that a change such as a change in cursor position has been made.
> This will typically cause the input method to clear the preedit state.

So, IME might need to know surrounding text change with this method call. However, if the selection change is caused by a signal from IME, perhaps, we should ignore selection changes during handling the signal.

Comment 11

[bingyao.liu]

I have encountered exactly the same problem!
I am using Ubuntu 14.04.3 64bit, with FireFox 40.0, and IBus Pinyin 1.5.0

Comment 12

[robinh3123]

I am not sure it is an ibus-pinyin bug:  Using ibus-pinyin on gedit, for example, does not have any problem.

Comment 13

[robinh3123]

In fact, I don't switch IME, but use gedit as my clipboard, so that when something is deleted, I can just copy-paste from the clipboard.

Comment 14

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(sickish, maybe slower respose)]

(In reply to robinh3123 from comment #12)
> I am not sure it is an ibus-pinyin bug:  Using ibus-pinyin on gedit, for
> example, does not have any problem.

This should be a bug of ibus pinyin because we started to call gtk-im-context-reset() at every selection change from Firefox 35. According to the GTK's document, applications should call it every selection change for notifying IME of cursor change. This must be useful for Japanese and South-Asian IMEs because they want/need to refer surrounding text.

Possible hacks for pinyin could be:

* Suppress some actions coming from IME to delete text during a call of gtk-im-context-reset() but there is no composition.  If pinyin does it asynchronously, this hack won't work as expected, though.
* Create a flag to "retrieve-surrounding" signal. If it's not received after last selection change, we don't need to call gtk-im-context-reset() since IME doesn't need to update its caching content.

# Although, if we could retrieve active IME name, we'd not call gtk-im-context-reset() only when pinyin is active. I cannot understand the reason why GTK developers don't think such API is necessary...

Comment 16

[Makoto Kato [:m_kato]]

(In reply to Masayuki Nakano [:masayuki] (Mozilla Japan) (Offline: 9/19, 9/22-9/25, 9/28)) from comment #14)
> (In reply to robinh3123 from comment #12)
> > I am not sure it is an ibus-pinyin bug:  Using ibus-pinyin on gedit, for
> > example, does not have any problem.
> 
> This should be a bug of ibus pinyin because we started to call
> gtk-im-context-reset() at every selection change from Firefox 35. According

Do you report this issue to ibus-pinyn?

gtk_im_context_reset with the latest ibus-pinyin always causes "preedit_start", "preedit_changed" with empty string and "preedit_end" signals.  It means that composition* event is always fired by moving caret :-<.

Comment 17

[Makoto Kato [:m_kato]]

https://github.com/ibus/ibus/issues/1738 ?

Comment 18

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(sickish, maybe slower respose)]

(In reply to Makoto Kato [:m_kato] from comment #16)
> (In reply to Masayuki Nakano [:masayuki] (Mozilla Japan) (Offline: 9/19,
> 9/22-9/25, 9/28)) from comment #14)
> > (In reply to robinh3123 from comment #12)
> > > I am not sure it is an ibus-pinyin bug:  Using ibus-pinyin on gedit, for
> > > example, does not have any problem.
> > 
> > This should be a bug of ibus pinyin because we started to call
> > gtk-im-context-reset() at every selection change from Firefox 35. According
> 
> Do you report this issue to ibus-pinyn?

No, I don't but there is.

> gtk_im_context_reset with the latest ibus-pinyin always causes
> "preedit_start", "preedit_changed" with empty string and "preedit_end"
> signals.  It means that composition* event is always fired by moving caret
> :-<.

Is it happens synchronously? I mean, 

IMContextWrapper::OnSelectionChange(...
{
  ...
  AutoRestore<bool> ignoreComposition(mIgnoreComposition);
  mIgnoreComposition = !IsComposing();
  ResetIME();
  ...
}

If checking mIgnoreComposition could fix this issue, we'd be able to hide the bug.

Comment 19

[Makoto Kato [:m_kato]]

(In reply to Masayuki Nakano [:masayuki] (Mozilla Japan) (Offline: 9/19, 9/22-9/25, 9/28)) from comment #18)
> (In reply to Makoto Kato [:m_kato] from comment #16)
> > (In reply to Masayuki Nakano [:masayuki] (Mozilla Japan) (Offline: 9/19,
> > 9/22-9/25, 9/28)) from comment #14)
> > > (In reply to robinh3123 from comment #12)
> > > > I am not sure it is an ibus-pinyin bug:  Using ibus-pinyin on gedit, for
> > > > example, does not have any problem.
> > > 
> > > This should be a bug of ibus pinyin because we started to call
> > > gtk-im-context-reset() at every selection change from Firefox 35. According
> > 
> > Do you report this issue to ibus-pinyn?
> 
> No, I don't but there is.

OK, I found the bug in ibus-pinyin.  I will send pull request to them.  But that project seems to be inactive a few year, so I don't know they will fix this.


> > gtk_im_context_reset with the latest ibus-pinyin always causes
> > "preedit_start", "preedit_changed" with empty string and "preedit_end"
> > signals.  It means that composition* event is always fired by moving caret
> > :-<.
> 
> Is it happens synchronously? I mean, 

No, asynchronous unfortunately.

Comment 20

[Makoto Kato [:m_kato]]

Fix for ibus-pinyin.  https://github.com/ibus/ibus-pinyin/pull/8

I don't know whether ibus-pinyin is still active project.  So although I recommend moving to fcitx, if you use any distributions, you should request this fix to your distribution's maintainer.

Comment 21

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(sickish, maybe slower respose)]

Thank you, Kato-san.

Another possible fix of our side is, the second approach of comment 14. I'm not sure if we should do that, though, because we need to add new messy flag...

Comment 22

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(sickish, maybe slower respose)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=16853123c518

Comment 23

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(sickish, maybe slower respose)]

Okay, it seems that we can avoid the ibus-pinyin's bug safely. Let's take the hack.

Comment 24

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(sickish, maybe slower respose)]

Attachment: Bug 1138159 Don't reset IM context at selection change when there is no composition and hasn't retrieved surrounding text after last selection change

ibus-pinyin has a bug.  When application calls gtk_im_context_reset(), which means selection is changed in application, ibus-pinyin sents a set of composition signals with empty commit string.  Therefore, selecting text causes removing it.

For preventing it but not breaking the other IMEs which use surrounding text, we should give up to call gtk_im_context_reset() if IME hasn't retrieved surrounding text after the last selection change.  Not having retrieved surrounding text means that the IME doesn't have any cache of contents.  Therefore, not calling gtk_im_context_reset() at selection change must be safe for such IMEs.

Review commit: https://reviewboard.mozilla.org/r/79002/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/79002/

Comment 25

[Makoto Kato [:m_kato]]

Comment on attachment 8791625 [details]
Bug 1138159 Don't reset IM context at selection change when there is no composition and hasn't retrieved surrounding text after last selection change

https://reviewboard.mozilla.org/r/79002/#review77732

::: widget/gtk/IMContextWrapper.cpp:1030
(Diff revision 1)
> +        // surrounding text has been retrieved after last selection range was
> +        // set.  If it's not retrieved, that means that current IME doesn't
> +        // have any content cache, so, it must not need the notification of
> +        // selection change.
> +        if (IsComposing() || retrievedSurroundingSignalReceived) {
> -        ResetIME();
> +            ResetIME();

Why always reset with IsComposing()?

Now, OnSelectionChange is called by bug 1280796 even if composing.  So does this mean that it resets IME's internal state becasue IME has invalid state Gecko doesn't know?

Comment 27

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(sickish, maybe slower respose)]

Comment on attachment 8791625 [details]
Bug 1138159 Don't reset IM context at selection change when there is no composition and hasn't retrieved surrounding text after last selection change

https://reviewboard.mozilla.org/r/79002/#review77732

> Why always reset with IsComposing()?
> 
> Now, OnSelectionChange is called by bug 1280796 even if composing.  So does this mean that it resets IME's internal state becasue IME has invalid state Gecko doesn't know?

No, I only want to restrict when the hack runs as far as possible. Other conditions are checked by the parent |if| and its previous |if|s. Checking IsComposing() with |or| means that this patch doesn't change any behavior during composition.

Comment 28

[Makoto Kato [:m_kato]]

Comment on attachment 8791625 [details]
Bug 1138159 Don't reset IM context at selection change when there is no composition and hasn't retrieved surrounding text after last selection change

https://reviewboard.mozilla.org/r/79002/#review77738

Comment 29

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/49e110586b98
Don't reset IM context at selection change when there is no composition and hasn't retrieved surrounding text after last selection change r=m_kato

Comment 30

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/49e110586b98

Comment 31

[Shian-Yow Wu [:swu]]

The issue also happens in gcin input method(the most popular IME for Taiwan Linux users), I tested on nightly with gcin and found it was fixed.  Thank you!

An issue report for gcin can be found at http://hyperrate.com/thread.php?tid=32587