Fix CV sequences gi and qu incorrectly handled

[Nekosha]

Same reason as https://github.com/fcitx/fcitx-unikey/pull/25 Summary: The CV sequence should be treated as an unit. Early commiting g ignores this, causing subsequent grammar checks to fail / tone marks inserted on the wrong character.

[Nekosha]

Q and q should be removed from that list too. but I forgot to commit the change to the branch.

[wengxt]

Hi, would you mind to give a test case for this?

Like, what's expected behavior and what's old behavior?

[Nekosha]

In Firefox 90.0.2 (latest)

1. gias
   • Expected giá
   • Got is gía, the diacritic is on the wrong character
2. gians
   • Expected gián
   • Got is gians, fails to insert the diacritic.
3. quer
   • Expected quẻ
   • Got qủe, the diacritic is on the wrong character.

This is a Firefox-related issue, and enabling macro fixes this as said in the original issue report. However it is still worth fixing, for example if we don't want to enable macro. This issue was present in older versions of Discord, however it seems to have been resolved. This fix can be reverted if Firefox changes their text handling.

The code path that causes this is line 332-344 in src/unikey-im.cpp

// auto commit word that never need to change later in preedit string
        // (like consonant - phu am) if macro enabled, then not auto commit.
        // Because macro may change any word
        if (!*engine_->config().macro &&
            (uic_.isAtWordBeginning() || autoCommit_)) {
            for (auto wordAutoCommit : WordAutoCommit) {
                if (sym == wordAutoCommit) {
                    uic_.putChar(sym);
                    autoCommit_ = true;
                    return;
                }
            }
        } // end auto commit

[wengxt]

First thank you for your PR, but I think we could use a different way to work around it.

I tried to compare m17n and unikey's behavior. M17n use commit string to produce "g", while unikey pass through the key "g" to produce "g". So I think we could do something similar here. This matches the firefox current behavior ("Reset" when key event produce string).

[wengxt]

emm, my guess seems to be wrong, actually m17n is correct because it's using the surrounding text...

[trmdi]

What is the purpose of auto committing word? Can we simply remove that feature? I mean this block: https://github.com/fcitx/fcitx5-unikey/blob/b833a682ef79eff5bc09e42f6929026b2cecca72/src/unikey-im.cpp#L332

What happens if we don't auto commit it when not using macro?

[Nekosha]

Removing it would not affect functionality, since enabling macros also disable it as well. This is more or less a hack to fix this specific issue, and a proper fix may have to change other things.

[trmdi]

Removing it would not affect functionality, since enabling macros also disable it as well. This is more or less a hack to fix this specific issue, and a proper fix may have to change other things.

But I don't see the purpose of word-auto-commit? Can you give me an example?

[wengxt]

That really depends on whether user like preedit or not.

Sometimes preedit may not work well with auto completion (popup stealing focus or etc), while auto commit works well.

Certain application doesn't work with client preedit at all, e.g. google docs. That's all from a IM developer POV because I'm not a daily user.

Do you know what is it like on other platforms e.g. windows?

[trmdi]

That really depends on whether user like preedit or not.

Sometimes preedit may not work well with auto completion (popup stealing focus or etc), while auto commit works well.

Certain application doesn't work with client preedit at all, e.g. google docs. That's all from a IM developer POV because I'm not a daily user.

ah I got it, thank you for your explanation.

Do you know what is it like on other platforms e.g. windows?

I've just tested with Microsoft's Keyboard on Android, typing on the browser's address bar gives you suggestion even when you're still in the preedit state,

[wengxt]

So after all, I don't think this is the right way do fix it. And since we now enable spell check by default, it shouldn't be a problem in default state. So I'd close this PR for now.

There's some potential that we can fix this via surrounding text.