[PATCH v5 1/7] shell32/autocomplete: Dynamically allocate hwndText so it can handle arbitrary sizes

[Gabriel Ivăncescu]

Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)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;
-- 
1.9.1

[PATCH v5 2/7] shell32/autocomplete: Move the autocomplete processing to a separate function

[Gabriel Ivăncescu]

Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)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:
         {
-- 
1.9.1

[PATCH v5 2/7] shell32/autocomplete: Move the autocomplete processing to a separate function

[Huw Davies]

On Wed, Sep 12, 2018 at 10:42:16PM +0300, Gabriel Ivăncescu wrote:
> Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)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

[PATCH v5 2/7] shell32/autocomplete: Move the autocomplete processing to a separate function

[Gabriel Ivăncescu]

On Thu, Sep 13, 2018 at 1:34 PM, Huw Davies <huw(a)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)

[PATCH v5 3/7] shell32/autocomplete: Redesign the window proc to trigger on key presses instead of key release

[Gabriel Ivăncescu]

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>
---

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;
-- 
1.9.1

[PATCH v5 3/7] shell32/autocomplete: Redesign the window proc to trigger on key presses instead of key release

[Huw Davies]

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(a)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

[PATCH v5 3/7] shell32/autocomplete: Redesign the window proc to trigger on key presses instead of key release

[Gabriel Ivăncescu]

On Thu, Sep 13, 2018 at 1:37 PM, Huw Davies <huw(a)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?

[PATCH v5 3/7] shell32/autocomplete: Redesign the window proc to trigger on key presses instead of key release

[Gabriel Ivăncescu]

On Thu, Sep 13, 2018 at 6:47 PM, Gabriel Ivăncescu
<gabrielopcode(a)gmail.com> wrote:
> On Thu, Sep 13, 2018 at 1:37 PM, Huw Davies <huw(a)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 :-)

[PATCH v5 3/7] shell32/autocomplete: Redesign the window proc to trigger on key presses instead of key release

[Huw Davies]

> On 13 Sep 2018, at 20:12, Gabriel Ivăncescu <gabrielopcode(a)gmail.com> wrote:
>
> On Thu, Sep 13, 2018 at 6:47 PM, Gabriel Ivăncescu
> <gabrielopcode(a)gmail.com> wrote:
> > On Thu, Sep 13, 2018 at 1:37 PM, Huw Davies <huw(a)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.

[PATCH v5 4/7] shell32/autocomplete: Avoid another buffer overflow and handle arbitrary sizes for the auto-append string

[Gabriel Ivăncescu]

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 | 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);
-- 
1.9.1

[PATCH v5 5/7] shell32/autocomplete: Simplify a calculation

[Gabriel Ivăncescu]

Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)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
-- 
1.9.1

[PATCH v5 6/7] shell32/autocomplete: Remove the property after replacing the callback instead of before

[Gabriel Ivăncescu]

Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)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);
         }
-- 
1.9.1

[PATCH v5 7/7] shell32/tests: Send WM_CHAR instead of WM_KEYUP

[Gabriel Ivăncescu]

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

[Huw Davies]

Signed-off-by: Huw Davies <huw(a)codeweavers.com>