[PATCH 0/1] MR7766: win32u: Preserve result string from multiple WINE_IME_POST_UPDATE call during ImeProcessKey.
In winemac.drv, Multiple ime update calls occur during ImeProcessKey. These multiple ime update calls need to be properly merged into ‘data->update’. However, currently win32u/imm.c::post_ime_update() only keeps the last ime update call. Valid rules are: - The comp_str is discarded when the next ime update call occurs, so it is only valid when the last ime update call has the comp_str. - The result_str is retained even if the next ime update call occurs. - If the next ime update call has the result_str, it is appended after the previous result_str. (e.g. 'r-k-1' or 'r-k-<SPACE>') Test key sequences are: - "Japanese Romanji" : 'nihongo-<SPACE>-n' - "Korean 2-Set Keyboard" : 'r-k-s-k' - "Korean 2-Set Keyboard" : 'r-k-1" or 'r-k-<SPACE>' -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7766
From: Byeongsik Jeon <bsjeon(a)hanmail.net> In winemac.drv, Multiple ime update calls occur during ImeProcessKey. These multiple ime update calls need to be properly merged into ‘data->update’. However, currently win32u/imm.c::post_ime_update() only keeps the last ime update call. Valid rules are: - The comp_str is discarded when the next ime update call occurs, so it is only valid when the last ime update call has the comp_str. - The result_str is retained even if the next ime update call occurs. - If the next ime update call has the result_str, it is appended after the previous result_str. (e.g. 'r-k-1' or 'r-k-<SPACE>') Test key sequences are: - "Japanese Romanji" : 'nihongo-<SPACE>-n' - "Korean 2-Set Keyboard" : 'r-k-s-k' - "Korean 2-Set Keyboard" : 'r-k-1" or 'r-k-<SPACE>' --- dlls/win32u/imm.c | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/dlls/win32u/imm.c b/dlls/win32u/imm.c index 99dc3e5e225..f673693f6a0 100644 --- a/dlls/win32u/imm.c +++ b/dlls/win32u/imm.c @@ -442,6 +442,8 @@ static void post_ime_update( HWND hwnd, UINT cursor_pos, WCHAR *comp_str, WCHAR struct imm_thread_data *data = get_imm_thread_data(); UINT id = -1, comp_len, result_len; struct ime_update *update; + WCHAR *prev_result_str = NULL; + UINT prev_result_len = 0; TRACE( "hwnd %p, cursor_pos %u, comp_str %s, result_str %s\n", hwnd, cursor_pos, debugstr_w(comp_str), debugstr_w(result_str) ); @@ -449,10 +451,23 @@ static void post_ime_update( HWND hwnd, UINT cursor_pos, WCHAR *comp_str, WCHAR comp_len = comp_str ? wcslen( comp_str ) + 1 : 0; result_len = result_str ? wcslen( result_str ) + 1 : 0; - if (!(update = malloc( offsetof(struct ime_update, buffer[comp_len + result_len]) ))) return; + if (data->ime_process_vkey && data->update && data->update->result_str) + { + prev_result_str = data->update->result_str; + prev_result_len = result_len ? wcslen( prev_result_str ) + : prev_result_str ? wcslen( prev_result_str ) + 1 : 0; + } + + if (!(update = malloc( offsetof(struct ime_update, + buffer[comp_len + prev_result_len + result_len]) ))) return; update->cursor_pos = cursor_pos; update->comp_str = comp_str ? memcpy( update->buffer, comp_str, comp_len * sizeof(WCHAR) ) : NULL; - update->result_str = result_str ? memcpy( update->buffer + comp_len, result_str, result_len * sizeof(WCHAR) ) : NULL; + update->result_str = result_str ? memcpy( update->buffer + comp_len + prev_result_len, + result_str, result_len * sizeof(WCHAR) ) + : NULL; + if (prev_result_len) + update->result_str = memcpy( update->buffer + comp_len, + prev_result_str, prev_result_len * sizeof(WCHAR) ); if (!(update->vkey = data->ime_process_vkey)) { -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7766
Rémi Bernon (@rbernon) commented about dlls/win32u/imm.c:
update->scan = data->ime_process_scan; free( data->update ); data->update = update; }
I think this loses the previous result if it was != NULL but the new update has result_str == NULL. What about: ```suggestion:-45+0 struct imm_thread_data *data = get_imm_thread_data(); UINT id = -1, comp_len, result_len, prev_result_len; WCHAR *prev_result_str, *tmp; struct ime_update *update; TRACE( "hwnd %p, cursor_pos %u, comp_str %s, result_str %s\n", hwnd, cursor_pos, debugstr_w(comp_str), debugstr_w(result_str) ); comp_len = comp_str ? wcslen( comp_str ) + 1 : 0; result_len = result_str ? wcslen( result_str ) + 1 : 0; /* prepend or keep the previous result string, if there was any */ if (!data->ime_process_vkey || !data->update) prev_result_str = NULL; else prev_result_str = data->update->result_str; prev_result_len = prev_result_str ? wcslen( prev_result_str ) + 1 : 0; if (!prev_result_len && !result_len) tmp = NULL; else if (!(tmp = malloc( (prev_result_len + result_len) * sizeof(WCHAR) ))) return; if (prev_result_len && result_len) prev_result_len -= 1; /* concat both strings */ if (prev_result_len) memcpy( tmp, prev_result_str, prev_result_len * sizeof(WCHAR) ); if (result_len) memcpy( tmp + prev_result_len, result_str, result_len * sizeof(WCHAR) ); result_len += prev_result_len; result_str = tmp; if (!(update = malloc( offsetof(struct ime_update, buffer[comp_len + result_len]) ))) return; update->cursor_pos = cursor_pos; update->comp_str = comp_str ? memcpy( update->buffer, comp_str, comp_len * sizeof(WCHAR) ) : NULL; update->result_str = result_str ? memcpy( update->buffer + comp_len, result_str, result_len * sizeof(WCHAR) ) : NULL; if (!(update->vkey = data->ime_process_vkey)) { pthread_mutex_lock( &imm_mutex ); id = update->scan = ++ime_update_count; update->vkey = VK_PROCESSKEY; list_add_tail( &ime_updates, &update->entry ); pthread_mutex_unlock( &imm_mutex ); NtUserPostMessage( hwnd, WM_WINE_IME_NOTIFY, IMN_WINE_SET_COMP_STRING, id ); } else { update->scan = data->ime_process_scan; free( data->update ); data->update = update; } free( result_str ); ``` -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7766#note_100259
participants (3)
-
Byeongsik Jeon -
Byeongsik Jeon (@bsjeon) -
Rémi Bernon