[PATCH v3] winex11.drv: call XIMReset on CPS_COMPLETE

[Alex Kwak]

Hi,

Thanks for your reviewing. great helpful to me.


I submit a new patch reflecting feedback.

https://www.winehq.org/pipermail/wine-devel/2022-April/212598.html


Regards.,

Alex

22. 4. 2. 22:57에 Akihiro Sagawa 이(가) 쓴 글:
> On Thu, 31 Mar 2022 14:30:23 +0900, Alex Kwak wrote:
>> The composition on the IME message is end, but the XIM string is
>> in-composition state, and the same string remains on the XIM
>> afterwards.
>> so, call X11DRV_ForceXIMReset and finish composition state in the XIM.
>>
>> Wine-Bug:https://bugs.winehq.org/show_bug.cgi?id=52754
>> Signed-off-by: Alex Kwak<take-me-home at kakao.com>
>> ---
>> v2: When XmbResetIC is called and KeyEvent occurs, it is a problem with
>> some IME (i.e., ibus-hangul).
>> v3: implement X11DRV_FindXIC(..) for getting exists XIC.
>> ---
>>   dlls/winex11.drv/ime.c    |  2 ++
>>   dlls/winex11.drv/x11drv.h |  1 +
>>   dlls/winex11.drv/xim.c    | 31 +++++++++++++++++++++++++++++--
>>   3 files changed, 32 insertions(+), 2 deletions(-)
>>
>> diff --git a/dlls/winex11.drv/ime.c b/dlls/winex11.drv/ime.c
>> index c1584930861..2fe78234671 100644
>> --- a/dlls/winex11.drv/ime.c
>> +++ b/dlls/winex11.drv/ime.c
>> @@ -753,6 +753,8 @@ BOOL WINAPI NotifyIME(HIMC hIMC, DWORD dwAction, DWORD dwIndex, DWORD dwValue)
>>                       myPrivate->bInComposition = FALSE;
>>                       ImmUnlockIMCC(lpIMC->hPrivate);
>>   
>> +                    X11DRV_ForceXIMReset(lpIMC->hWnd);
>> +
> I understood the basic concept of this patch, but...
>
>>                       bRet = TRUE;
>>                   }
>>                   break;
>> diff --git a/dlls/winex11.drv/x11drv.h b/dlls/winex11.drv/x11drv.h
>> index f389f3e0836..de882a532f3 100644
>> --- a/dlls/winex11.drv/x11drv.h
>> +++ b/dlls/winex11.drv/x11drv.h
>> @@ -827,6 +827,7 @@ extern BOOL X11DRV_InitXIM( const char *input_style ) DECLSPEC_HIDDEN;
>>   extern XIC X11DRV_CreateIC(XIM xim, struct x11drv_win_data *data) DECLSPEC_HIDDEN;
>>   extern void X11DRV_SetupXIM(void) DECLSPEC_HIDDEN;
>>   extern void X11DRV_XIMLookupChars( const char *str, DWORD count ) DECLSPEC_HIDDEN;
>> +XIC X11DRV_FindXIC(HWND hwnd) DECLSPEC_HIDDEN;
> You might want to use "extern" for consistency.
>
>>   extern void X11DRV_ForceXIMReset(HWND hwnd) DECLSPEC_HIDDEN;
>>   extern void X11DRV_SetPreeditState(HWND hwnd, BOOL fOpen) DECLSPEC_HIDDEN;
>>   
>> diff --git a/dlls/winex11.drv/xim.c b/dlls/winex11.drv/xim.c
>> index 3994c2106cc..7a4ec432bbf 100644
>> --- a/dlls/winex11.drv/xim.c
>> +++ b/dlls/winex11.drv/xim.c
>> @@ -249,9 +249,36 @@ static void XIMPreEditCaretCallback(XIC ic, XPointer client_data,
>>       TRACE("Finished\n");
>>   }
>>   
>> +/**
>> + * X11DRV_FindXIC
>> + *
>> + * This function finds the already created XIC in itself and parents.
>> + * If only the specified handle is get via X11DRV_get_ic, the XIC can be exists
>> + * in the parents and cannot be get.
>> + */
>> +XIC X11DRV_FindXIC(HWND hwnd)
>> +{
>> +    HWND hwndParent = hwnd;
> You can name the parameter hwndParent.
>
>> +    XIC ic;
>> +    if (!hwndParent)
>> +        return NULL;
>> +    for (;;)
>> +    {
>> +        ic = get_win_data( hwndParent )->xic;
>> +        if (ic)
>> +            break;
>> +        else
>> +            hwndParent = GetParent(hwnd);
>> +
>> +        if (!hwndParent)
>> +            return NULL;
>> +    }
> Because I'm not an expert at Wine's XIC handling, I wonder whether this
> approach is acceptable. Anyway, it's better to rewrite this code snippet
> with while loop.
>
>> +    return ic;
>> +}
>> +
>>   void X11DRV_ForceXIMReset(HWND hwnd)
>>   {
>> -    XIC ic = X11DRV_get_ic(hwnd);
>> +    XIC ic = X11DRV_FindXIC(hwnd);
>>       if (ic)
>>       {
>>           char* leftover;
>> @@ -267,7 +294,7 @@ void X11DRV_SetPreeditState(HWND hwnd, BOOL fOpen)
>>       XIMPreeditState state;
>>       XVaNestedList attr;
>>   
>> -    ic = X11DRV_get_ic(hwnd);
>> +    ic = X11DRV_FindXIC(hwnd);
>>       if (!ic)
>>           return;
>>   
> These code change the preexisting behaviors.
> Would you mind dividing this patch into two or more?
> e.g.
>   - part 1: introduces X11DRV_FindXIC() and updates X11DRV_ForceXIMReset(),
>   - part 2: calls X11DRV_FindXIC() in X11DRV_SetPreeditState() (this part
> might merge into part 1)
>   - part 3: calls X11DRV_ForceXIMRest() in NotifyIME(CPS_COMPLETE).
>
> Akihiro Sagawa
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.winehq.org/pipermail/wine-devel/attachments/20220403/68a67793/attachment.htm>