1742039 - [GTK][Fcitx][Fcitx5] If preedit is disabled, first type for second or later word is discarded

Status: VERIFIED FIXED

(Core :: DOM: UI Events & Focus Handling, defect, P2)

Description

[lilydjwg]

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:96.0) Gecko/20100101 Firefox/96.0

Steps to reproduce:

1. find some place to type text (e.g. the urlbar or here)
2. activate fcitx5
3. switch to the Wubi (五笔) input method
4. press Ctrl-Alt-P once to disable preedit (by default it is enabled)
5. press these keys "wqvbkb" and a space

Actual results:

"你好了" is committed

Expected results:

"你好啊" should be committed.

Code for "你好" is "wqvb". Code for "了" is "b". Code for "啊" is "kb".

In Wubi, after a complete four-key code, on next keystroke the first candidate should be committed and the next keystroke is the first key of the next code. Firefox loses this key.

mozregression result:

18:33.71 INFO: Last good revision: d97093bef6913a2771713e2db2d80bb0b1d69627
18:33.71 INFO: First bad revision: 19d5d6282da194b61704a5edd89f3d5613113512
18:33.71 INFO: Pushlog:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=d97093bef6913a2771713e2db2d80bb0b1d69627&tochange=19d5d6282da194b61704a5edd89f3d5613113512

This has been reported to a Chinese forum here: https://forum.suse.org.cn/t/topic/14465

I tested with fcitx5. The original poster tested with fcitx4.

Comment 1

[BugBot [:suhaib / :marco/ :calixte]]

The Bugbug bot thinks this bug should belong to the 'Firefox::Address Bar' component, and is moving the bug to that component. Please revert this change in case you think the bot is wrong.

Comment 2

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1712269

Comment 3

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

I'm creating the environment, but sill not finished yet.

If somebody puts the following logs, I could fix this faster.

• Which events are fired in https://w3c.github.io/uievents/tools/key-event-viewer.html (Perhaps, screenshot is better because pasted table data is hard to read)
• Attach a log file of the parent process with launching with MOZ_LOG=nsGtkIMModuleWidgets:5,sync and MOZ_LOG_FILE setting path to the log file (Don't type anything your privacy like passwords while you're launching Firefox with these env variables).

Comment 4

[lilydjwg]

Attachment: requested screenshot

Comment 5

[lilydjwg]

Attachment: fxim.log.moz_log

I was using the exact step given in comment 0 while generating this log and screenshot. In step 2 "activate fcitx5" the key to do so is Ctrl+space.

Comment 6

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Thank you very much! I'll check what's going on.

Comment 7

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

According to the log, k key press is notified to IME, then, 你好 is comitted asynchronously. At this time, OnSelectionChange is called and it causes resetting IMContext, then, we receive b key press. Perhaps, at this time, IME forgets the preceding k key press due to the reset.

I think that we should avoid resetting IMContext in this case. On the other hand, I do not understand why the change of bug 1712269 caused this regression yet, but I guess making it in composing state prevents the resetting. I'll post a patch.

Comment 8

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

I still cannot create an environment to test this. Could you try to check whether an experimental build fixes this bug?
https://firefox-ci-tc.services.mozilla.com/api/queue/v1/task/J59seROzTgKQHJi4Sqyzog/runs/0/artifacts/public/build/target.tar.bz2

Comment 9

[lilydjwg]

No, that build still has the same issue.

Comment 10

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Thanks. I'll be back to the log file for understanding what's going on...

Comment 11

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Okay, I succeeded crating a test environment, and indeed, my experimental build does not work.

Comment 12

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

It seems that this build works, could you try this?
https://firefox-ci-tc.services.mozilla.com/api/queue/v1/task/WzrCvWNBQ7CFg7RlGYF8hQ/runs/0/artifacts/public/build/target.tar.bz2

Comment 13

[lilydjwg]

Yes, this build fixes the issue!

Comment 14

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Thank you!

Comment 15

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Bug 1742039 - Make `IMContextWrapper::OnSelectionChange()` not call `ResetIME()` if selection range is expected one r=m_kato!

Some IME of fcitx and fcitx5 have a mode to disable preedit. This works
similar to dead key typing on Windows. I.e., there is an "invisible"
composing text, and only commit string is inserted directly.

After bug 1712269, we don't use a set of composition events for direct
commit from IME because of web-compat. Therefore, IMEContentObserver
cannot know whether the insert is caused by composition or not in this
case.

Additionally, the commit of the IME may be triggered by a key press
for next word. Therefore, we shouldn't call ResetIME() from
OnSelectionChange() in this case.

Fortunately, we've already have selection cache which is expected by
IME after dispatching a commit event, and if the actual selection is
same as expected one, IME shouldn't require explicit reset. Therefore,
this patch makes OnSelectionChange() check whether selection is
actually changed, and stop calling ResetIME() if selection is not
changed.

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/25050ce9d63f
Make `IMContextWrapper::OnSelectionChange()` not call `ResetIME()` if selection range is expected one r=m_kato

Comment 17

[Cristina Cozmuta (:CrissCozmuta)]

https://hg.mozilla.org/mozilla-central/rev/25050ce9d63f

Comment 18

[Anca Soncutean, Desktop QA]

I have some problems with setting the environment on my Ubuntu machine. Lilydjwg, could you help us confirm the fix with the latest 96 beta build (download link)? Thank you!

Comment 19

[lilydjwg]

Yes. With 96.0b10 zh-CN variant I confirm that this issue has been fixed!

Comment 20

[Anca Soncutean, Desktop QA]

Thank you Lilydjwg for your prompt reply!
As per comment 19 I will mark this issue as verified fixed.