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

[Alex Kwak]

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?


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
>>