Signed-off-by: Gabriel Ivăncescu gabrielopcode@gmail.com ---
v5: Fix heap_realloc size mistake
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..9f35ff7 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) * sizeof(WCHAR)); + 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;
Signed-off-by: Gabriel Ivăncescu gabrielopcode@gmail.com --- dlls/shell32/autocomplete.c | 135 ++++++++++++++++++++++++-------------------- 1 file changed, 73 insertions(+), 62 deletions(-)
diff --git a/dlls/shell32/autocomplete.c b/dlls/shell32/autocomplete.c index 9f35ff7..63fdf3d 100644 --- a/dlls/shell32/autocomplete.c +++ b/dlls/shell32/autocomplete.c @@ -119,6 +119,76 @@ static size_t format_quick_complete(WCHAR *dst, const WCHAR *qc, const WCHAR *st return dst - base; }
+static void autocomplete_text(IAutoCompleteImpl *ac, WCHAR *text, UINT len, HWND hwnd, BOOL displayall) +{ + HRESULT hr; + UINT cpt; + + SendMessageW(ac->hwndListBox, LB_RESETCONTENT, 0, 0); + + /* Set txtbackup to point to text itself (which must not be released) */ + heap_free(ac->txtbackup); + ac->txtbackup = text; + + if (!displayall && !len) + return; + + IEnumString_Reset(ac->enumstr); + for(cpt = 0;;) + { + LPOLESTR strs = NULL; + ULONG fetched; + + hr = IEnumString_Next(ac->enumstr, 1, &strs, &fetched); + if (hr != S_OK) + break; + + if (!strncmpiW(text, strs, len)) + { + if (cpt == 0 && (ac->options & ACO_AUTOAPPEND)) + { + WCHAR buffW[255]; + + strcpyW(buffW, text); + strcatW(buffW, &strs[len]); + SetWindowTextW(hwnd, buffW); + SendMessageW(hwnd, EM_SETSEL, len, strlenW(strs)); + if (!(ac->options & ACO_AUTOSUGGEST)) + { + CoTaskMemFree(strs); + break; + } + } + + if (ac->options & ACO_AUTOSUGGEST) + SendMessageW(ac->hwndListBox, LB_ADDSTRING, 0, (LPARAM)strs); + + cpt++; + } + + CoTaskMemFree(strs); + } + + if (ac->options & ACO_AUTOSUGGEST) + { + if (cpt) + { + RECT r; + UINT height = SendMessageW(ac->hwndListBox, LB_GETITEMHEIGHT, 0, 0); + SendMessageW(ac->hwndListBox, LB_CARETOFF, 0, 0); + GetWindowRect(hwnd, &r); + SetParent(ac->hwndListBox, HWND_DESKTOP); + /* It seems that Windows XP displays 7 lines at most + and otherwise displays a vertical scroll bar */ + SetWindowPos(ac->hwndListBox, HWND_TOP, + r.left, r.bottom + 1, r.right - r.left, min(height * 7, height*(cpt+1)), + SWP_SHOWWINDOW ); + } + else + ShowWindow(ac->hwndListBox, SW_HIDE); + } +} + static void destroy_autocomplete_object(IAutoCompleteImpl *ac) { ac->hwndEdit = NULL; @@ -133,12 +203,9 @@ static void destroy_autocomplete_object(IAutoCompleteImpl *ac) static LRESULT APIENTRY ACEditSubclassProc(HWND hwnd, UINT uMsg, WPARAM wParam, LPARAM lParam) { IAutoCompleteImpl *This = GetPropW(hwnd, autocomplete_propertyW); - HRESULT hr; WCHAR *hwndText; - UINT len, size, cpt; - RECT r; + UINT len, size; BOOL displayall = FALSE; - int height, sel;
if (!This->enabled) return CallWindowProcW(This->wpOrigEditProc, hwnd, uMsg, wParam, lParam);
@@ -201,7 +268,7 @@ static LRESULT APIENTRY ACEditSubclassProc(HWND hwnd, UINT uMsg, WPARAM wParam, } else { heap_free(hwndText); if (IsWindowVisible(This->hwndListBox)) { - int count; + int count, sel;
count = SendMessageW(This->hwndListBox, LB_GETCOUNT, 0, 0); /* Change the selection */ @@ -245,63 +312,7 @@ static LRESULT APIENTRY ACEditSubclassProc(HWND hwnd, UINT uMsg, WPARAM wParam, if (len + 1 != size) hwndText = heap_realloc(hwndText, (len + 1) * sizeof(WCHAR));
- SendMessageW(This->hwndListBox, LB_RESETCONTENT, 0, 0); - - /* Set txtbackup to point to hwndText itself (which must not be released) */ - heap_free(This->txtbackup); - This->txtbackup = hwndText; - - if (!displayall && !len) - break; - - IEnumString_Reset(This->enumstr); - for(cpt = 0;;) { - LPOLESTR strs = NULL; - ULONG fetched; - - hr = IEnumString_Next(This->enumstr, 1, &strs, &fetched); - if (hr != S_OK) - break; - - if (!strncmpiW(hwndText, strs, len)) { - if (cpt == 0 && (This->options & ACO_AUTOAPPEND)) { - WCHAR buffW[255]; - - 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; - } - } - - if (This->options & ACO_AUTOSUGGEST) - SendMessageW(This->hwndListBox, LB_ADDSTRING, 0, (LPARAM)strs); - - cpt++; - } - - CoTaskMemFree(strs); - } - - if (This->options & ACO_AUTOSUGGEST) { - if (cpt) { - 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)), - SWP_SHOWWINDOW ); - } else { - ShowWindow(This->hwndListBox, SW_HIDE); - } - } - + autocomplete_text(This, hwndText, len, hwnd, displayall); break; case WM_DESTROY: {
On Wed, Sep 12, 2018 at 10:42:16PM +0300, Gabriel Ivăncescu wrote:
Signed-off-by: Gabriel Ivăncescu gabrielopcode@gmail.com
dlls/shell32/autocomplete.c | 135 ++++++++++++++++++++++++-------------------- 1 file changed, 73 insertions(+), 62 deletions(-)
This wasn't exactly what I was expecting. What I'd hoped to see was the entire WM_KEYUP handler moved to a helper function, not half of it. [FWIW, renaming variable and re-formatting the code in the new helper is a good plan].
After you do that, I then suggest pulling the code that deals with the displayall stuff into a sub-helper---this is most of the code that you have in autocomplete_text().
Huw.
diff --git a/dlls/shell32/autocomplete.c b/dlls/shell32/autocomplete.c index 9f35ff7..63fdf3d 100644 --- a/dlls/shell32/autocomplete.c +++ b/dlls/shell32/autocomplete.c @@ -119,6 +119,76 @@ static size_t format_quick_complete(WCHAR *dst, const WCHAR *qc, const WCHAR *st return dst - base; }
+static void autocomplete_text(IAutoCompleteImpl *ac, WCHAR *text, UINT len, HWND hwnd, BOOL displayall) +{
- HRESULT hr;
- UINT cpt;
- SendMessageW(ac->hwndListBox, LB_RESETCONTENT, 0, 0);
- /* Set txtbackup to point to text itself (which must not be released) */
- heap_free(ac->txtbackup);
- ac->txtbackup = text;
- if (!displayall && !len)
return;
- IEnumString_Reset(ac->enumstr);
- for(cpt = 0;;)
- {
LPOLESTR strs = NULL;
ULONG fetched;
hr = IEnumString_Next(ac->enumstr, 1, &strs, &fetched);
if (hr != S_OK)
break;
if (!strncmpiW(text, strs, len))
{
if (cpt == 0 && (ac->options & ACO_AUTOAPPEND))
{
WCHAR buffW[255];
strcpyW(buffW, text);
strcatW(buffW, &strs[len]);
SetWindowTextW(hwnd, buffW);
SendMessageW(hwnd, EM_SETSEL, len, strlenW(strs));
if (!(ac->options & ACO_AUTOSUGGEST))
{
CoTaskMemFree(strs);
break;
}
}
if (ac->options & ACO_AUTOSUGGEST)
SendMessageW(ac->hwndListBox, LB_ADDSTRING, 0, (LPARAM)strs);
cpt++;
}
CoTaskMemFree(strs);
- }
- if (ac->options & ACO_AUTOSUGGEST)
- {
if (cpt)
{
RECT r;
UINT height = SendMessageW(ac->hwndListBox, LB_GETITEMHEIGHT, 0, 0);
SendMessageW(ac->hwndListBox, LB_CARETOFF, 0, 0);
GetWindowRect(hwnd, &r);
SetParent(ac->hwndListBox, HWND_DESKTOP);
/* It seems that Windows XP displays 7 lines at most
and otherwise displays a vertical scroll bar */
SetWindowPos(ac->hwndListBox, HWND_TOP,
r.left, r.bottom + 1, r.right - r.left, min(height * 7, height*(cpt+1)),
SWP_SHOWWINDOW );
}
else
ShowWindow(ac->hwndListBox, SW_HIDE);
- }
+}
static void destroy_autocomplete_object(IAutoCompleteImpl *ac) { ac->hwndEdit = NULL; @@ -133,12 +203,9 @@ static void destroy_autocomplete_object(IAutoCompleteImpl *ac) static LRESULT APIENTRY ACEditSubclassProc(HWND hwnd, UINT uMsg, WPARAM wParam, LPARAM lParam) { IAutoCompleteImpl *This = GetPropW(hwnd, autocomplete_propertyW);
- HRESULT hr; WCHAR *hwndText;
- UINT len, size, cpt;
- RECT r;
- UINT len, size; BOOL displayall = FALSE;
int height, sel;
if (!This->enabled) return CallWindowProcW(This->wpOrigEditProc, hwnd, uMsg, wParam, lParam);
@@ -201,7 +268,7 @@ static LRESULT APIENTRY ACEditSubclassProc(HWND hwnd, UINT uMsg, WPARAM wParam, } else { heap_free(hwndText); if (IsWindowVisible(This->hwndListBox)) {
int count;
int count, sel; count = SendMessageW(This->hwndListBox, LB_GETCOUNT, 0, 0); /* Change the selection */
@@ -245,63 +312,7 @@ static LRESULT APIENTRY ACEditSubclassProc(HWND hwnd, UINT uMsg, WPARAM wParam, if (len + 1 != size) hwndText = heap_realloc(hwndText, (len + 1) * sizeof(WCHAR));
SendMessageW(This->hwndListBox, LB_RESETCONTENT, 0, 0);
/* Set txtbackup to point to hwndText itself (which must not be released) */
heap_free(This->txtbackup);
This->txtbackup = hwndText;
if (!displayall && !len)
break;
IEnumString_Reset(This->enumstr);
for(cpt = 0;;) {
LPOLESTR strs = NULL;
ULONG fetched;
hr = IEnumString_Next(This->enumstr, 1, &strs, &fetched);
if (hr != S_OK)
break;
if (!strncmpiW(hwndText, strs, len)) {
if (cpt == 0 && (This->options & ACO_AUTOAPPEND)) {
WCHAR buffW[255];
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;
}
}
if (This->options & ACO_AUTOSUGGEST)
SendMessageW(This->hwndListBox, LB_ADDSTRING, 0, (LPARAM)strs);
cpt++;
}
CoTaskMemFree(strs);
}
if (This->options & ACO_AUTOSUGGEST) {
if (cpt) {
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)),
SWP_SHOWWINDOW );
} else {
ShowWindow(This->hwndListBox, SW_HIDE);
}
}
autocomplete_text(This, hwndText, len, hwnd, displayall); break; case WM_DESTROY: {
-- 1.9.1
On Thu, Sep 13, 2018 at 1:34 PM, Huw Davies huw@codeweavers.com wrote:
This wasn't exactly what I was expecting. What I'd hoped to see was the entire WM_KEYUP handler moved to a helper function, not half of it. [FWIW, renaming variable and re-formatting the code in the new helper is a good plan].
After you do that, I then suggest pulling the code that deals with the displayall stuff into a sub-helper---this is most of the code that you have in autocomplete_text().
Huw.
Oh okay I'll do that as well then. Can I also change the int len to UINT in that block when doing it? (I know it doesn't matter in practice, but this would limit x64 to 2GB due to a useless sign extension to SIZE_T, which is what HeapAlloc uses -- it's mostly for code purity than anything practical)
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@gmail.com ---
Compared to version 4 of the patch, noautoappend and displayall have been collapsed into one parameter to reduce clutter when calling the function, since the latter implies the former (uses value 2 since it's only used in one place at the beginning of the function); to me it's better than seeing stuff like TRUE, FALSE or TRUE, TRUE (of course can be changed to that, if needed... just a preference here)
dlls/shell32/autocomplete.c | 108 ++++++++++++++++++++++++-------------------- 1 file changed, 59 insertions(+), 49 deletions(-)
diff --git a/dlls/shell32/autocomplete.c b/dlls/shell32/autocomplete.c index 63fdf3d..71259af 100644 --- a/dlls/shell32/autocomplete.c +++ b/dlls/shell32/autocomplete.c @@ -119,10 +119,25 @@ static size_t format_quick_complete(WCHAR *dst, const WCHAR *qc, const WCHAR *st return dst - base; }
-static void autocomplete_text(IAutoCompleteImpl *ac, WCHAR *text, UINT len, HWND hwnd, BOOL displayall) +static void autocomplete_text(IAutoCompleteImpl *ac, HWND hwnd, BOOL noautoappend) { HRESULT hr; - UINT cpt; + WCHAR *text; + UINT cpt, size, len = SendMessageW(hwnd, WM_GETTEXTLENGTH, 0, 0); + + if (noautoappend != 2 && len == 0) + { + if (ac->options & ACO_AUTOSUGGEST) + ShowWindow(ac->hwndListBox, SW_HIDE); + return; + } + + size = len + 1; + if (!(text = heap_alloc(size * sizeof(WCHAR)))) + return; + len = SendMessageW(hwnd, WM_GETTEXT, size, (LPARAM)text); + if (len + 1 != size) + text = heap_realloc(text, (len + 1) * sizeof(WCHAR));
SendMessageW(ac->hwndListBox, LB_RESETCONTENT, 0, 0);
@@ -130,9 +145,6 @@ static void autocomplete_text(IAutoCompleteImpl *ac, WCHAR *text, UINT len, HWND heap_free(ac->txtbackup); ac->txtbackup = text;
- if (!displayall && !len) - return; - IEnumString_Reset(ac->enumstr); for(cpt = 0;;) { @@ -145,7 +157,7 @@ static void autocomplete_text(IAutoCompleteImpl *ac, WCHAR *text, UINT len, HWND
if (!strncmpiW(text, strs, len)) { - if (cpt == 0 && (ac->options & ACO_AUTOAPPEND)) + if (cpt == 0 && noautoappend == FALSE) { WCHAR buffW[255];
@@ -203,9 +215,7 @@ static void destroy_autocomplete_object(IAutoCompleteImpl *ac) static LRESULT APIENTRY ACEditSubclassProc(HWND hwnd, UINT uMsg, WPARAM wParam, LPARAM lParam) { IAutoCompleteImpl *This = GetPropW(hwnd, autocomplete_propertyW); - WCHAR *hwndText; - UINT len, size; - BOOL displayall = FALSE; + LRESULT ret;
if (!This->enabled) return CallWindowProcW(This->wpOrigEditProc, hwnd, uMsg, wParam, lParam);
@@ -221,20 +231,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; + WCHAR *hwndText, *buf; + size_t sz; + UINT len = SendMessageW(hwnd, WM_GETTEXTLENGTH, 0, 0); + if (!(hwndText = heap_alloc((len + 1) * sizeof(WCHAR)))) + return 0; + len = SendMessageW(hwnd, WM_GETTEXT, len + 1, (LPARAM)hwndText); + sz = strlenW(This->quickComplete) + 1 + len; + if ((buf = heap_alloc(sz * sizeof(WCHAR)))) { len = format_quick_complete(buf, This->quickComplete, hwndText, len); @@ -242,32 +252,35 @@ 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)) ) + if (This->options & ACO_AUTOSUGGEST) { - /* We must display all the entries */ - displayall = TRUE; - } else { - heap_free(hwndText); - if (IsWindowVisible(This->hwndListBox)) { + if (!IsWindowVisible(This->hwndListBox)) + { + if (This->options & ACO_UPDOWNKEYDROPSLIST) + { + autocomplete_text(This, hwnd, 2); + return 0; + } + } + else + { int count, sel;
count = SendMessageW(This->hwndListBox, LB_GETCOUNT, 0, 0); @@ -295,25 +308,22 @@ 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); + autocomplete_text(This, hwnd, TRUE); + return ret; } - - if (len + 1 != size) - hwndText = heap_realloc(hwndText, (len + 1) * sizeof(WCHAR)); - - autocomplete_text(This, hwndText, len, hwnd, displayall); - break; + return CallWindowProcW(This->wpOrigEditProc, hwnd, uMsg, wParam, lParam); + case WM_CHAR: + case WM_UNICHAR: + ret = CallWindowProcW(This->wpOrigEditProc, hwnd, uMsg, wParam, lParam); + autocomplete_text(This, hwnd, (wParam < ' ' && wParam != 0x16 /* ^V (paste) */) + || !(This->options & ACO_AUTOAPPEND)); + return ret; case WM_DESTROY: { WNDPROC proc = This->wpOrigEditProc;
On Wed, Sep 12, 2018 at 10:42:17PM +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@gmail.com
Compared to version 4 of the patch, noautoappend and displayall have been collapsed into one parameter to reduce clutter when calling the function, since the latter implies the former (uses value 2 since it's only used in one place at the beginning of the function); to me it's better than seeing stuff like TRUE, FALSE or TRUE, TRUE (of course can be changed to that, if needed... just a preference here)
Not that it really matters, because this isn't going to go in as it is, but having noautoappend as a tri-state is fine, except you can't declare it as a BOOL and use FALSE, TRUE and 2 as its states. You'd want an enum.
Huw.
dlls/shell32/autocomplete.c | 108 ++++++++++++++++++++++++-------------------- 1 file changed, 59 insertions(+), 49 deletions(-)
diff --git a/dlls/shell32/autocomplete.c b/dlls/shell32/autocomplete.c index 63fdf3d..71259af 100644 --- a/dlls/shell32/autocomplete.c +++ b/dlls/shell32/autocomplete.c @@ -119,10 +119,25 @@ static size_t format_quick_complete(WCHAR *dst, const WCHAR *qc, const WCHAR *st return dst - base; }
-static void autocomplete_text(IAutoCompleteImpl *ac, WCHAR *text, UINT len, HWND hwnd, BOOL displayall) +static void autocomplete_text(IAutoCompleteImpl *ac, HWND hwnd, BOOL noautoappend) { HRESULT hr;
- UINT cpt;
WCHAR *text;
UINT cpt, size, len = SendMessageW(hwnd, WM_GETTEXTLENGTH, 0, 0);
if (noautoappend != 2 && len == 0)
{
if (ac->options & ACO_AUTOSUGGEST)
ShowWindow(ac->hwndListBox, SW_HIDE);
return;
}
size = len + 1;
if (!(text = heap_alloc(size * sizeof(WCHAR))))
return;
len = SendMessageW(hwnd, WM_GETTEXT, size, (LPARAM)text);
if (len + 1 != size)
text = heap_realloc(text, (len + 1) * sizeof(WCHAR));
SendMessageW(ac->hwndListBox, LB_RESETCONTENT, 0, 0);
@@ -130,9 +145,6 @@ static void autocomplete_text(IAutoCompleteImpl *ac, WCHAR *text, UINT len, HWND heap_free(ac->txtbackup); ac->txtbackup = text;
- if (!displayall && !len)
return;
- IEnumString_Reset(ac->enumstr); for(cpt = 0;;) {
@@ -145,7 +157,7 @@ static void autocomplete_text(IAutoCompleteImpl *ac, WCHAR *text, UINT len, HWND
if (!strncmpiW(text, strs, len)) {
if (cpt == 0 && (ac->options & ACO_AUTOAPPEND))
if (cpt == 0 && noautoappend == FALSE) { WCHAR buffW[255];
@@ -203,9 +215,7 @@ static void destroy_autocomplete_object(IAutoCompleteImpl *ac) static LRESULT APIENTRY ACEditSubclassProc(HWND hwnd, UINT uMsg, WPARAM wParam, LPARAM lParam) { IAutoCompleteImpl *This = GetPropW(hwnd, autocomplete_propertyW);
- WCHAR *hwndText;
- UINT len, size;
- BOOL displayall = FALSE;
LRESULT ret;
if (!This->enabled) return CallWindowProcW(This->wpOrigEditProc, hwnd, uMsg, wParam, lParam);
@@ -221,20 +231,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;
WCHAR *hwndText, *buf;
size_t sz;
UINT len = SendMessageW(hwnd, WM_GETTEXTLENGTH, 0, 0);
if (!(hwndText = heap_alloc((len + 1) * sizeof(WCHAR))))
return 0;
len = SendMessageW(hwnd, WM_GETTEXT, len + 1, (LPARAM)hwndText);
sz = strlenW(This->quickComplete) + 1 + len;
if ((buf = heap_alloc(sz * sizeof(WCHAR)))) { len = format_quick_complete(buf, This->quickComplete, hwndText, len);
@@ -242,32 +252,35 @@ 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)) )
if (This->options & ACO_AUTOSUGGEST) {
/* We must display all the entries */
displayall = TRUE;
} else {
heap_free(hwndText);
if (IsWindowVisible(This->hwndListBox)) {
if (!IsWindowVisible(This->hwndListBox))
{
if (This->options & ACO_UPDOWNKEYDROPSLIST)
{
autocomplete_text(This, hwnd, 2);
return 0;
}
}
else
{ int count, sel; count = SendMessageW(This->hwndListBox, LB_GETCOUNT, 0, 0);
@@ -295,25 +308,22 @@ 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);
autocomplete_text(This, hwnd, TRUE);
return ret; }
if (len + 1 != size)
hwndText = heap_realloc(hwndText, (len + 1) * sizeof(WCHAR));
autocomplete_text(This, hwndText, len, hwnd, displayall);
break;
return CallWindowProcW(This->wpOrigEditProc, hwnd, uMsg, wParam, lParam);
case WM_CHAR:
case WM_UNICHAR:
ret = CallWindowProcW(This->wpOrigEditProc, hwnd, uMsg, wParam, lParam);
autocomplete_text(This, hwnd, (wParam < ' ' && wParam != 0x16 /* ^V (paste) */)
|| !(This->options & ACO_AUTOAPPEND));
return ret; case WM_DESTROY: { WNDPROC proc = This->wpOrigEditProc;
-- 1.9.1
On Thu, Sep 13, 2018 at 1:37 PM, Huw Davies huw@codeweavers.com wrote:
Not that it really matters, because this isn't going to go in as it is, but having noautoappend as a tri-state is fine, except you can't declare it as a BOOL and use FALSE, TRUE and 2 as its states. You'd want an enum.
Huw.
I have two questions about the enum. First, is there some sort of naming convention that should be used to avoid future clashes with public definitions (headers)? For example, I know that using all-caps for enum values is common, but that can easily conflict with future public macros or the like, so I personally dislike it. Of course, the Windows API doesn't really use underscores like this_is_an_enum_value for its public definitions, so maybe I should go with that approach? (e.g. noautoappend_displayall as enum value? or some other naming scheme?). Basically I'm asking how to best make the enum "private" for internal linkage purposes in terms of naming convention.
Second question is, how should I tell the enum that any value other than zero is "no auto append", including the "displayall" part? (to simplify the code checks since it implies it) For example currently i just check if it's FALSE, because both TRUE and 2 means "don't auto append" (while 2 further means displayall at the beginning). How to best proceed with an enum there? Maybe I should use a short helper function with the enum as parameter that returns BOOL whether it should auto-append or not?
On Thu, Sep 13, 2018 at 6:47 PM, Gabriel Ivăncescu gabrielopcode@gmail.com wrote:
On Thu, Sep 13, 2018 at 1:37 PM, Huw Davies huw@codeweavers.com wrote:
Not that it really matters, because this isn't going to go in as it is, but having noautoappend as a tri-state is fine, except you can't declare it as a BOOL and use FALSE, TRUE and 2 as its states. You'd want an enum.
Huw.
I have two questions about the enum. First, is there some sort of naming convention that should be used to avoid future clashes with public definitions (headers)? For example, I know that using all-caps for enum values is common, but that can easily conflict with future public macros or the like, so I personally dislike it. Of course, the Windows API doesn't really use underscores like this_is_an_enum_value for its public definitions, so maybe I should go with that approach? (e.g. noautoappend_displayall as enum value? or some other naming scheme?). Basically I'm asking how to best make the enum "private" for internal linkage purposes in terms of naming convention.
Second question is, how should I tell the enum that any value other than zero is "no auto append", including the "displayall" part? (to simplify the code checks since it implies it) For example currently i just check if it's FALSE, because both TRUE and 2 means "don't auto append" (while 2 further means displayall at the beginning). How to best proceed with an enum there? Maybe I should use a short helper function with the enum as parameter that returns BOOL whether it should auto-append or not?
Here's what I have so far:
enum autoappend_flag { autoappend_flag_yes = 0, autoappend_flag_no = 1, autoappend_flag_displayempty = -1 };
static BOOL autoappend_flag_enabled(enum autoappend_flag flag) { /* "no" and "displayempty" don't autoappend, only "yes" does */ return flag == autoappend_flag_yes; }
Is this fine like this? I find it natural with accessor but of course that's just me :-)
On 13 Sep 2018, at 20:12, Gabriel Ivăncescu gabrielopcode@gmail.com wrote:
On Thu, Sep 13, 2018 at 6:47 PM, Gabriel Ivăncescu gabrielopcode@gmail.com wrote:
On Thu, Sep 13, 2018 at 1:37 PM, Huw Davies huw@codeweavers.com wrote:
Not that it really matters, because this isn't going to go in as it is, but having noautoappend as a tri-state is fine, except you can't declare it as a BOOL and use FALSE, TRUE and 2 as its states. You'd want an enum.
Huw.
I have two questions about the enum. First, is there some sort of naming convention that should be used to avoid future clashes with public definitions (headers)? For example, I know that using all-caps for enum values is common, but that can easily conflict with future public macros or the like, so I personally dislike it. Of course, the Windows API doesn't really use underscores like this_is_an_enum_value for its public definitions, so maybe I should go with that approach? (e.g. noautoappend_displayall as enum value? or some other naming scheme?). Basically I'm asking how to best make the enum "private" for internal linkage purposes in terms of naming convention.
Second question is, how should I tell the enum that any value other than zero is "no auto append", including the "displayall" part? (to simplify the code checks since it implies it) For example currently i just check if it's FALSE, because both TRUE and 2 means "don't auto append" (while 2 further means displayall at the beginning). How to best proceed with an enum there? Maybe I should use a short helper function with the enum as parameter that returns BOOL whether it should auto-append or not?
Here's what I have so far:
enum autoappend_flag { autoappend_flag_yes = 0, autoappend_flag_no = 1, autoappend_flag_displayempty = -1 };
Since the actual values are irrelevant don't assign them, just let the compiler pick the defaults.
static BOOL autoappend_flag_enabled(enum autoappend_flag flag) { /* "no" and "displayempty" don't autoappend, only "yes" does */ return flag == autoappend_flag_yes; }
Is this fine like this? I find it natural with accessor but of course that's just me :-)
You're making this more complicated than it needs be; testing for the enabled case is as simple as:
if (flag == autoappend_flag_yes)
there's no need to wrap this in a helper.
Huw.
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@gmail.com --- dlls/shell32/autocomplete.c | 29 +++++++++++++++++++++++------ 1 file changed, 23 insertions(+), 6 deletions(-)
diff --git a/dlls/shell32/autocomplete.c b/dlls/shell32/autocomplete.c index 71259af..c735cec 100644 --- a/dlls/shell32/autocomplete.c +++ b/dlls/shell32/autocomplete.c @@ -119,6 +119,28 @@ static size_t format_quick_complete(WCHAR *dst, const WCHAR *qc, const WCHAR *st return dst - base; }
+static void autoappend_str(IAutoCompleteImpl *ac, WCHAR *text, UINT len, WCHAR *str, HWND hwnd) +{ + WCHAR *tmp; + size_t size; + + /* The character capitalization can be different, + so merge text and str into a new string */ + size = len + strlenW(&str[len]) + 1; + + if ((tmp = heap_alloc(size * sizeof(*tmp)))) + { + memcpy(tmp, text, len * sizeof(*tmp)); + memcpy(&tmp[len], &str[len], (size - len) * sizeof(*tmp)); + } + else tmp = str; + + SendMessageW(hwnd, WM_SETTEXT, 0, (LPARAM)tmp); + SendMessageW(hwnd, EM_SETSEL, len, size - 1); + if (tmp != str) + heap_free(tmp); +} + static void autocomplete_text(IAutoCompleteImpl *ac, HWND hwnd, BOOL noautoappend) { HRESULT hr; @@ -159,12 +181,7 @@ static void autocomplete_text(IAutoCompleteImpl *ac, HWND hwnd, BOOL noautoappen { if (cpt == 0 && noautoappend == FALSE) { - WCHAR buffW[255]; - - strcpyW(buffW, text); - strcatW(buffW, &strs[len]); - SetWindowTextW(hwnd, buffW); - SendMessageW(hwnd, EM_SETSEL, len, strlenW(strs)); + autoappend_str(ac, text, len, strs, hwnd); if (!(ac->options & ACO_AUTOSUGGEST)) { CoTaskMemFree(strs);
Signed-off-by: Gabriel Ivăncescu gabrielopcode@gmail.com --- dlls/shell32/autocomplete.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/dlls/shell32/autocomplete.c b/dlls/shell32/autocomplete.c index c735cec..23db919 100644 --- a/dlls/shell32/autocomplete.c +++ b/dlls/shell32/autocomplete.c @@ -210,7 +210,7 @@ static void autocomplete_text(IAutoCompleteImpl *ac, HWND hwnd, BOOL noautoappen /* It seems that Windows XP displays 7 lines at most and otherwise displays a vertical scroll bar */ SetWindowPos(ac->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
Signed-off-by: Gabriel Ivăncescu gabrielopcode@gmail.com ---
Avoids a race condition in extremely rare situations (i.e. if another thread calls the window procedure while the property is removed, it will crash). It should never happen since it should be tied to 1 thread, but there's no harm to it anyway and it feels better to me.
dlls/shell32/autocomplete.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/dlls/shell32/autocomplete.c b/dlls/shell32/autocomplete.c index 23db919..30190ef 100644 --- a/dlls/shell32/autocomplete.c +++ b/dlls/shell32/autocomplete.c @@ -345,8 +345,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); }
There's no need to send a WM_KEYUP anymore since it now matches Windows behavior.
Signed-off-by: Gabriel Ivăncescu gabrielopcode@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)) {