ibus / ibus-m17n

The m17n engine for IBus
Other
29 stars 10 forks source link

[BUG] typing errors when typing on Firefox 119.0.1 - Fedora 39 #72

Closed CodyShiro closed 11 months ago

CodyShiro commented 11 months ago

Describe the bug Random spaces appear when typing on certain websites Bugged websites I found: https://chat.zalo.me/ and https://www.facebook.com/

To Reproduce Steps to reproduce the behavior: 1, go to one of the 2 listed websites 2, go into any text box and start typing

Expected behavior Clear Vietnamese telex without any additional spaces

Screenshots or videos Screencast from 2023-11-19 08-43-27.webm

ibus-m17n version? ibus-m17n-1.4.23-1.fc39.x86_64

m17n-db versions? I dont know but its rpm from fedoraproject.org

ibus version? IBus 1.5.29-rc2

Distribution and version? Fedora 39 (upgraded from Fedora 38)

Desktop and version? GNOME 45.1

Xorg or Wayland? Wayland

Additional context Add any other context about the problem here.

mike-fabian commented 11 months ago

You are using Vietnamese (vi-telex (m17n)), right?

mike-fabian commented 11 months ago

Here I am using Vietnamese (vi-telex (m17n)) and type cos into

The result of typing cos should be but sometimes it is c ó. See this video:

https://github.com/ibus/ibus-m17n/assets/2330175/c00e63f7-81d8-45b4-a22c-fa6559403902

mike-fabian commented 11 months ago

Similar problem with ibus-hangul-1.5.5-3.fc39.x86_64 in Gnome Wayland, see this video where I type a (a and a space) many times with ibus-hangul. The result should be but sometimes it is :

https://github.com/ibus/ibus-m17n/assets/2330175/4a0ed9eb-fa60-424f-8576-a8d01e071fdc

mike-fabian commented 11 months ago

For ibus-hangul, this seems to be reported here against mutter: https://gitlab.gnome.org/GNOME/mutter/-/issues/3090

mike-fabian commented 11 months ago

This looks like a déjà vu, I think we had this problem before but I cannot find the old bug at the moment. I thought this had been solved.....

mike-fabian commented 11 months ago

@CodyShiro As a workaround you can use vi-telex with ibus-typing-booster instead of with ibus-m17n, see this video:

(The video shows the setup tool of ibus-typing-booster showing the setup for vi-telex and then tying cos into gedit, first with the default settings, then with the option ☑️ Enable suggestions by key (Default is the Tab key) if you don't like the popups with suggestions)

https://github.com/ibus/ibus-m17n/assets/2330175/db59a3f5-876d-45c3-af46-93072086c555

mike-fabian commented 11 months ago

Same problem with Return instead of space, here I type cos + Return many times, sometimes the Return is inserted between the c and the ó:

https://github.com/ibus/ibus-m17n/assets/2330175/e59d4aa7-58f9-49f6-98e2-ed3e5bcc646d

CodyShiro commented 11 months ago

This looks like a déjà vu, I think we had this problem before but I cannot find the old bug at the moment. I thought this had been solved.....

If I'm not mistaken this seems like a similar bug in 2020: https://github.com/ibus/ibus-m17n/issues/19

CodyShiro commented 11 months ago

@CodyShiro As a workaround you can use vi-telex with ibus-typing-booster instead of with ibus-m17n, see this video:

(The video shows the setup tool of ibus-typing-booster showing the setup for vi-telex and then tying cos into gedit, first with the default settings, then with the option ☑️ Enable suggestions by key (Default is the Tab key) if you don't like the popups with suggestions) Peek.2023-11-20.20-54.mp4

Thank you for the workaround, I'm trying it and it seems to be working pretty well. P.S: Really sorry for the late reply since 'm not so active on Github

mike-fabian commented 11 months ago

This looks like a déjà vu, I think we had this problem before but I cannot find the old bug at the moment. I thought this had been solved.....

If I'm not mistaken this seems like a similar bug in 2020: #19

Ah, right, thank you! I wonder why this bug was gone for a while and is back now ...

mike-fabian commented 11 months ago

The same bug is reported here: https://gitlab.gnome.org/GNOME/mutter/-/issues/3090

mike-fabian commented 11 months ago

I tried to work around that problem by committing the space instead of passing it through with return FALSE, but this does not help.

If I quickly commit two things in a row, the first commit ist lost, for example when doing

        ibus_engine_commit_text ((IBusEngine *)m17n, ibus_text_new_from_string ("e"));
        ibus_engine_commit_text ((IBusEngine *)m17n, ibus_text_new_from_string ("s"));

Then the "e" ist lost and only the "s" is seen.

A sleep of 0.1 s between the commits makes it work.

A sleep of 0.1 s between the regular commit of ibus-m17n and the return FALSE which passes the space key which triggered the commit also makes the problem reported in this bug disappear, i.e. when typing os, committing the ó, then sleeping 0.1 s, then passing the space which triggered the commit with return FALSE also makes it work.

Such a sleep seems like a weird workaround though.

mike-fabian commented 11 months ago

I tried to work around that problem by committing the space instead of passing it through with return FALSE, but this does not help.

If I quickly commit two things in a row, the first commit ist lost, for example when doing

        ibus_engine_commit_text ((IBusEngine *)m17n, ibus_text_new_from_string ("e"));
        ibus_engine_commit_text ((IBusEngine *)m17n, ibus_text_new_from_string ("s"));

Then the "e" ist lost and only the "s" is seen.

A sleep of 0.1 s between the commits makes it work.

A sleep of 0.1 s between the regular commit of ibus-m17n and the return FALSE which passes the space key which triggered the commit also makes the problem reported in this bug disappear, i.e. when typing os, committing the ó, then sleeping 0.1 s, then passing the space which triggered the commit with return FALSE also makes it work.

Such a sleep seems like a weird workaround though.

When not even two commits in a row work correctly without adding a sleep inbetween, this shows that the problem is definitely not caused by the input method (ibus-m17n, ibus-hangul, ...) but the problem must be in Gnome or Mutter.

CodyShiro commented 11 months ago

I tried to work around that problem by committing the space instead of passing it through with return FALSE, but this does not help.

If I quickly commit two things in a row, the first commit ist lost, for example when doing

        ibus_engine_commit_text ((IBusEngine *)m17n, ibus_text_new_from_string ("e"));
        ibus_engine_commit_text ((IBusEngine *)m17n, ibus_text_new_from_string ("s"));

Then the "e" ist lost and only the "s" is seen.

A sleep of 0.1 s between the commits makes it work.

A sleep of 0.1 s between the regular commit of ibus-m17n and the return FALSE which passes the space key which triggered the commit also makes the problem reported in this bug disappear, i.e. when typing os, committing the ó, then sleeping 0.1 s, then passing the space which triggered the commit with return FALSE also makes it work.

Such a sleep seems like a weird workaround though.

To be honest with you, that is a very weird workaround even for a newbie coder like me. Anywho, it should be working now right? Also if you haven't I'll go open a bug report on GNOME and mutter to see what thay can do

mike-fabian commented 11 months ago

I tried to work around that problem by committing the space instead of passing it through with return FALSE, but this does not help. If I quickly commit two things in a row, the first commit ist lost, for example when doing

        ibus_engine_commit_text ((IBusEngine *)m17n, ibus_text_new_from_string ("e"));
        ibus_engine_commit_text ((IBusEngine *)m17n, ibus_text_new_from_string ("s"));

Then the "e" ist lost and only the "s" is seen. A sleep of 0.1 s between the commits makes it work. A sleep of 0.1 s between the regular commit of ibus-m17n and the return FALSE which passes the space key which triggered the commit also makes the problem reported in this bug disappear, i.e. when typing os, committing the ó, then sleeping 0.1 s, then passing the space which triggered the commit with return FALSE also makes it work. Such a sleep seems like a weird workaround though.

To be honest with you, that is a very weird workaround even for a newbie coder like me. Anywho, it should be working now right?

No, it is not working, I was just experimenting to find out where the problem is. There is no solution yet.

Also if you haven't I'll go open a bug report on GNOME and mutter to see what thay can do

"Workaround" is maybe not the right word. I just tried to find out whether the problem is in ibus-m17n or not. If I cannot even commit two letters quickly one after each other, then something must be wrong outside of ibus-m17n. I think it is a bug in mutter. Somebody else already reported it. I think this is the bug:

https://gitlab.gnome.org/GNOME/mutter/-/issues/3090

mike-fabian commented 11 months ago

Carlos Garnacho wrote to me:

at the moment it is mutter painstakingly, but it does rely on some nice behavior like IBus not committing twice on a single key event. I'd suggest to make the IM module concatenate everything in the commit buffer itself.

mike-fabian commented 11 months ago

You can try the release candidate ibus-m17n-1.4.25-1 from copr:

https://copr.fedorainfracloud.org/coprs/mfabian/ibus-m17n/

This should fix the problem (See the commit referenced above).

The change avoids the return FALSE; to pass through a typed space or Return and commits a space " " or a newline "\n" instead. But as I found out already (see the comments above), using two commits for one key event sometimes does not work with mutter, Carlos Garnacho said that mutter assumes that there is always only one commit per key event and suggests to concatenate the strings first and then commit only once. My tentative "fix" does that: when a commit is triggered by typing a space or a Return, a space " " or a newline "\n" is added to the string which is due to be committed and then the commit is done. This seems to work and fix the problem and I found no disadvantages so far.

Please test this!

I still think mutter should allow more than one commit per key event and of course reversing the order of a commit and a return FALSE; is also bad, this used to work. Carlos says

nothing has changed substantially there on the Mutter side

but this used to work, it worked in Fedora 38 Gnome Wayland with gnome-shell-44.6-1.fc38.x86_64 mutter-44.6-1.fc38.x86_64.

CodyShiro commented 11 months ago

You can try the release candidate ibus-m17n-1.4.25-1 from copr:

https://copr.fedorainfracloud.org/coprs/mfabian/ibus-m17n/

This should fix the problem (See the commit referenced above).

The change avoids the return FALSE; to pass through a typed space or Return and commits a space " " or a newline "\n" instead. But as I found out already (see the comments above), using two commits for one key event sometimes does not work with mutter, Carlos Garnacho said that mutter assumes that there is always only one commit per key event and suggests to concatenate the strings first and then commit only once. My tentative "fix" does that: when a commit is triggered by typing a space or a Return, a space " " or a newline "\n" is added to the string which is due to be committed and then the commit is done. This seems to work and fix the problem and I found no disadvantages so far.

Please test this!

I still think mutter should allow more than one commit per key event and of course reversing the order of a commit and a return FALSE; is also bad, this used to work. Carlos says

nothing has changed substantially there on the Mutter side

but this used to work, it worked in Fedora 38 Gnome Wayland with gnome-shell-44.6-1.fc38.x86_64 mutter-44.6-1.fc38.x86_64.

I'm very sorry but could you guide me through the installation process, I've been trying to figure it out for some time now

Thanks in advance!

mike-fabian commented 11 months ago

No problem, you can install it by first enabling the copr repository with

sudo dnf copr enable mfabian/ibus-m17n 

and then update:

sudo dnf update ibus-m17n
CodyShiro commented 11 months ago

You can try the release candidate ibus-m17n-1.4.25-1 from copr:

https://copr.fedorainfracloud.org/coprs/mfabian/ibus-m17n/

This should fix the problem (See the commit referenced above).

The change avoids the return FALSE; to pass through a typed space or Return and commits a space " " or a newline "\n" instead. But as I found out already (see the comments above), using two commits for one key event sometimes does not work with mutter, Carlos Garnacho said that mutter assumes that there is always only one commit per key event and suggests to concatenate the strings first and then commit only once. My tentative "fix" does that: when a commit is triggered by typing a space or a Return, a space " " or a newline "\n" is added to the string which is due to be committed and then the commit is done. This seems to work and fix the problem and I found no disadvantages so far.

Please test this!

I still think mutter should allow more than one commit per key event and of course reversing the order of a commit and a return FALSE; is also bad, this used to work. Carlos says

nothing has changed substantially there on the Mutter side

but this used to work, it worked in Fedora 38 Gnome Wayland with gnome-shell-44.6-1.fc38.x86_64 mutter-44.6-1.fc38.x86_64.

gg.webm

Unfortunately, it seems to not work on my system, at the moment I've used dnf copr enable mfabian/ibus-m17n and dnf install ibus-m17n to install the copr repo Please contact me if I've dont anything wrong

mike-fabian commented 11 months ago

So you have ibus-m17n-1.4.25 installed now, right? You can check with

$ rpm -q ibus-m17n
ibus-m17n-1.4.25-1.fc39.x86_64

And you need to restart ibus:

ibus restart

or log out of your Gnome session and log in again.

CodyShiro commented 11 months ago

So you have ibus-m17n-1.4.25 installed now, right? You can check with

$ rpm -q ibus-m17n
ibus-m17n-1.4.25-1.fc39.x86_64

And you need to restart ibus:

ibus restart

or log out of you Gnome session and log in again.

Yes, I've just installed ibus-m17n-1.4.25 and trying it out right now

CodyShiro commented 11 months ago

Screencast from 2023-11-28 21-18-03.webm

Seems to be working perfectly!

CodyShiro commented 11 months ago

@mike-fabian I spoke too soon, there still seems to be a problem with typing underscores (say in usernames) with the new update: temp.webm

P.S: Yes I just spent 10 minutes typing random phrases to find bugs :)

mike-fabian commented 11 months ago

Ah, OK, if you look at my patch:

https://github.com/ibus/ibus-m17n/commit/e2cea7168e1eb173be8f927789a890aa654f2cff

you see that I used the trick to combine the key which caused the commit and the commit of the word before that key to one string and then commit that string in a single action only for

and I did not do it for the underscore _.

Of course now I realise that lots of other characters except _ will have that problem as well, every character which causes a commit needs to be added to that workaround. Other examples for characters causing commits are ( and ) and many others like .;:^.

mike-fabian commented 11 months ago

I don’t want to add every such character individually, there are probably very many characters which trigger a commit and then still get passed through using return FALSE; and then Mutter may reorder them before the commit.

But maybe I can do it like this:

If the character causing the commit is Return or KP_Enter, add a "\n" to the string to be committed. For all other characters causing a commit, if that character has a Unicode value and no Modifiers were pressed, add that character itself to the string to be committed. Then commit the string in one action and use return TRUE; which means to not pass any other action except the commit to Mutter, if there is only one action Mutter cannot mess the order up.

I am experimenting with this ... ⏳

mike-fabian commented 11 months ago

https://copr.fedorainfracloud.org/coprs/mfabian/ibus-m17n/ has now ibus-m17n-1.4.26 with an improved workaround which should work for _ and I hope all other characters which trigger a commit and should then be inserted themselves.

Please test this version. Thank you very much for testing so thoroughly!

CodyShiro commented 11 months ago

https://copr.fedorainfracloud.org/coprs/mfabian/ibus-m17n/ has now ibus-m17n-1.4.26 with an improved workaround which should work for _ and I hope all other characters which trigger a commit and should then be inserted themselves.

Please test this version. Thank you very much for testing so thoroughly!

After some testing I found out that when pressing enter to send a text message (mostly in https://chat.zalo.me/) ibus will add another space in befor a second 'enter' press that it will send the message. Right now I'm not able to repoduce it every time but it happens every so often. So I'll send a screen recording once I got it nailed down later.

mike-fabian commented 11 months ago

https://copr.fedorainfracloud.org/coprs/mfabian/ibus-m17n/ has now ibus-m17n-1.4.26 with an improved workaround which should work for _ and I hope all other characters which trigger a commit and should then be inserted themselves. Please test this version. Thank you very much for testing so thoroughly!

After some testing I found out that when pressing enter to send a text message (mostly in https://chat.zalo.me/) ibus will add another space in befor a second 'enter' press that it will send the message. Right now I'm not able to repoduce it every time but it happens every so often. So I'll send a screen recording once I got it nailed down later.

Is that a new problem or did this already occur with older versions of ibus-m17n?

CodyShiro commented 11 months ago

https://copr.fedorainfracloud.org/coprs/mfabian/ibus-m17n/ has now ibus-m17n-1.4.26 with an improved workaround which should work for _ and I hope all other characters which trigger a commit and should then be inserted themselves. Please test this version. Thank you very much for testing so thoroughly!

After some testing I found out that when pressing enter to send a text message (mostly in https://chat.zalo.me/) ibus will add another space in befor a second 'enter' press that it will send the message. Right now I'm not able to repoduce it every time but it happens every so often. So I'll send a screen recording once I got it nailed down later.

Is that a new problem or did this already occur with older versions of ibus-m17n?

It occured on ibus-m17n-1.4.26 after I updated ibus

mike-fabian commented 11 months ago

After some testing I found out that when pressing enter to send a text message (mostly in https://chat.zalo.me/) ibus will add another space in befor a second 'enter' press that it will send the message. Right now I'm not able to repoduce it every time but it happens every so often. So I'll send a screen recording once I got it nailed down later.

I can actually reproduce that in https://chat.zalo.me/

When I type os Return the result is ó. And the message is not send. Typing another Return sends it.

Workaround: Type Control+Return to send the message immediately.

I understand why this happens, I’ll try to explain later.

CodyShiro commented 11 months ago

After some testing I found out that when pressing enter to send a text message (mostly in https://chat.zalo.me/) ibus will add another space in befor a second 'enter' press that it will send the message. Right now I'm not able to repoduce it every time but it happens every so often. So I'll send a screen recording once I got it nailed down later.

I can actually reproduce that in https://chat.zalo.me/

When I type os Return the result is ó. And the message is not send. Typing another Return sends it.

Workaround: Type Control+Return to send the message immediately.

I understand why this happens, I’ll try to explain later.

Thanks, it seems to be working now but I've found Ibus is messing up in LibreOffice: Here, I'm typing chawm the result is chăm but it seems to always add a enter mark behind it. I found this can be aliviated by typing a space inbetween the word you are looking up and pressing enter.

ibus.webm

mike-fabian commented 11 months ago

Thanks, it seems to be working now

I can reproduce this 100%.

but I've found Ibus is messing up in LibreOffice: Here, I'm typing chawm the result is chăm but it seems to always add a enter mark behind it. I found this can be aliviated by typing a space inbetween the word you are looking up and pressing enter.

Both the problem in https://chat.zalo.me/ and in the search entry in LibreOffice have the same cause.

If you type a (Vietnamese) word and then a space, the space triggers the "commit" of the word. "commit" means that the word is sort of "pasted" into the application now, before it was still in "preedit". If you don't understand what preedit and commit means, watch the underlining you see sometimes while typing. The underlined part is in preedit, i.e. not really in the application yet but controlled by ibus. If it is committed, ibus sort of "pastes" it into the application.

Usually (before my attempt to improve the problem you reported here), the space which triggered the commit is then "forwarded" to the application (using either a special function forward_key_event() or doing a return FALSE; from the function in ibus-m17n processing key events. return FALSE; means that this event (the space) has not been handled yet and the application receives it.

During processing a key event, an input engine like ibus-m17n could use commit and forward_key_event() several times and in the end do return TRUE; if the key event was fully handled already or return FALSE; if the application still should receive that key event.

Now when you are using Gnome Wayland where Mutter is involved, Carlos Garnacho explained to me that Mutter does currently not know when the processing of a key event has started and when it ends. So Mutter receives a stream of commit and forward_key_event() (possibly many of them) and in the end a return TRUE; or return FALSE; where the latter is basically the same as a final forward_key_event(). I still don’t understand why Mutter can mess up the order, but apparently if ibus-m17n does something like

Mutter may get them in a different order and that causes the problem that after typing os + space you may get ó with the space before the ó.

The same problem can happen with Return, i.e. if you type os + Return Mutter may handle the two events in the wrong order and the result is a newline before the ó instead of after where it should be.

Putting a sleep of 0.1 seconds in between:

makes it work because Mutter tries to "guess" which events come from the same key event by checking whether there is a longer pause. I.e. with the sleep of 0.1 seconds, after the commit Mutter thinks all events for this key press are done and does not wait any more and does the commit. Then the return FALSE; comes and as far as Mutter is concerned, this is the next key event already and Mutter correctly inserts the space or return after the commit.

But inserting such long 0.1 second sleeps (shorter sleeps don't work reliably) will cause noticable delays when typing fast. So this is no solution.

Carlos Garnacho suggested I should try to do only one single event for each key press. So when typing a key which causes a commit (For example when typing a space or a return or a arrow right or something like .;:_^ I can try whether I can append the key which caused the commit to the text to be committed and then commit the extended text as a single action. As it is a single action, there is no possibility that the order can be reversed.

For keys like space or _().;:^, I can easily add them to the text to be committed, I can do something like ó + _ = ó_ and then commit the ó_. This works fine for all characters which

But there are keys where this is not possible. For example if the key which caused the commit was the Right key (arrow-right). There is no way to add an arrow key to a text. Also if some modifier was pressed, if the key which caused the commit was Control+space, I could add only the space to the commit and this would lose information, the Control would be lost. This would make some shortcut keys not work anymore.

Return is quite special, Return sort of has a Unicode text value, "\n". So if os+Return was typed, I can try to commit ó\n. This works in some cases but not all.

It does not work in all cases because inserting the text ó\n into an application is not the same as inserting ó and then receiving a Return event. For example in https://chat.zalo.me/ , the key event Return causes the message to be sent, inserting a text which has a new line at the end like ó\n does not do that, it just inserts the text. On top of that, https://chat.zalo.me/ converts the new line into a space. The reaction of applications to inserting a text with a trailing newline differs:

So as you correctly observed, if you commit using space in the search entry of libreoffice, the Vietnamese text and the trailing space are correctly inserted and if you then type Return then that Return is passed through as an event (because there is no text to be committed, everything has been committed already because of the space) and thus the search is started.

So maybe I should take out the part of my workaround where I commit the Return together with the text as this causes other problems.

Of course that brings back the old problem that typing os + Return sometimes gives a newline first followed by ó. We can only choose the lesser evil here. What do you think is the lesser evil?

In the distant future, things could be improved if Mutter and Ibus are improved. Carlos Garnacho says that he would like to get information from IBus when processing of a key starts and when it is finished:

and Mutter must execute all these events in the right order.

ibus-m17n unfortunately cannot fix it.

So at the moment I think I probably should do

But as the insertion of a new line at the wrong position is really bad, maybe I should try this:

One sleep of 0.1 seconds once per line is probably acceptable.

Sorry for the long explanation, I hope it helps to understand what is going on and what the options are.

mike-fabian commented 11 months ago

https://copr.fedorainfracloud.org/coprs/mfabian/ibus-m17n/ has now ibus-m17n-1.4.27

I improved the workaround to append to the commit only for keys where there are no known disadvantages for doing so (space, .;:_^)( and all such characters) but not for Return and KP_Enter. For Return and KP_Enter and keys where modifiers (Control, Alt, ...) were pressed, I use a sleep of 0.1 seconds instead before a return FALSE; to pass the key through.

Seems like an improvement to me, the problems you reported when typing Return in https://chat.zalo.me/ and in the search entry in Libreoffice are "fixed" with this hack.

mike-fabian commented 11 months ago

Does everything work for you now?

CodyShiro commented 11 months ago

Does everything work for you now?

I've been a little caught up with school lately so I haven't got the time to thoroughly but after a sudo dnf update and a few hours of use, it seems to be working flawlessly, especially in https://chat.zalo.me/ where most of my work is situated.

Thank you very much!

mike-fabian commented 11 months ago

Thank you very much for testing!

By the way, it seems to be that there is room for improvement in vi-telex.mim, see:

https://github.com/ibus/ibus/issues/2445#issuecomment-1244243318

ibus-m17n (or ibus-typing-booster) with vi-telex.mim and ibus-unikey can do this:

chaof ➡️ chào moij ➡️ mọi

correctly, but only ibus-unikey gets

nguoiwf ➡️ người

correct, ibus-m17n and ibus-typing-booster do

nguoiwf ➡️ nguoìw

That does not seem optiomal, so probably vi-telex.mim should be improved.

As you are a native speaker, maybe you could help to figure out what exactly needs to be improved?

mike-fabian commented 11 months ago

I released ibus-m17n-1.4.27 with the fix for this issue: https://github.com/ibus/ibus-m17n/releases/tag/1.4.27

nirmalpathak commented 11 months ago

Thanks for the fix. It works!