[PATCH v4 2/3] winex11: Clear only non-empty existing Composition String.

[Byeongsik Jeon]

On 4/11/22 5:50 PM, Alex Kwak wrote:
> Hi, Jeon
> 
> ( from: https://source.winehq.org/patches/data/231475 )
> 
> Sorry for late reply. I didn't see my email.
> 
> I'm so happy to have someone interested in this issue.
> 
> 
> But, one thing i don't understand that why must signaling
> WM_IME_ENDCOMPOSITION.
> 
> It will handled from preedit callbacks.
> 
> Doesn't the WM_IME_ENDCOMPOSITION signal occur twice?
> 
Hi,

Actually, I don't know why.


### xim server sended event sequence ###
preedit start::
preedit draw:: caret 1 chg_first  0 chg_length  0 string 'ㄱ'
preedit draw:: caret 1 chg_first  0 chg_length  1 string '가'
preedit draw:: caret 1 chg_first  0 chg_length  1 string '간'
preedit draw:: caret 0 chg_first  0 chg_length  1 NULL
preedit done::
lookup chars::  keycode    0 string  3 '가'                   <== HERE 1
preedit start::                                               <== HERE 2
preedit draw:: caret 1 chg_first  0 chg_length  0 string '나' <== HERE 3


I'm not sure, but if I guess. By "[PATCH v4 1/3]" issue, the sequence of
events changes as follows:

### current Wine process_events() simulated event sequence ###
preedit start::
preedit draw:: caret 1 chg_first  0 chg_length  0 string 'ㄱ'
preedit draw:: caret 1 chg_first  0 chg_length  1 string '가'
preedit draw:: caret 1 chg_first  0 chg_length  1 string '간'
preedit draw:: caret 0 chg_first  0 chg_length  1 NULL
preedit done::
preedit start::                                               <== HERE 4
preedit draw:: caret 1 chg_first  0 chg_length  0 string '나' <== HERE 5
lookup chars::  keycode    0 string  3 '가'                   <== HERE 6

IME_SetResultString() is in "HERE 6".

At this time, it is "inComp = TRUE" and WM_IME_STARTCOMPOSITION
occurred. it may be thought that WM_IME_ENDCOMPOSITION is necessary.

When it and "if (!inComp) GenerateIMEMessage(imc, WM_IME_ENDCOMPOSITION,
0, 0);" combine, WM_IME_ENDCOMPOSITION come out of 'if'.

It's just guess.


> 
> Regards.,
> 
> Alex
> 
> 
> On 22. 4. 11. 16:57, Byeongsik Jeon wrote:
>> On 4/10/22 8:52 PM, Akihiro Sagawa wrote:
>>> On Fri,  8 Apr 2022 21:45:40 +0900, Byeongsik Jeon wrote:
>>> [...]
>>>> @@ -1040,7 +1046,7 @@ void IME_SetResultString(LPWSTR lpResult,
>>>> DWORD dwResultLen)
>>>>       inComp = myPrivate->bInComposition;
>>>>       ImmUnlockIMCC(lpIMC->hPrivate);
>>>>   -    if (!inComp)
>>>> +    if (!inComp && compstr_len)
>>>>       {
>>>>           ImmSetOpenStatus(imc, TRUE);
>>>>           GenerateIMEMessage(imc, WM_IME_STARTCOMPOSITION, 0, 0);
>>>> @@ -1050,7 +1056,7 @@ void IME_SetResultString(LPWSTR lpResult,
>>>> DWORD dwResultLen)
>>>>       GenerateIMEMessage(imc, WM_IME_COMPOSITION, lpResult[0],
>>>> GCS_RESULTSTR|GCS_RESULTCLAUSE);
>>>>       GenerateIMEMessage(imc, WM_IME_ENDCOMPOSITION, 0, 0);
>>>>   -    if (!inComp)
>>>> +    if (!inComp && compstr_len)
>>>>           ImmSetOpenStatus(imc, FALSE);
>>>>         ImmUnlockIMC(imc);
>>> Hi,
>>>  From my point of view, the purpose of these !inComp blocks is
>>> to synchronize the status when root window style is used, i.e. no
>>> preedit callbacks happen. Moreover, the composition string length is
>>> mostly zero unless XIMPreEditDrawCallback is called.
>>> I'd suggest you to omit status updates if the preedit callback is used
>>> before.
>>>
>> Hi,
>> Thank you for review :-)
>>
>> The patch was defensively written because the e4d94e48138f situation was
>> not known exactly. I will try to improve the patch by referring to this
>> review.
>>
>> I found something strange during this issue test. This is probably
>> related to the issue that Alex Kwak found. It's going to take some time
>> to analyze.
>>
>> https://www.winehq.org/pipermail/wine-devel/2022-April/212598.html
>>
>>> I leave the review of other two patches to the maintainers because I'm
>>> not familiar with X event handlers.
>>>
>> I don't know if there's any way to write 'Wine test code' for this
>> issue. Instead, I wrote a small "xim event print" program.
>>
>> $ XMODIFIERS=@im=ibus ./xim-test
>> $ XMODIFIERS=@im=ibus ./xim-test --wine
>>
>> If events are generated in the sequence "result_string >>
>> composition_string" by one key input, the sequence will change. My
>> Japanese test sample is "日本の" and the key sequence is
>> "nihon<SPACE>no".
>>
>> It happens very often in the Korean "du-bul-sik" keymap automata, and
>> "du-bul-sik" is the dominant automata.
>>
>> Because composition_string occurs in XFilterEvent() and result_string
>> occurs in XKeyEvent handler, the sequence will change in the current
>> Wine merge_events() implementation.
>>
>>
>>> Akihiro Sagawa
>>>