[PATCH v4 01/10] shell32/autocomplete: Fix a vulnerability by avoiding the use of snprintf
The quickComplete format can have more than one % argument, or stuff like %*.* or %1234s, which can be exploited since the format string can be read from the registry, so handle it manually instead of using sprintf. Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- v4: Return the length to not overestimate. dlls/shell32/autocomplete.c | 42 ++++++++++++++++++++++++++++++++++++------ 1 file changed, 36 insertions(+), 6 deletions(-) diff --git a/dlls/shell32/autocomplete.c b/dlls/shell32/autocomplete.c index 4ec7387..587d41f 100644 --- a/dlls/shell32/autocomplete.c +++ b/dlls/shell32/autocomplete.c @@ -93,6 +93,32 @@ static inline IAutoCompleteImpl *impl_from_IAutoCompleteDropDown(IAutoCompleteDr return CONTAINING_RECORD(iface, IAutoCompleteImpl, IAutoCompleteDropDown_iface); } +static size_t format_quick_complete(WCHAR *dst, const WCHAR *qc, const WCHAR *str, size_t str_len) +{ + /* Replace the first %s directly without using snprintf, to avoid + exploits since the format string can be retrieved from the registry */ + WCHAR *base = dst; + UINT args = 0; + while (*qc != '\0') + { + if (qc[0] == '%') + { + if (args < 1 && qc[1] == 's') + { + memcpy(dst, str, str_len * sizeof(WCHAR)); + dst += str_len; + qc += 2; + args++; + continue; + } + qc += (qc[1] == '%'); + } + *dst++ = *qc++; + } + *dst = '\0'; + return dst - base; +} + static void destroy_autocomplete_object(IAutoCompleteImpl *ac) { ac->hwndEdit = NULL; @@ -109,7 +135,6 @@ static LRESULT APIENTRY ACEditSubclassProc(HWND hwnd, UINT uMsg, WPARAM wParam, IAutoCompleteImpl *This = GetPropW(hwnd, autocomplete_propertyW); HRESULT hr; WCHAR hwndText[255]; - WCHAR *hwndQCText; RECT r; BOOL control, filled, displayall = FALSE; int cpt, height, sel; @@ -138,11 +163,16 @@ static LRESULT APIENTRY ACEditSubclassProc(HWND hwnd, UINT uMsg, WPARAM wParam, /* If quickComplete is set and control is pressed, replace the string */ control = GetKeyState(VK_CONTROL) & 0x8000; if (control && This->quickComplete) { - hwndQCText = heap_alloc((lstrlenW(This->quickComplete)+lstrlenW(hwndText))*sizeof(WCHAR)); - sel = sprintfW(hwndQCText, This->quickComplete, hwndText); - SendMessageW(hwnd, WM_SETTEXT, 0, (LPARAM)hwndQCText); - SendMessageW(hwnd, EM_SETSEL, 0, sel); - heap_free(hwndQCText); + WCHAR *buf; + size_t len = strlenW(hwndText); + size_t sz = strlenW(This->quickComplete) + 1 + len; + if ((buf = heap_alloc(sz * sizeof(WCHAR)))) + { + len = format_quick_complete(buf, This->quickComplete, hwndText, len); + SendMessageW(hwnd, WM_SETTEXT, 0, (LPARAM)buf); + SendMessageW(hwnd, EM_SETSEL, 0, len); + heap_free(buf); + } } ShowWindow(This->hwndListBox, SW_HIDE); -- 1.9.1
Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/shell32/autocomplete.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/dlls/shell32/autocomplete.c b/dlls/shell32/autocomplete.c index 587d41f..6a77ddb 100644 --- a/dlls/shell32/autocomplete.c +++ b/dlls/shell32/autocomplete.c @@ -210,7 +210,8 @@ static LRESULT APIENTRY ACEditSubclassProc(HWND hwnd, UINT uMsg, WPARAM wParam, int len; len = SendMessageW(This->hwndListBox, LB_GETTEXTLEN, sel, 0); - msg = heap_alloc((len + 1)*sizeof(WCHAR)); + if (!(msg = heap_alloc((len + 1) * sizeof(WCHAR)))) + return 0; SendMessageW(This->hwndListBox, LB_GETTEXT, sel, (LPARAM)msg); SendMessageW(hwnd, WM_SETTEXT, 0, (LPARAM)msg); SendMessageW(hwnd, EM_SETSEL, lstrlenW(msg), lstrlenW(msg)); @@ -329,7 +330,8 @@ static LRESULT APIENTRY ACLBoxSubclassProc(HWND hwnd, UINT uMsg, WPARAM wParam, if (sel < 0) break; len = SendMessageW(This->hwndListBox, LB_GETTEXTLEN, sel, 0); - msg = heap_alloc((len + 1)*sizeof(WCHAR)); + if (!(msg = heap_alloc((len + 1) * sizeof(WCHAR)))) + break; SendMessageW(hwnd, LB_GETTEXT, sel, (LPARAM)msg); SendMessageW(This->hwndEdit, WM_SETTEXT, 0, (LPARAM)msg); SendMessageW(This->hwndEdit, EM_SETSEL, 0, lstrlenW(msg)); -- 1.9.1
Signed-off-by: Huw Davies <huw(a)codeweavers.com>
We can retrieve the length of the string from the SendMessage calls already. Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/shell32/autocomplete.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/dlls/shell32/autocomplete.c b/dlls/shell32/autocomplete.c index 6a77ddb..4160235 100644 --- a/dlls/shell32/autocomplete.c +++ b/dlls/shell32/autocomplete.c @@ -212,13 +212,15 @@ static LRESULT APIENTRY ACEditSubclassProc(HWND hwnd, UINT uMsg, WPARAM wParam, len = SendMessageW(This->hwndListBox, LB_GETTEXTLEN, sel, 0); if (!(msg = heap_alloc((len + 1) * sizeof(WCHAR)))) return 0; - SendMessageW(This->hwndListBox, LB_GETTEXT, sel, (LPARAM)msg); + len = SendMessageW(This->hwndListBox, LB_GETTEXT, sel, (LPARAM)msg); SendMessageW(hwnd, WM_SETTEXT, 0, (LPARAM)msg); - SendMessageW(hwnd, EM_SETSEL, lstrlenW(msg), lstrlenW(msg)); + SendMessageW(hwnd, EM_SETSEL, len, len); heap_free(msg); } else { + UINT len; SendMessageW(hwnd, WM_SETTEXT, 0, (LPARAM)This->txtbackup); - SendMessageW(hwnd, EM_SETSEL, lstrlenW(This->txtbackup), lstrlenW(This->txtbackup)); + len = strlenW(This->txtbackup); + SendMessageW(hwnd, EM_SETSEL, len, len); } } return 0; @@ -332,9 +334,9 @@ static LRESULT APIENTRY ACLBoxSubclassProc(HWND hwnd, UINT uMsg, WPARAM wParam, len = SendMessageW(This->hwndListBox, LB_GETTEXTLEN, sel, 0); if (!(msg = heap_alloc((len + 1) * sizeof(WCHAR)))) break; - SendMessageW(hwnd, LB_GETTEXT, sel, (LPARAM)msg); + len = SendMessageW(hwnd, LB_GETTEXT, sel, (LPARAM)msg); SendMessageW(This->hwndEdit, WM_SETTEXT, 0, (LPARAM)msg); - SendMessageW(This->hwndEdit, EM_SETSEL, 0, lstrlenW(msg)); + SendMessageW(This->hwndEdit, EM_SETSEL, 0, len); ShowWindow(hwnd, SW_HIDE); heap_free(msg); break; -- 1.9.1
Signed-off-by: Huw Davies <huw(a)codeweavers.com>
Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/shell32/autocomplete.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/dlls/shell32/autocomplete.c b/dlls/shell32/autocomplete.c index 4160235..a76a01f 100644 --- a/dlls/shell32/autocomplete.c +++ b/dlls/shell32/autocomplete.c @@ -144,7 +144,8 @@ static LRESULT APIENTRY ACEditSubclassProc(HWND hwnd, UINT uMsg, WPARAM wParam, switch (uMsg) { case CB_SHOWDROPDOWN: - ShowWindow(This->hwndListBox, SW_HIDE); + if (This->options & ACO_AUTOSUGGEST) + ShowWindow(This->hwndListBox, SW_HIDE); break; case WM_KILLFOCUS: if ((This->options & ACO_AUTOSUGGEST) && ((HWND)wParam != This->hwndListBox)) @@ -175,7 +176,8 @@ static LRESULT APIENTRY ACEditSubclassProc(HWND hwnd, UINT uMsg, WPARAM wParam, } } - ShowWindow(This->hwndListBox, SW_HIDE); + if (This->options & ACO_AUTOSUGGEST) + ShowWindow(This->hwndListBox, SW_HIDE); return 0; case VK_LEFT: case VK_RIGHT: @@ -310,7 +312,6 @@ static LRESULT APIENTRY ACEditSubclassProc(HWND hwnd, UINT uMsg, WPARAM wParam, } default: return CallWindowProcW(This->wpOrigEditProc, hwnd, uMsg, wParam, lParam); - } return 0; @@ -362,6 +363,8 @@ static void create_listbox(IAutoCompleteImpl *This) This->wpOrigLBoxProc = (WNDPROC) SetWindowLongPtrW( This->hwndListBox, GWLP_WNDPROC, (LONG_PTR) ACLBoxSubclassProc); SetWindowLongPtrW( This->hwndListBox, GWLP_USERDATA, (LONG_PTR)This); } + else + This->options &= ~ACO_AUTOSUGGEST; } /************************************************************************** -- 1.9.1
Signed-off-by: Huw Davies <huw(a)codeweavers.com>
There's no need to have filled, since cpt can already provide the same information. Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/shell32/autocomplete.c | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/dlls/shell32/autocomplete.c b/dlls/shell32/autocomplete.c index a76a01f..6f97665 100644 --- a/dlls/shell32/autocomplete.c +++ b/dlls/shell32/autocomplete.c @@ -136,7 +136,7 @@ static LRESULT APIENTRY ACEditSubclassProc(HWND hwnd, UINT uMsg, WPARAM wParam, HRESULT hr; WCHAR hwndText[255]; RECT r; - BOOL control, filled, displayall = FALSE; + BOOL displayall = FALSE; int cpt, height, sel; if (!This->enabled) return CallWindowProcW(This->wpOrigEditProc, hwnd, uMsg, wParam, lParam); @@ -162,8 +162,8 @@ static LRESULT APIENTRY ACEditSubclassProc(HWND hwnd, UINT uMsg, WPARAM wParam, switch(wParam) { case VK_RETURN: /* If quickComplete is set and control is pressed, replace the string */ - control = GetKeyState(VK_CONTROL) & 0x8000; - if (control && This->quickComplete) { + if (This->quickComplete && (GetKeyState(VK_CONTROL) & 0x8000)) + { WCHAR *buf; size_t len = strlenW(hwndText); size_t sz = strlenW(This->quickComplete) + 1 + len; @@ -249,7 +249,6 @@ static LRESULT APIENTRY ACEditSubclassProc(HWND hwnd, UINT uMsg, WPARAM wParam, break; IEnumString_Reset(This->enumstr); - filled = FALSE; for(cpt = 0;;) { LPOLESTR strs = NULL; ULONG fetched; @@ -259,7 +258,7 @@ static LRESULT APIENTRY ACEditSubclassProc(HWND hwnd, UINT uMsg, WPARAM wParam, break; if (!strncmpiW(hwndText, strs, len)) { - if (!filled && (This->options & ACO_AUTOAPPEND)) { + if (cpt == 0 && (This->options & ACO_AUTOAPPEND)) { WCHAR buffW[255]; strcpyW(buffW, hwndText); @@ -272,19 +271,17 @@ static LRESULT APIENTRY ACEditSubclassProc(HWND hwnd, UINT uMsg, WPARAM wParam, } } - if (This->options & ACO_AUTOSUGGEST) { + if (This->options & ACO_AUTOSUGGEST) SendMessageW(This->hwndListBox, LB_ADDSTRING, 0, (LPARAM)strs); - cpt++; - } - filled = TRUE; + cpt++; } CoTaskMemFree(strs); } if (This->options & ACO_AUTOSUGGEST) { - if (filled) { + if (cpt) { height = SendMessageW(This->hwndListBox, LB_GETITEMHEIGHT, 0, 0); SendMessageW(This->hwndListBox, LB_CARETOFF, 0, 0); GetWindowRect(hwnd, &r); -- 1.9.1
Signed-off-by: Huw Davies <huw(a)codeweavers.com>
Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/shell32/autocomplete.c | 31 ++++++++++++++++++------------- 1 file changed, 18 insertions(+), 13 deletions(-) diff --git a/dlls/shell32/autocomplete.c b/dlls/shell32/autocomplete.c index 6f97665..7421679 100644 --- a/dlls/shell32/autocomplete.c +++ b/dlls/shell32/autocomplete.c @@ -134,10 +134,11 @@ static LRESULT APIENTRY ACEditSubclassProc(HWND hwnd, UINT uMsg, WPARAM wParam, { IAutoCompleteImpl *This = GetPropW(hwnd, autocomplete_propertyW); HRESULT hr; - WCHAR hwndText[255]; + WCHAR *hwndText; + UINT len, size, cpt; RECT r; BOOL displayall = FALSE; - int cpt, height, sel; + int height, sel; if (!This->enabled) return CallWindowProcW(This->wpOrigEditProc, hwnd, uMsg, wParam, lParam); @@ -154,10 +155,11 @@ static LRESULT APIENTRY ACEditSubclassProc(HWND hwnd, UINT uMsg, WPARAM wParam, } return CallWindowProcW(This->wpOrigEditProc, hwnd, uMsg, wParam, lParam); case WM_KEYUP: - { - int len; - - GetWindowTextW(hwnd, hwndText, ARRAY_SIZE(hwndText)); + len = SendMessageW(hwnd, WM_GETTEXTLENGTH, 0, 0); + size = len + 1; + if (!(hwndText = heap_alloc(size * sizeof(WCHAR)))) + return 0; + len = SendMessageW(hwnd, WM_GETTEXT, size, (LPARAM)hwndText); switch(wParam) { case VK_RETURN: @@ -165,7 +167,6 @@ static LRESULT APIENTRY ACEditSubclassProc(HWND hwnd, UINT uMsg, WPARAM wParam, if (This->quickComplete && (GetKeyState(VK_CONTROL) & 0x8000)) { WCHAR *buf; - size_t len = strlenW(hwndText); size_t sz = strlenW(This->quickComplete) + 1 + len; if ((buf = heap_alloc(sz * sizeof(WCHAR)))) { @@ -178,9 +179,11 @@ static LRESULT APIENTRY ACEditSubclassProc(HWND hwnd, UINT uMsg, WPARAM wParam, if (This->options & ACO_AUTOSUGGEST) ShowWindow(This->hwndListBox, SW_HIDE); + heap_free(hwndText); return 0; case VK_LEFT: case VK_RIGHT: + heap_free(hwndText); return 0; case VK_UP: case VK_DOWN: @@ -196,6 +199,7 @@ static LRESULT APIENTRY ACEditSubclassProc(HWND hwnd, UINT uMsg, WPARAM wParam, /* We must display all the entries */ displayall = TRUE; } else { + heap_free(hwndText); if (IsWindowVisible(This->hwndListBox)) { int count; @@ -231,21 +235,23 @@ static LRESULT APIENTRY ACEditSubclassProc(HWND hwnd, UINT uMsg, WPARAM wParam, case VK_BACK: case VK_DELETE: if ((! *hwndText) && (This->options & ACO_AUTOSUGGEST)) { + heap_free(hwndText); ShowWindow(This->hwndListBox, SW_HIDE); return CallWindowProcW(This->wpOrigEditProc, hwnd, uMsg, wParam, lParam); } break; } + if (len + 1 != size) + hwndText = heap_realloc(hwndText, len + 1); + SendMessageW(This->hwndListBox, LB_RESETCONTENT, 0, 0); + /* Set txtbackup to point to hwndText itself (which must not be released) */ heap_free(This->txtbackup); - len = strlenW(hwndText); - This->txtbackup = heap_alloc((len + 1)*sizeof(WCHAR)); - lstrcpyW(This->txtbackup, hwndText); + This->txtbackup = hwndText; - /* Returns if there is no text to search and we doesn't want to display all the entries */ - if ((!displayall) && (! *hwndText) ) + if (!displayall && !len) break; IEnumString_Reset(This->enumstr); @@ -297,7 +303,6 @@ static LRESULT APIENTRY ACEditSubclassProc(HWND hwnd, UINT uMsg, WPARAM wParam, } break; - } case WM_DESTROY: { WNDPROC proc = This->wpOrigEditProc; -- 1.9.1
On Mon, Sep 10, 2018 at 10:09:36PM +0300, Gabriel Ivăncescu wrote:
Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/shell32/autocomplete.c | 31 ++++++++++++++++++------------- 1 file changed, 18 insertions(+), 13 deletions(-)
diff --git a/dlls/shell32/autocomplete.c b/dlls/shell32/autocomplete.c index 6f97665..7421679 100644 --- a/dlls/shell32/autocomplete.c +++ b/dlls/shell32/autocomplete.c @@ -134,10 +134,11 @@ static LRESULT APIENTRY ACEditSubclassProc(HWND hwnd, UINT uMsg, WPARAM wParam, { IAutoCompleteImpl *This = GetPropW(hwnd, autocomplete_propertyW); HRESULT hr; - WCHAR hwndText[255]; + WCHAR *hwndText; + UINT len, size, cpt; RECT r; BOOL displayall = FALSE; - int cpt, height, sel; + int height, sel;
if (!This->enabled) return CallWindowProcW(This->wpOrigEditProc, hwnd, uMsg, wParam, lParam);
@@ -154,10 +155,11 @@ static LRESULT APIENTRY ACEditSubclassProc(HWND hwnd, UINT uMsg, WPARAM wParam, } return CallWindowProcW(This->wpOrigEditProc, hwnd, uMsg, wParam, lParam); case WM_KEYUP: - { - int len; - - GetWindowTextW(hwnd, hwndText, ARRAY_SIZE(hwndText)); + len = SendMessageW(hwnd, WM_GETTEXTLENGTH, 0, 0); + size = len + 1; + if (!(hwndText = heap_alloc(size * sizeof(WCHAR)))) + return 0; + len = SendMessageW(hwnd, WM_GETTEXT, size, (LPARAM)hwndText);
switch(wParam) { case VK_RETURN: @@ -165,7 +167,6 @@ static LRESULT APIENTRY ACEditSubclassProc(HWND hwnd, UINT uMsg, WPARAM wParam, if (This->quickComplete && (GetKeyState(VK_CONTROL) & 0x8000)) { WCHAR *buf; - size_t len = strlenW(hwndText); size_t sz = strlenW(This->quickComplete) + 1 + len; if ((buf = heap_alloc(sz * sizeof(WCHAR)))) { @@ -178,9 +179,11 @@ static LRESULT APIENTRY ACEditSubclassProc(HWND hwnd, UINT uMsg, WPARAM wParam,
if (This->options & ACO_AUTOSUGGEST) ShowWindow(This->hwndListBox, SW_HIDE); + heap_free(hwndText); return 0; case VK_LEFT: case VK_RIGHT: + heap_free(hwndText); return 0; case VK_UP: case VK_DOWN: @@ -196,6 +199,7 @@ static LRESULT APIENTRY ACEditSubclassProc(HWND hwnd, UINT uMsg, WPARAM wParam, /* We must display all the entries */ displayall = TRUE; } else { + heap_free(hwndText); if (IsWindowVisible(This->hwndListBox)) { int count;
@@ -231,21 +235,23 @@ static LRESULT APIENTRY ACEditSubclassProc(HWND hwnd, UINT uMsg, WPARAM wParam, case VK_BACK: case VK_DELETE: if ((! *hwndText) && (This->options & ACO_AUTOSUGGEST)) { + heap_free(hwndText); ShowWindow(This->hwndListBox, SW_HIDE); return CallWindowProcW(This->wpOrigEditProc, hwnd, uMsg, wParam, lParam); } break; }
+ if (len + 1 != size) + hwndText = heap_realloc(hwndText, len + 1);
(len + 1) * sizeof(WCHAR) This is a good example of why we want small patches. It would have been much harder to spot that in one of your earlier versions. Huw.
On Tue, Sep 11, 2018 at 1:55 PM, Huw Davies <huw(a)codeweavers.com> wrote:
(len + 1) * sizeof(WCHAR)
This is a good example of why we want small patches. It would have been much harder to spot that in one of your earlier versions.
Huw.
Oops yeah you're right. I missed it I guess it never happened in testing due to no ANSI conversions. :-)
AutoComplete currently shows up when the user releases a key, which is wrong. Windows does it when the user presses a key, so use both WM_KEYDOWN and WM_CHAR and redesign it so that it matches Windows behavior. Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/shell32/autocomplete.c | 97 +++++++++++++++++++++++++++------------------ 1 file changed, 58 insertions(+), 39 deletions(-) diff --git a/dlls/shell32/autocomplete.c b/dlls/shell32/autocomplete.c index 7421679..49cf37a 100644 --- a/dlls/shell32/autocomplete.c +++ b/dlls/shell32/autocomplete.c @@ -134,10 +134,11 @@ static LRESULT APIENTRY ACEditSubclassProc(HWND hwnd, UINT uMsg, WPARAM wParam, { IAutoCompleteImpl *This = GetPropW(hwnd, autocomplete_propertyW); HRESULT hr; + LRESULT ret; WCHAR *hwndText; UINT len, size, cpt; RECT r; - BOOL displayall = FALSE; + BOOLEAN displayall = FALSE, noautoappend = !(This->options & ACO_AUTOAPPEND); int height, sel; if (!This->enabled) return CallWindowProcW(This->wpOrigEditProc, hwnd, uMsg, wParam, lParam); @@ -154,20 +155,20 @@ static LRESULT APIENTRY ACEditSubclassProc(HWND hwnd, UINT uMsg, WPARAM wParam, ShowWindow(This->hwndListBox, SW_HIDE); } return CallWindowProcW(This->wpOrigEditProc, hwnd, uMsg, wParam, lParam); - case WM_KEYUP: - len = SendMessageW(hwnd, WM_GETTEXTLENGTH, 0, 0); - size = len + 1; - if (!(hwndText = heap_alloc(size * sizeof(WCHAR)))) - return 0; - len = SendMessageW(hwnd, WM_GETTEXT, size, (LPARAM)hwndText); - + case WM_KEYDOWN: switch(wParam) { case VK_RETURN: /* If quickComplete is set and control is pressed, replace the string */ if (This->quickComplete && (GetKeyState(VK_CONTROL) & 0x8000)) { WCHAR *buf; - size_t sz = strlenW(This->quickComplete) + 1 + len; + size_t sz; + len = SendMessageW(hwnd, WM_GETTEXTLENGTH, 0, 0) + 1; /* include NUL */ + if (!(hwndText = heap_alloc(len * sizeof(WCHAR)))) + return 0; + len = SendMessageW(hwnd, WM_GETTEXT, len, (LPARAM)hwndText); + sz = strlenW(This->quickComplete) + 1 + len; + if ((buf = heap_alloc(sz * sizeof(WCHAR)))) { len = format_quick_complete(buf, This->quickComplete, hwndText, len); @@ -175,32 +176,31 @@ static LRESULT APIENTRY ACEditSubclassProc(HWND hwnd, UINT uMsg, WPARAM wParam, SendMessageW(hwnd, EM_SETSEL, 0, len); heap_free(buf); } + if (This->options & ACO_AUTOSUGGEST) + ShowWindow(This->hwndListBox, SW_HIDE); + heap_free(hwndText); + return 0; } if (This->options & ACO_AUTOSUGGEST) ShowWindow(This->hwndListBox, SW_HIDE); - heap_free(hwndText); - return 0; - case VK_LEFT: - case VK_RIGHT: - heap_free(hwndText); - return 0; + break; case VK_UP: case VK_DOWN: - /* Two cases here : - - if the listbox is not visible, displays it - with all the entries if the style ACO_UPDOWNKEYDROPSLIST - is present but does not select anything. + /* Two cases here: + - if the listbox is not visible and ACO_UPDOWNKEYDROPSLIST is + set, display it with all the entries, without selecting any - if the listbox is visible, change the selection */ - if ( (This->options & (ACO_AUTOSUGGEST | ACO_UPDOWNKEYDROPSLIST)) - && (!IsWindowVisible(This->hwndListBox) && (! *hwndText)) ) - { - /* We must display all the entries */ - displayall = TRUE; - } else { - heap_free(hwndText); - if (IsWindowVisible(This->hwndListBox)) { + if (This->options & ACO_AUTOSUGGEST) { + if (!IsWindowVisible(This->hwndListBox)) { + if (This->options & ACO_UPDOWNKEYDROPSLIST) { + ret = 0; + displayall = TRUE; + noautoappend = TRUE; + goto handle_char; + } + } else { int count; count = SendMessageW(This->hwndListBox, LB_GETCOUNT, 0, 0); @@ -228,20 +228,40 @@ static LRESULT APIENTRY ACEditSubclassProc(HWND hwnd, UINT uMsg, WPARAM wParam, len = strlenW(This->txtbackup); SendMessageW(hwnd, EM_SETSEL, len, len); } + return 0; } - return 0; } break; - case VK_BACK: case VK_DELETE: - if ((! *hwndText) && (This->options & ACO_AUTOSUGGEST)) { - heap_free(hwndText); - ShowWindow(This->hwndListBox, SW_HIDE); - return CallWindowProcW(This->wpOrigEditProc, hwnd, uMsg, wParam, lParam); - } - break; + ret = CallWindowProcW(This->wpOrigEditProc, hwnd, uMsg, wParam, lParam); + goto handle_control_char; + } + return CallWindowProcW(This->wpOrigEditProc, hwnd, uMsg, wParam, lParam); + case WM_CHAR: + case WM_UNICHAR: + ret = CallWindowProcW(This->wpOrigEditProc, hwnd, uMsg, wParam, lParam); + if (wParam < ' ') + { + handle_control_char: + len = SendMessageW(hwnd, WM_GETTEXTLENGTH, 0, 0); + if ((This->options & ACO_AUTOSUGGEST) && len == 0) + { + ShowWindow(This->hwndListBox, SW_HIDE); + return ret; + } + if (wParam != 0x16 /* ^V (paste) */) + noautoappend = TRUE; + } + else + { + handle_char: + len = SendMessageW(hwnd, WM_GETTEXTLENGTH, 0, 0); } + size = len + 1; + if (!(hwndText = heap_alloc(size * sizeof(WCHAR)))) + return ret; + len = SendMessageW(hwnd, WM_GETTEXT, size, (LPARAM)hwndText); if (len + 1 != size) hwndText = heap_realloc(hwndText, len + 1); @@ -252,7 +272,7 @@ static LRESULT APIENTRY ACEditSubclassProc(HWND hwnd, UINT uMsg, WPARAM wParam, This->txtbackup = hwndText; if (!displayall && !len) - break; + return ret; IEnumString_Reset(This->enumstr); for(cpt = 0;;) { @@ -264,7 +284,7 @@ static LRESULT APIENTRY ACEditSubclassProc(HWND hwnd, UINT uMsg, WPARAM wParam, break; if (!strncmpiW(hwndText, strs, len)) { - if (cpt == 0 && (This->options & ACO_AUTOAPPEND)) { + if (cpt == 0 && noautoappend == FALSE) { WCHAR buffW[255]; strcpyW(buffW, hwndText); @@ -301,8 +321,7 @@ static LRESULT APIENTRY ACEditSubclassProc(HWND hwnd, UINT uMsg, WPARAM wParam, ShowWindow(This->hwndListBox, SW_HIDE); } } - - break; + return ret; case WM_DESTROY: { WNDPROC proc = This->wpOrigEditProc; -- 1.9.1
On Mon, Sep 10, 2018 at 10:09:37PM +0300, Gabriel Ivăncescu wrote:
AutoComplete currently shows up when the user releases a key, which is wrong. Windows does it when the user presses a key, so use both WM_KEYDOWN and WM_CHAR and redesign it so that it matches Windows behavior.
Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/shell32/autocomplete.c | 97 +++++++++++++++++++++++++++------------------ 1 file changed, 58 insertions(+), 39 deletions(-)
diff --git a/dlls/shell32/autocomplete.c b/dlls/shell32/autocomplete.c index 7421679..49cf37a 100644 --- a/dlls/shell32/autocomplete.c +++ b/dlls/shell32/autocomplete.c @@ -134,10 +134,11 @@ static LRESULT APIENTRY ACEditSubclassProc(HWND hwnd, UINT uMsg, WPARAM wParam, { IAutoCompleteImpl *This = GetPropW(hwnd, autocomplete_propertyW); HRESULT hr; + LRESULT ret; WCHAR *hwndText; UINT len, size, cpt; RECT r; - BOOL displayall = FALSE; + BOOLEAN displayall = FALSE, noautoappend = !(This->options & ACO_AUTOAPPEND); int height, sel;
if (!This->enabled) return CallWindowProcW(This->wpOrigEditProc, hwnd, uMsg, wParam, lParam); @@ -154,20 +155,20 @@ static LRESULT APIENTRY ACEditSubclassProc(HWND hwnd, UINT uMsg, WPARAM wParam, ShowWindow(This->hwndListBox, SW_HIDE); } return CallWindowProcW(This->wpOrigEditProc, hwnd, uMsg, wParam, lParam); - case WM_KEYUP: - len = SendMessageW(hwnd, WM_GETTEXTLENGTH, 0, 0); - size = len + 1; - if (!(hwndText = heap_alloc(size * sizeof(WCHAR)))) - return 0; - len = SendMessageW(hwnd, WM_GETTEXT, size, (LPARAM)hwndText); - + case WM_KEYDOWN: switch(wParam) { case VK_RETURN: /* If quickComplete is set and control is pressed, replace the string */ if (This->quickComplete && (GetKeyState(VK_CONTROL) & 0x8000)) { WCHAR *buf; - size_t sz = strlenW(This->quickComplete) + 1 + len; + size_t sz; + len = SendMessageW(hwnd, WM_GETTEXTLENGTH, 0, 0) + 1; /* include NUL */ + if (!(hwndText = heap_alloc(len * sizeof(WCHAR)))) + return 0; + len = SendMessageW(hwnd, WM_GETTEXT, len, (LPARAM)hwndText); + sz = strlenW(This->quickComplete) + 1 + len; + if ((buf = heap_alloc(sz * sizeof(WCHAR)))) { len = format_quick_complete(buf, This->quickComplete, hwndText, len); @@ -175,32 +176,31 @@ static LRESULT APIENTRY ACEditSubclassProc(HWND hwnd, UINT uMsg, WPARAM wParam, SendMessageW(hwnd, EM_SETSEL, 0, len); heap_free(buf); } + if (This->options & ACO_AUTOSUGGEST) + ShowWindow(This->hwndListBox, SW_HIDE); + heap_free(hwndText); + return 0; }
if (This->options & ACO_AUTOSUGGEST) ShowWindow(This->hwndListBox, SW_HIDE); - heap_free(hwndText); - return 0; - case VK_LEFT: - case VK_RIGHT: - heap_free(hwndText); - return 0; + break; case VK_UP: case VK_DOWN: - /* Two cases here : - - if the listbox is not visible, displays it - with all the entries if the style ACO_UPDOWNKEYDROPSLIST - is present but does not select anything. + /* Two cases here: + - if the listbox is not visible and ACO_UPDOWNKEYDROPSLIST is + set, display it with all the entries, without selecting any - if the listbox is visible, change the selection */ - if ( (This->options & (ACO_AUTOSUGGEST | ACO_UPDOWNKEYDROPSLIST)) - && (!IsWindowVisible(This->hwndListBox) && (! *hwndText)) ) - { - /* We must display all the entries */ - displayall = TRUE; - } else { - heap_free(hwndText); - if (IsWindowVisible(This->hwndListBox)) { + if (This->options & ACO_AUTOSUGGEST) { + if (!IsWindowVisible(This->hwndListBox)) { + if (This->options & ACO_UPDOWNKEYDROPSLIST) { + ret = 0; + displayall = TRUE; + noautoappend = TRUE; + goto handle_char;
No. I'm not signing off on this. You're going to have to come on with a much cleaner way to do this.
+ } + } else { int count;
count = SendMessageW(This->hwndListBox, LB_GETCOUNT, 0, 0); @@ -228,20 +228,40 @@ static LRESULT APIENTRY ACEditSubclassProc(HWND hwnd, UINT uMsg, WPARAM wParam, len = strlenW(This->txtbackup); SendMessageW(hwnd, EM_SETSEL, len, len); } + return 0; } - return 0; } break; - case VK_BACK: case VK_DELETE: - if ((! *hwndText) && (This->options & ACO_AUTOSUGGEST)) { - heap_free(hwndText); - ShowWindow(This->hwndListBox, SW_HIDE); - return CallWindowProcW(This->wpOrigEditProc, hwnd, uMsg, wParam, lParam); - } - break; + ret = CallWindowProcW(This->wpOrigEditProc, hwnd, uMsg, wParam, lParam); + goto handle_control_char; + } + return CallWindowProcW(This->wpOrigEditProc, hwnd, uMsg, wParam, lParam); + case WM_CHAR: + case WM_UNICHAR: + ret = CallWindowProcW(This->wpOrigEditProc, hwnd, uMsg, wParam, lParam); + if (wParam < ' ') + { + handle_control_char: + len = SendMessageW(hwnd, WM_GETTEXTLENGTH, 0, 0); + if ((This->options & ACO_AUTOSUGGEST) && len == 0) + { + ShowWindow(This->hwndListBox, SW_HIDE); + return ret; + } + if (wParam != 0x16 /* ^V (paste) */) + noautoappend = TRUE; + } + else + { + handle_char: + len = SendMessageW(hwnd, WM_GETTEXTLENGTH, 0, 0); }
+ size = len + 1; + if (!(hwndText = heap_alloc(size * sizeof(WCHAR)))) + return ret; + len = SendMessageW(hwnd, WM_GETTEXT, size, (LPARAM)hwndText); if (len + 1 != size) hwndText = heap_realloc(hwndText, len + 1);
@@ -252,7 +272,7 @@ static LRESULT APIENTRY ACEditSubclassProc(HWND hwnd, UINT uMsg, WPARAM wParam, This->txtbackup = hwndText;
if (!displayall && !len) - break; + return ret;
IEnumString_Reset(This->enumstr); for(cpt = 0;;) { @@ -264,7 +284,7 @@ static LRESULT APIENTRY ACEditSubclassProc(HWND hwnd, UINT uMsg, WPARAM wParam, break;
if (!strncmpiW(hwndText, strs, len)) { - if (cpt == 0 && (This->options & ACO_AUTOAPPEND)) { + if (cpt == 0 && noautoappend == FALSE) { WCHAR buffW[255];
strcpyW(buffW, hwndText); @@ -301,8 +321,7 @@ static LRESULT APIENTRY ACEditSubclassProc(HWND hwnd, UINT uMsg, WPARAM wParam, ShowWindow(This->hwndListBox, SW_HIDE); } } - - break; + return ret; case WM_DESTROY: { WNDPROC proc = This->wpOrigEditProc; -- 1.9.1
On Tue, Sep 11, 2018 at 2:54 PM, Huw Davies <huw(a)codeweavers.com> wrote:
No. I'm not signing off on this. You're going to have to come on with a much cleaner way to do this.
What specifically? The goto after the displayall stuff, or the three nested ifs there? Obviously, my first thought was to just break off on the first if, but then it would change the indentation and the patch would be much larger (same as with my initial patches with heap_alloc failure). I can't split it either, since otherwise the label wouldn't exist and it would fail to compile. (even if it did compile being split, it would amount of wrong usage results, half of it WM_KEYDOWN and no WM_CHAR etc) If it's the goto... I don't know how to handle it differently without some other ugly complication, or a lot more code / changes.
On Tue, Sep 11, 2018 at 08:04:50PM +0300, Gabriel Ivăncescu wrote:
On Tue, Sep 11, 2018 at 2:54 PM, Huw Davies <huw(a)codeweavers.com> wrote:
No. I'm not signing off on this. You're going to have to come on with a much cleaner way to do this.
What specifically? The goto after the displayall stuff, or the three nested ifs there? Obviously, my first thought was to just break off on the first if, but then it would change the indentation and the patch would be much larger (same as with my initial patches with heap_alloc failure). I can't split it either, since otherwise the label wouldn't exist and it would fail to compile. (even if it did compile being split, it would amount of wrong usage results, half of it WM_KEYDOWN and no WM_CHAR etc)
If it's the goto... I don't know how to handle it differently without some other ugly complication, or a lot more code / changes.
Specifically the gotos, but I've already commented on the high identation level in this function (I know that's not your fault). It seems to me that you want the WM_KEYDOWN handler to call the WM_CHAR handler. At the risk of repeating myself, having the handlers as a helper functions would make this easier. Huw.
On Tue, Sep 11, 2018 at 8:22 PM, Huw Davies <huw(a)codeweavers.com> wrote:
Specifically the gotos, but I've already commented on the high identation level in this function (I know that's not your fault).
It seems to me that you want the WM_KEYDOWN handler to call the WM_CHAR handler. At the risk of repeating myself, having the handlers as a helper functions would make this easier.
Huw.
I honestly don't think the gotos are too bad (especially the VK_DELETE one since it's also very close), and my reasoning for it is because all the variables that could be affected are set right before the goto (ret, displayall, noautoappend), so there's no state to memorize, because every other variable is set before both switch statements (including parameters). However, that probably won't convince you, so I'll maybe end up having to rewrite this, but would a larger patch be acceptable then? I mean, to get it in a "workable" state it's pretty hard to split it up. That said there are still some difficulties with functions instead of goto, since the goto doesn't jump directly into WM_CHAR but a sub-block depending on condition. It is easy to make most of WM_CHAR a separate function, but not the original if blocks. For example, VK_DELETE's wParam is actually not a "control character" in terms of WM_CHAR's wParam, yet it needs to use that same code path (that's why it jumps inside the if statement, to share the code). Maybe I can give an extra bool argument to the helper function, to know whether to go the "control character" route or not. Definitely not as elegant in my opinion, but if it has to be done heh. But yeah, I won't be able to do this without rewriting it... BTW just wondering but what is exactly bad about the goto in this situation? (just the first one or both?) Of course, I know it's not ideal, but what makes it so bad I mean? I'm one of those who dislikes littering code with BOOLs and have a lot of checks just to avoid some gotos (which make code cleaner in that situation), so it's not obvious to me due to that background, that's why I'd like to know :-)
On 11 Sep 2018, at 18:48, Gabriel Ivăncescu <gabrielopcode(a)gmail.com> wrote:
On Tue, Sep 11, 2018 at 8:22 PM, Huw Davies <huw(a)codeweavers.com> wrote:
Specifically the gotos, but I've already commented on the high identation level in this function (I know that's not your fault).
It seems to me that you want the WM_KEYDOWN handler to call the WM_CHAR handler. At the risk of repeating myself, having the handlers as a helper functions would make this easier.
Huw.
I honestly don't think the gotos are too bad (especially the VK_DELETE one since it's also very close), and my reasoning for it is because all the variables that could be affected are set right before the goto (ret, displayall, noautoappend), so there's no state to memorize, because every other variable is set before both switch statements (including parameters).
However, that probably won't convince you, so I'll maybe end up having to rewrite this, but would a larger patch be acceptable then? I mean, to get it in a "workable" state it's pretty hard to split it up.
That said there are still some difficulties with functions instead of goto, since the goto doesn't jump directly into WM_CHAR but a sub-block depending on condition. It is easy to make most of WM_CHAR a separate function, but not the original if blocks. For example, VK_DELETE's wParam is actually not a "control character" in terms of WM_CHAR's wParam, yet it needs to use that same code path (that's why it jumps inside the if statement, to share the code). Maybe I can give an extra bool argument to the helper function, to know whether to go the "control character" route or not. Definitely not as elegant in my opinion, but if it has to be done heh.
But yeah, I won't be able to do this without rewriting it...
BTW just wondering but what is exactly bad about the goto in this situation? (just the first one or both?) Of course, I know it's not ideal, but what makes it so bad I mean? I'm one of those who dislikes littering code with BOOLs and have a lot of checks just to avoid some gotos (which make code cleaner in that situation), so it's not obvious to me due to that background, that's why I'd like to know :-)
It makes following the flow of the code incredibly difficult. If you don't believe me, come back to this patch in a month and see how you feel then. You'll probably find it easier to first move the existing WM_KEYUP code to a helper function as essentially a no-op patch. That patch will be big, but as it's a cut-n-paste job that won't matter. Then you can start pulling it apart. Huw.
On 11 September 2018 at 23:05, Huw Davies <huw(a)codeweavers.com> wrote:
It makes following the flow of the code incredibly difficult. If you don't believe me, come back to this patch in a month and see how you feel then.
Yeah, this. You have goto's in fairly deeply branched code, inside a nested switch, jumping to a branch somewhere inside the outer switch, all of which happens inside a relatively long function.
On Tue, Sep 11, 2018 at 9:51 PM, Henri Verbeet <hverbeet(a)gmail.com> wrote:
On 11 September 2018 at 23:05, Huw Davies <huw(a)codeweavers.com> wrote:
It makes following the flow of the code incredibly difficult. If you don't believe me, come back to this patch in a month and see how you feel then.
Yeah, this. You have goto's in fairly deeply branched code, inside a nested switch, jumping to a branch somewhere inside the outer switch, all of which happens inside a relatively long function.
I see. I usually view switch cases as similar to labels and don't mind goto between nested switch if it jumps near the actual case of the switch and not deep within it (it's basically like sharing the same "case" in my mind), but point taken. I admit that mindset applies to small switch/case blocks though, not like this function. To be honest, I think it will be easier if I skip the no-op patch because this will allow me to allocate the hwndText in the helper function itself and skip passing it as argument, and encapsulate it there along with other stuff like the bools. Of course, most of the function will still be a cut & paste just like before (after the realloc part) so it will only seem artificially large but not that many changes (the control_char handling can be simplified by checking all of them for len of zero, which is better anyway). There's no issue with this, I hope? Or do I really have to make the no-op patch first?
On Wed, Sep 12, 2018 at 01:42:26PM +0300, Gabriel Ivăncescu wrote:
To be honest, I think it will be easier if I skip the no-op patch because this will allow me to allocate the hwndText in the helper function itself and skip passing it as argument, and encapsulate it there along with other stuff like the bools. Of course, most of the function will still be a cut & paste just like before (after the realloc part) so it will only seem artificially large but not that many changes (the control_char handling can be simplified by checking all of them for len of zero, which is better anyway). There's no issue with this, I hope? Or do I really have to make the no-op patch first?
I don't follow. All of the uses of hwndText are inside the current WM_KEYUP handler (at least after you resend the dynamic allocation of hwndText patch), so what's the issue? Remember, you're trying to make this as simple as possible for the reviewers. If there is a straight copy-n-paste patch into a helper that makes life easy. If not, then it's a large patch to review. Huw.
On Wed, Sep 12, 2018 at 1:52 PM, Huw Davies <huw(a)codeweavers.com> wrote:
I don't follow. All of the uses of hwndText are inside the current WM_KEYUP handler (at least after you resend the dynamic allocation of hwndText patch), so what's the issue?
Remember, you're trying to make this as simple as possible for the reviewers. If there is a straight copy-n-paste patch into a helper that makes life easy. If not, then it's a large patch to review.
Huw.
Well, with WM_KEYDOWN the hwndText allocation has to be done inside WM_CHAR and only in VK_RETURN part of WM_KEYDOWN. The reason it has to be done in WM_CHAR is because we have to first pass the WM_CHAR message through to the edit control and *then* get the resulting text. And we can't just pass through the message before WM_CHAR, because VK_RETURN and the arrow keys have to eat it and not forward it for proper behavior (for example arrow keys can conflict with a combo box's arrow keys otherwise, tested this in Total Commander, this doesn't happen on Windows; the return key needs more fixes to match Windows but that will come later). That said, I'll split it then, cause it seems a bit big indeed. I hope you don't mind I'll do some basic syntactic changes in the first patch instead of a straight cut & paste (trivial stuff like braces on new lines, changing names from hwndText to just text, and This to ac, and so on, no actual changes to the code behavior).
The previous code caps the auto-append text at 255 characters, which can be easily exploited. It's also less efficient as it scans the string multiple times. Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/shell32/autocomplete.c | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/dlls/shell32/autocomplete.c b/dlls/shell32/autocomplete.c index 49cf37a..fbf02f9 100644 --- a/dlls/shell32/autocomplete.c +++ b/dlls/shell32/autocomplete.c @@ -285,12 +285,23 @@ static LRESULT APIENTRY ACEditSubclassProc(HWND hwnd, UINT uMsg, WPARAM wParam, if (!strncmpiW(hwndText, strs, len)) { if (cpt == 0 && noautoappend == FALSE) { - WCHAR buffW[255]; + /* The character capitalization can be different, + so merge hwndText and strs into a new string */ + WCHAR *tmp; + size_t strslen = len + strlenW(&strs[len]); + + if ((tmp = heap_alloc((strslen + 1) * sizeof(WCHAR)))) + { + memcpy(tmp, hwndText, len * sizeof(WCHAR)); + memcpy(&tmp[len], &strs[len], (strslen - len + 1) * sizeof(WCHAR)); + } + else tmp = strs; + + SendMessageW(hwnd, WM_SETTEXT, 0, (LPARAM)tmp); + SendMessageW(hwnd, EM_SETSEL, len, strslen); + if (tmp != strs) + heap_free(tmp); - strcpyW(buffW, hwndText); - strcatW(buffW, &strs[len]); - SetWindowTextW(hwnd, buffW); - SendMessageW(hwnd, EM_SETSEL, len, strlenW(strs)); if (!(This->options & ACO_AUTOSUGGEST)) { CoTaskMemFree(strs); break; -- 1.9.1
Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/shell32/autocomplete.c | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/dlls/shell32/autocomplete.c b/dlls/shell32/autocomplete.c index fbf02f9..d2ed30d 100644 --- a/dlls/shell32/autocomplete.c +++ b/dlls/shell32/autocomplete.c @@ -137,9 +137,8 @@ static LRESULT APIENTRY ACEditSubclassProc(HWND hwnd, UINT uMsg, WPARAM wParam, LRESULT ret; WCHAR *hwndText; UINT len, size, cpt; - RECT r; BOOLEAN displayall = FALSE, noautoappend = !(This->options & ACO_AUTOAPPEND); - int height, sel; + INT sel; if (!This->enabled) return CallWindowProcW(This->wpOrigEditProc, hwnd, uMsg, wParam, lParam); @@ -201,9 +200,8 @@ static LRESULT APIENTRY ACEditSubclassProc(HWND hwnd, UINT uMsg, WPARAM wParam, goto handle_char; } } else { - int count; + INT count = SendMessageW(This->hwndListBox, LB_GETCOUNT, 0, 0); - count = SendMessageW(This->hwndListBox, LB_GETCOUNT, 0, 0); /* Change the selection */ sel = SendMessageW(This->hwndListBox, LB_GETCURSEL, 0, 0); if (wParam == VK_UP) @@ -319,14 +317,15 @@ static LRESULT APIENTRY ACEditSubclassProc(HWND hwnd, UINT uMsg, WPARAM wParam, if (This->options & ACO_AUTOSUGGEST) { if (cpt) { - height = SendMessageW(This->hwndListBox, LB_GETITEMHEIGHT, 0, 0); + RECT r; + UINT height = SendMessageW(This->hwndListBox, LB_GETITEMHEIGHT, 0, 0); SendMessageW(This->hwndListBox, LB_CARETOFF, 0, 0); GetWindowRect(hwnd, &r); SetParent(This->hwndListBox, HWND_DESKTOP); /* It seems that Windows XP displays 7 lines at most and otherwise displays a vertical scroll bar */ SetWindowPos(This->hwndListBox, HWND_TOP, - r.left, r.bottom + 1, r.right - r.left, min(height * 7, height*(cpt+1)), + r.left, r.bottom + 1, r.right - r.left, height * min(cpt + 1, 7), SWP_SHOWWINDOW ); } else { ShowWindow(This->hwndListBox, SW_HIDE); @@ -337,8 +336,8 @@ static LRESULT APIENTRY ACEditSubclassProc(HWND hwnd, UINT uMsg, WPARAM wParam, { WNDPROC proc = This->wpOrigEditProc; - RemovePropW(hwnd, autocomplete_propertyW); SetWindowLongPtrW(hwnd, GWLP_WNDPROC, (LONG_PTR)proc); + RemovePropW(hwnd, autocomplete_propertyW); destroy_autocomplete_object(This); return CallWindowProcW(proc, hwnd, uMsg, wParam, lParam); } -- 1.9.1
There's no need to send a WM_KEYUP anymore since it now matches Windows behavior. Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/shell32/tests/autocomplete.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/dlls/shell32/tests/autocomplete.c b/dlls/shell32/tests/autocomplete.c index 3c8de88..b9c6374 100644 --- a/dlls/shell32/tests/autocomplete.c +++ b/dlls/shell32/tests/autocomplete.c @@ -386,8 +386,7 @@ static void test_custom_source(void) ok(hr == S_OK, "IAutoComplete_Init failed: %x\n", hr); SendMessageW(hwnd_edit, WM_CHAR, 'a', 1); - /* Send a keyup message since wine doesn't handle WM_CHAR yet */ - SendMessageW(hwnd_edit, WM_KEYUP, 'u', 1); + SendMessageW(hwnd_edit, WM_CHAR, 'u', 1); Sleep(100); while (PeekMessageA(&msg, 0, 0, 0, PM_REMOVE)) { -- 1.9.1
participants (3)
-
Gabriel Ivăncescu -
Henri Verbeet -
Huw Davies