[PATCH] winex11.drv: Fix IME_SetResultString for Korean(hangul) IME.

[Alex Kwak]

> I'm not sure about this change.
> It always turns off IME open status. Since composition might be
> continuous at this moment, it can't be set unconditionally.
> If we should do so for some reasons, could you explain in detail?
If WM_IME_ENDCOMPOSITION comes, I think WM_IME_STARTCOMPOSITION
should come next time.
  but it was not.
If continue to expect composition, I don't think WM_IME_ENDCOMPOSITION
should not come.

>  From my point of view, the main cause of this issue is that the order of
> PreeditCallbacks and XmbLookupString() aren't stable. The current wine's
> implementation seems to rely on getting the result of XmbLookupString()
> before PreeditCallback for the next clause. However, when one or more
> clauses are left after commiting a clause, PreeditCallback for the left
> clauses sometimes (or always in Hangul?) called before getting the
> result of XmbLookupString.
> What do you think about these XIM situations? Does your patch works
> well for both cases?
I agree with unstable on CJK inputs.

Unlike Windows,
it is implemented without considering what language you are typing and what
characteristics each language is input to, so I thought it would not be
possible to apply it to all CJK collectively. ( also, XIM doesn't support )

The same situation as the case you mentioned cannot happen in Hangul.
because in Hangul, when a word is completed, it must be one clause or the
composition must be exited with non-composition character(or event).

so, I'm worried about causing problems in Chinese and Japanese.

22. 3. 26. 19:44에 Akihiro Sagawa 이(가) 쓴 글:
> On Sun, 20 Mar 2022 21:17:27 +0900, Alex Kwak wrote:
>> In a special case for Hangul(Korean), when Composition is completed and
>> this function is called, the new Composition disappears. so, calling
>> IME_SetCompositionString again for makes composition will be
>> starting.
>>
> Please add Wine-Bug header before Signed-off-by line.
>> Signed-off-by: Alex Kwak <take-me-home at kakao.com>
>> ---
>>   dlls/winex11.drv/ime.c    |  4 ++--
>>   dlls/winex11.drv/x11drv.h | 27 +++++++++++++++++++++++++++
>>   dlls/winex11.drv/xim.c    | 13 +++++++++++++
>>   3 files changed, 42 insertions(+), 2 deletions(-)
>>
>> diff --git a/dlls/winex11.drv/ime.c b/dlls/winex11.drv/ime.c
>> index c1584930861..f7827b31635 100644
>> --- a/dlls/winex11.drv/ime.c
>> +++ b/dlls/winex11.drv/ime.c
>> @@ -1050,8 +1050,8 @@ 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)
>> -        ImmSetOpenStatus(imc, FALSE);
>> +    myPrivate->bInComposition = FALSE;
>> +    ImmSetOpenStatus(imc, FALSE);
>>   
>>       ImmUnlockIMC(imc);
>>   }
> I'm not sure about this change.
> It always turns off IME open status. Since composition might be
> continuous at this moment, it can't be set unconditionally.
> If we should do so for some reasons, could you explain in detail?
>
>> diff --git a/dlls/winex11.drv/x11drv.h b/dlls/winex11.drv/x11drv.h
>> index f389f3e0836..8ce3a7e8528 100644
>> --- a/dlls/winex11.drv/x11drv.h
>> +++ b/dlls/winex11.drv/x11drv.h
> [..]
>> --- a/dlls/winex11.drv/xim.c
>> +++ b/dlls/winex11.drv/xim.c
>> @@ -117,6 +117,19 @@ void X11DRV_XIMLookupChars( const char *str, DWORD count )
>>   
>>       IME_SetResultString(wcOutput, dwOutput);
>>       HeapFree(GetProcessHeap(), 0, wcOutput);
>> +
>> +    /*
>> +     * In a special case for Hangul(Korean), when Composition is completed and
>> +     * this function is called, the new Composition disappears. so, calling
>> +     * IME_SetCompositionString again for makes composition will be
>> +     * starting.
>> +     */
>> +    if (CompositionString && dwCompStringLength >= sizeof(WCHAR) &&
>> +        IsHangul(((const WCHAR*) CompositionString)[0]))
>> +    {
>> +        IME_SetCompositionString(SCS_SETSTR, CompositionString,
>> +                                 dwCompStringLength, NULL, 0);
>> +    }
>>   }
>>   
>>   static BOOL XIMPreEditStateNotifyCallback(XIC xic, XPointer p, XPointer data)
> Could you revert Hangul(Korean) specific changes? This issue seems not
> to be Korean specific because I can occasionally reproduce it with
> Japanese input method.
>
>  From my point of view, the main cause of this issue is that the order of
> PreeditCallbacks and XmbLookupString() aren't stable. The current wine's
> implementation seems to rely on getting the result of XmbLookupString()
> before PreeditCallback for the next clause. However, when one or more
> clauses are left after commiting a clause, PreeditCallback for the left
> clauses sometimes (or always in Hangul?) called before getting the
> result of XmbLookupString.
> What do you think about these XIM situations? Does your patch works
> well for both cases?
>
> Regards,
> Akihiro Sagawa