[PATCH v6 1/6] shell32/autocomplete: Move the autocomplete processing and WM_KEYUP to separate functions
Signed-off-by: Gabriel Ivăncescu gabrielopcode@gmail.com ---
v6: Also move WM_KEYUP to a separate function.
dlls/shell32/autocomplete.c | 336 ++++++++++++++++++++++++-------------------- 1 file changed, 182 insertions(+), 154 deletions(-)
diff --git a/dlls/shell32/autocomplete.c b/dlls/shell32/autocomplete.c index 9f35ff7..ebbb9ee 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; @@ -128,17 +198,122 @@ static void destroy_autocomplete_object(IAutoCompleteImpl *ac) }
/* + Helper for ACEditSubclassProc +*/ +static LRESULT ACEditSubclassProc_KeyUp(IAutoCompleteImpl *ac, HWND hwnd, UINT uMsg, + WPARAM wParam, LPARAM lParam) +{ + WCHAR *text; + UINT len, size; + BOOL displayall = FALSE; + + len = SendMessageW(hwnd, WM_GETTEXTLENGTH, 0, 0); + size = len + 1; + if (!(text = heap_alloc(size * sizeof(WCHAR)))) + return 0; + len = SendMessageW(hwnd, WM_GETTEXT, size, (LPARAM)text); + + switch(wParam) + { + case VK_RETURN: + /* If quickComplete is set and control is pressed, replace the string */ + if (ac->quickComplete && (GetKeyState(VK_CONTROL) & 0x8000)) + { + WCHAR *buf; + size_t sz = strlenW(ac->quickComplete) + 1 + len; + if ((buf = heap_alloc(sz * sizeof(WCHAR)))) + { + len = format_quick_complete(buf, ac->quickComplete, text, len); + SendMessageW(hwnd, WM_SETTEXT, 0, (LPARAM)buf); + SendMessageW(hwnd, EM_SETSEL, 0, len); + heap_free(buf); + } + } + + if (ac->options & ACO_AUTOSUGGEST) + ShowWindow(ac->hwndListBox, SW_HIDE); + heap_free(text); + return 0; + case VK_LEFT: + case VK_RIGHT: + heap_free(text); + return 0; + case VK_UP: + case VK_DOWN: + /* 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 ( (ac->options & (ACO_AUTOSUGGEST | ACO_UPDOWNKEYDROPSLIST)) + && (!IsWindowVisible(ac->hwndListBox) && (! *text)) ) + { + /* We must display all the entries */ + displayall = TRUE; + } + else + { + INT count, sel; + heap_free(text); + if (!IsWindowVisible(ac->hwndListBox)) + return 0; + + count = SendMessageW(ac->hwndListBox, LB_GETCOUNT, 0, 0); + + /* Change the selection */ + sel = SendMessageW(ac->hwndListBox, LB_GETCURSEL, 0, 0); + if (wParam == VK_UP) + sel = ((sel - 1) < 0) ? count - 1 : sel - 1; + else + sel = ((sel + 1) >= count) ? -1 : sel + 1; + SendMessageW(ac->hwndListBox, LB_SETCURSEL, sel, 0); + if (sel >= 0) + { + WCHAR *msg; + UINT len; + + len = SendMessageW(ac->hwndListBox, LB_GETTEXTLEN, sel, 0); + if (!(msg = heap_alloc((len + 1) * sizeof(WCHAR)))) + return 0; + len = SendMessageW(ac->hwndListBox, LB_GETTEXT, sel, (LPARAM)msg); + SendMessageW(hwnd, WM_SETTEXT, 0, (LPARAM)msg); + SendMessageW(hwnd, EM_SETSEL, len, len); + heap_free(msg); + } + else + { + UINT len; + SendMessageW(hwnd, WM_SETTEXT, 0, (LPARAM)ac->txtbackup); + len = strlenW(ac->txtbackup); + SendMessageW(hwnd, EM_SETSEL, len, len); + } + return 0; + } + break; + case VK_BACK: + case VK_DELETE: + if ((! *text) && (ac->options & ACO_AUTOSUGGEST)) + { + heap_free(text); + ShowWindow(ac->hwndListBox, SW_HIDE); + return CallWindowProcW(ac->wpOrigEditProc, hwnd, uMsg, wParam, lParam); + } + break; + } + + if (len + 1 != size) + text = heap_realloc(text, (len + 1) * sizeof(WCHAR)); + + autocomplete_text(ac, text, len, hwnd, displayall); + return 0; +} + +/* Window procedure for autocompletion */ 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; - BOOL displayall = FALSE; - int height, sel;
if (!This->enabled) return CallWindowProcW(This->wpOrigEditProc, hwnd, uMsg, wParam, lParam);
@@ -155,154 +330,7 @@ static LRESULT APIENTRY ACEditSubclassProc(HWND hwnd, UINT uMsg, WPARAM wParam, } 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); - - 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; - 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); - } - } - - 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: - /* 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. - - 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)) { - int count; - - count = SendMessageW(This->hwndListBox, LB_GETCOUNT, 0, 0); - /* Change the selection */ - sel = SendMessageW(This->hwndListBox, LB_GETCURSEL, 0, 0); - if (wParam == VK_UP) - sel = ((sel-1) < 0) ? count-1 : sel-1; - else - sel = ((sel+1) >= count) ? -1 : sel+1; - SendMessageW(This->hwndListBox, LB_SETCURSEL, sel, 0); - if (sel != -1) { - WCHAR *msg; - int len; - - len = SendMessageW(This->hwndListBox, LB_GETTEXTLEN, sel, 0); - if (!(msg = heap_alloc((len + 1) * sizeof(WCHAR)))) - return 0; - len = SendMessageW(This->hwndListBox, LB_GETTEXT, sel, (LPARAM)msg); - SendMessageW(hwnd, WM_SETTEXT, 0, (LPARAM)msg); - SendMessageW(hwnd, EM_SETSEL, len, len); - heap_free(msg); - } else { - UINT len; - SendMessageW(hwnd, WM_SETTEXT, 0, (LPARAM)This->txtbackup); - len = strlenW(This->txtbackup); - SendMessageW(hwnd, EM_SETSEL, len, len); - } - } - 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; - } - - 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); - } - } - - break; + return ACEditSubclassProc_KeyUp(This, hwnd, uMsg, wParam, lParam); case WM_DESTROY: { WNDPROC proc = This->wpOrigEditProc;
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 ---
v6: Use an enum instead of BOOL for the autoappend flag.
dlls/shell32/autocomplete.c | 116 +++++++++++++++++++++++++------------------- 1 file changed, 66 insertions(+), 50 deletions(-)
diff --git a/dlls/shell32/autocomplete.c b/dlls/shell32/autocomplete.c index ebbb9ee..b9ec228 100644 --- a/dlls/shell32/autocomplete.c +++ b/dlls/shell32/autocomplete.c @@ -79,6 +79,13 @@ typedef struct AUTOCOMPLETEOPTIONS options; } IAutoCompleteImpl;
+enum autoappend_flag +{ + autoappend_flag_yes, + autoappend_flag_no, + autoappend_flag_displayempty +}; + static const WCHAR autocomplete_propertyW[] = {'W','i','n','e',' ','A','u','t','o', 'c','o','m','p','l','e','t','e',' ', 'c','o','n','t','r','o','l',0}; @@ -119,10 +126,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, enum autoappend_flag flag) { HRESULT hr; - UINT cpt; + WCHAR *text; + UINT cpt, size, len = SendMessageW(hwnd, WM_GETTEXTLENGTH, 0, 0); + + if (flag != autoappend_flag_displayempty && 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 +152,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 +164,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 && flag == autoappend_flag_yes) { WCHAR buffW[255];
@@ -200,27 +219,23 @@ static void destroy_autocomplete_object(IAutoCompleteImpl *ac) /* Helper for ACEditSubclassProc */ -static LRESULT ACEditSubclassProc_KeyUp(IAutoCompleteImpl *ac, HWND hwnd, UINT uMsg, - WPARAM wParam, LPARAM lParam) +static LRESULT ACEditSubclassProc_KeyDown(IAutoCompleteImpl *ac, HWND hwnd, UINT uMsg, + WPARAM wParam, LPARAM lParam) { - WCHAR *text; - UINT len, size; - BOOL displayall = FALSE; - - len = SendMessageW(hwnd, WM_GETTEXTLENGTH, 0, 0); - size = len + 1; - if (!(text = heap_alloc(size * sizeof(WCHAR)))) - return 0; - len = SendMessageW(hwnd, WM_GETTEXT, size, (LPARAM)text); - switch(wParam) { case VK_RETURN: /* If quickComplete is set and control is pressed, replace the string */ if (ac->quickComplete && (GetKeyState(VK_CONTROL) & 0x8000)) { - WCHAR *buf; - size_t sz = strlenW(ac->quickComplete) + 1 + len; + WCHAR *text, *buf; + size_t sz; + UINT len = SendMessageW(hwnd, WM_GETTEXTLENGTH, 0, 0); + if (!(text = heap_alloc((len + 1) * sizeof(WCHAR)))) + return 0; + len = SendMessageW(hwnd, WM_GETTEXT, len + 1, (LPARAM)text); + sz = strlenW(ac->quickComplete) + 1 + len; + if ((buf = heap_alloc(sz * sizeof(WCHAR)))) { len = format_quick_complete(buf, ac->quickComplete, text, len); @@ -228,16 +243,16 @@ static LRESULT ACEditSubclassProc_KeyUp(IAutoCompleteImpl *ac, HWND hwnd, UINT u SendMessageW(hwnd, EM_SETSEL, 0, len); heap_free(buf); } + + if (ac->options & ACO_AUTOSUGGEST) + ShowWindow(ac->hwndListBox, SW_HIDE); + heap_free(text); + return 0; }
if (ac->options & ACO_AUTOSUGGEST) ShowWindow(ac->hwndListBox, SW_HIDE); - heap_free(text); - return 0; - case VK_LEFT: - case VK_RIGHT: - heap_free(text); - return 0; + break; case VK_UP: case VK_DOWN: /* Two cases here: @@ -245,19 +260,20 @@ static LRESULT ACEditSubclassProc_KeyUp(IAutoCompleteImpl *ac, HWND hwnd, UINT u set, display it with all the entries, without selecting any - if the listbox is visible, change the selection */ - if ( (ac->options & (ACO_AUTOSUGGEST | ACO_UPDOWNKEYDROPSLIST)) - && (!IsWindowVisible(ac->hwndListBox) && (! *text)) ) + if (!(ac->options & ACO_AUTOSUGGEST)) + break; + + if (!IsWindowVisible(ac->hwndListBox)) { - /* We must display all the entries */ - displayall = TRUE; + if (ac->options & ACO_UPDOWNKEYDROPSLIST) + { + autocomplete_text(ac, hwnd, autoappend_flag_displayempty); + return 0; + } } else { INT count, sel; - heap_free(text); - if (!IsWindowVisible(ac->hwndListBox)) - return 0; - count = SendMessageW(ac->hwndListBox, LB_GETCOUNT, 0, 0);
/* Change the selection */ @@ -290,22 +306,14 @@ static LRESULT ACEditSubclassProc_KeyUp(IAutoCompleteImpl *ac, HWND hwnd, UINT u return 0; } break; - case VK_BACK: case VK_DELETE: - if ((! *text) && (ac->options & ACO_AUTOSUGGEST)) - { - heap_free(text); - ShowWindow(ac->hwndListBox, SW_HIDE); - return CallWindowProcW(ac->wpOrigEditProc, hwnd, uMsg, wParam, lParam); - } - break; + { + LRESULT ret = CallWindowProcW(ac->wpOrigEditProc, hwnd, uMsg, wParam, lParam); + autocomplete_text(ac, hwnd, autoappend_flag_no); + return ret; + } } - - if (len + 1 != size) - text = heap_realloc(text, (len + 1) * sizeof(WCHAR)); - - autocomplete_text(ac, text, len, hwnd, displayall); - return 0; + return CallWindowProcW(ac->wpOrigEditProc, hwnd, uMsg, wParam, lParam); }
/* @@ -314,6 +322,7 @@ static LRESULT ACEditSubclassProc_KeyUp(IAutoCompleteImpl *ac, HWND hwnd, UINT u static LRESULT APIENTRY ACEditSubclassProc(HWND hwnd, UINT uMsg, WPARAM wParam, LPARAM lParam) { IAutoCompleteImpl *This = GetPropW(hwnd, autocomplete_propertyW); + LRESULT ret;
if (!This->enabled) return CallWindowProcW(This->wpOrigEditProc, hwnd, uMsg, wParam, lParam);
@@ -329,8 +338,15 @@ 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: - return ACEditSubclassProc_KeyUp(This, hwnd, uMsg, wParam, lParam); + case WM_KEYDOWN: + return ACEditSubclassProc_KeyDown(This, hwnd, uMsg, wParam, lParam); + case WM_CHAR: + case WM_UNICHAR: + ret = CallWindowProcW(This->wpOrigEditProc, hwnd, uMsg, wParam, lParam); + autocomplete_text(This, hwnd, (This->options & ACO_AUTOAPPEND) && + (wParam >= ' ' || wParam == 0x16 /* ^V (paste) */) + ? autoappend_flag_yes : autoappend_flag_no); + return ret; case WM_DESTROY: { WNDPROC proc = This->wpOrigEditProc;
On Fri, Sep 14, 2018 at 02:00:32PM +0300, Gabriel Ivăncescu wrote:
@@ -314,6 +322,7 @@ static LRESULT ACEditSubclassProc_KeyUp(IAutoCompleteImpl *ac, HWND hwnd, UINT u static LRESULT APIENTRY ACEditSubclassProc(HWND hwnd, UINT uMsg, WPARAM wParam, LPARAM lParam) { IAutoCompleteImpl *This = GetPropW(hwnd, autocomplete_propertyW);
LRESULT ret;
if (!This->enabled) return CallWindowProcW(This->wpOrigEditProc, hwnd, uMsg, wParam, lParam);
@@ -329,8 +338,15 @@ 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:return ACEditSubclassProc_KeyUp(This, hwnd, uMsg, wParam, lParam);
case WM_KEYDOWN:return ACEditSubclassProc_KeyDown(This, hwnd, uMsg, wParam, lParam);case WM_CHAR:case WM_UNICHAR:ret = CallWindowProcW(This->wpOrigEditProc, hwnd, uMsg, wParam, lParam);autocomplete_text(This, hwnd, (This->options & ACO_AUTOAPPEND) &&(wParam >= ' ' || wParam == 0x16 /* ^V (paste) */)? autoappend_flag_yes : autoappend_flag_no);
The condition on flag setting is a hack. For paste you'll likely want a WM_PASTE handler. If pasting didn't work before, then leave it for a later patch, if it did work before this patch you'll have to add such a handler before this one.
Huw.
On Mon, Sep 17, 2018 at 1:10 PM, Huw Davies huw@codeweavers.com wrote:
The condition on flag setting is a hack. For paste you'll likely want a WM_PASTE handler. If pasting didn't work before, then leave it for a later patch, if it did work before this patch you'll have to add such a handler before this one.
Huw.
Well it has to be done this way, because this doesn't handle WM_PASTE (which can also happen with e.g. right-click on edit control and choose Paste), but a straight CTRL+V message, which can be sent by applications like AutoHotkey. This hack is needed to match Windows behavior.
Also the wParam >= ' ' is *very* needed to avoid control characters, *especially* backspace, otherwise you wouldn't be able to delete the last character with a backspace as it would immediately be auto-appended, it doesn't happen in Windows. Same thing with e.g. CTRL+X (Cut) sent via message directly, bypassing WM_CUT. (remember one of my previous band-aids had a hardcoded check for VK_BACK and VK_DELETE, I think handling all control characters is much more elegant and matches Windows, except for CTRL+V).
I'm not sure I can get rid of this hack at all and still guarantee correct behavior. I can, of course, add it as an extra patch following it, and only handle ACO_AUTOAPPEND in this patch. Is that ok? I'll still need the hack though in the next patch in such case, unless you have some better method.
On Mon, Sep 17, 2018 at 02:48:05PM +0300, Gabriel Ivăncescu wrote:
On Mon, Sep 17, 2018 at 1:10 PM, Huw Davies huw@codeweavers.com wrote:
The condition on flag setting is a hack. For paste you'll likely want a WM_PASTE handler. If pasting didn't work before, then leave it for a later patch, if it did work before this patch you'll have to add such a handler before this one.
Huw.
Well it has to be done this way, because this doesn't handle WM_PASTE (which can also happen with e.g. right-click on edit control and choose Paste), but a straight CTRL+V message, which can be sent by applications like AutoHotkey. This hack is needed to match Windows behavior.
On Windows 7 auto-append behaves the same whether paste is driven by a WM_PASTE or a ^V. So you will at least need to handle WM_PASTE, which makes the ^V exclusion dubious.
Also the wParam >= ' ' is *very* needed to avoid control characters, *especially* backspace, otherwise you wouldn't be able to delete the last character with a backspace as it would immediately be auto-appended, it doesn't happen in Windows. Same thing with e.g. CTRL+X (Cut) sent via message directly, bypassing WM_CUT. (remember one of my previous band-aids had a hardcoded check for VK_BACK and VK_DELETE, I think handling all control characters is much more elegant and matches Windows, except for CTRL+V).
Ok, this part does seem to be necessary and if it turns out you do need to keep the ^V exception after handling WM_PASTE, then it'll have to move to a small helper function.
Huw.
On Tue, Sep 18, 2018 at 12:17 PM, Huw Davies huw@codeweavers.com wrote:
On Windows 7 auto-append behaves the same whether paste is driven by a WM_PASTE or a ^V. So you will at least need to handle WM_PASTE, which makes the ^V exclusion dubious.
Yes, I will handle WM_PASTE in the next patch series, I wanted to get these committed first. They're separate fixes, anyway. At least the wParam >= ' ' would have to remain.
Ok, this part does seem to be necessary and if it turns out you do need to keep the ^V exception after handling WM_PASTE, then it'll have to move to a small helper function.
Huw.
I don't think there's a need for a helper function, since the WM_PASTE handler will be very small and basically just a "forward it to the edit control then call autocomplete_text". I've already have it written but I didn't send it because I wanted to have these committed first before I bring new patches in, and besides the previous code did not handle WM_PASTE anyway so it's an extra functionality, not a regression. The reason there's no need for helper function is because this is what it is:
+ case WM_PASTE: + ret = CallWindowProcW(This->wpOrigEditProc, hwnd, uMsg, wParam, lParam); + autocomplete_text(This, hwnd, autoappend_flag_yes); + return ret;
in a later patch.
I guess I can remove the ^V if you really want to and keep the wParam
= ' ', would that be acceptable? I don't know what Windows does if it
hijacks WM_PASTE but not ^V, but honestly it doesn't really matter anymore.
I know I already sent the v7 patch with the ^V split off from patch 2, but you can review or sign-off on the rest of the patches, as they don't depend on it, so I don't have to resend them again. Then I can just resend patch 3 and have it only check for ' ', if you really want to, and get rid of the ^V hack. (WM_PASTE will come later, it's not like it's a big deal to not auto-append on a paste yet!)
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 b9ec228..c7c6240 100644 --- a/dlls/shell32/autocomplete.c +++ b/dlls/shell32/autocomplete.c @@ -126,6 +126,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, enum autoappend_flag flag) { HRESULT hr; @@ -166,12 +188,7 @@ static void autocomplete_text(IAutoCompleteImpl *ac, HWND hwnd, enum autoappend_ { if (cpt == 0 && flag == autoappend_flag_yes) { - 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 c7c6240..547267d 100644 --- a/dlls/shell32/autocomplete.c +++ b/dlls/shell32/autocomplete.c @@ -217,7 +217,7 @@ static void autocomplete_text(IAutoCompleteImpl *ac, HWND hwnd, enum autoappend_ /* 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. (other patches don't depend on this one so it doesn't matter anyway if you feel it's pointless)
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 547267d..188dfb2 100644 --- a/dlls/shell32/autocomplete.c +++ b/dlls/shell32/autocomplete.c @@ -368,8 +368,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)) {
On Fri, Sep 14, 2018 at 02:00:31PM +0300, Gabriel Ivăncescu wrote:
Signed-off-by: Gabriel Ivăncescu gabrielopcode@gmail.com
v6: Also move WM_KEYUP to a separate function.
dlls/shell32/autocomplete.c | 336 ++++++++++++++++++++++++-------------------- 1 file changed, 182 insertions(+), 154 deletions(-)
diff --git a/dlls/shell32/autocomplete.c b/dlls/shell32/autocomplete.c index 9f35ff7..ebbb9ee 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;;)
Could you please add spaces after the 'for' to be consistent with the 'if's.
@@ -128,17 +198,122 @@ static void destroy_autocomplete_object(IAutoCompleteImpl *ac) }
/*
- Helper for ACEditSubclassProc
+*/ +static LRESULT ACEditSubclassProc_KeyUp(IAutoCompleteImpl *ac, HWND hwnd, UINT uMsg,
WPARAM wParam, LPARAM lParam)+{
- WCHAR *text;
- UINT len, size;
- BOOL displayall = FALSE;
- len = SendMessageW(hwnd, WM_GETTEXTLENGTH, 0, 0);
- size = len + 1;
- if (!(text = heap_alloc(size * sizeof(WCHAR))))
return 0;- len = SendMessageW(hwnd, WM_GETTEXT, size, (LPARAM)text);
- switch(wParam)
Likewise after the 'switch'.
Otherwsise, although this wasn't what I'd suggested, it looks ok.
Huw.
-
Gabriel Ivăncescu -
Huw Davies