[PATCH v4] comctl32: Implement handling of EM_SETCUEBANNER/EM_GETCUEBANNER messages.

[Sergio Gómez Del Real]

Signed-off-by: Sergio Gómez Del Real <sdelreal(a)codeweavers.com>
---
 dlls/comctl32/edit.c       | 65 +++++++++++++++++++++++++++++++++++
 dlls/comctl32/tests/edit.c | 69 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 134 insertions(+)

diff --git a/dlls/comctl32/edit.c b/dlls/comctl32/edit.c
index f0180adfbe..3da7b98491 100644
--- a/dlls/comctl32/edit.c
+++ b/dlls/comctl32/edit.c
@@ -131,6 +131,7 @@ typedef struct
 					   should be sent to the first parent. */
 	HWND hwndListBox;		/* handle of ComboBox's listbox or NULL */
 	INT wheelDeltaRemainder;        /* scroll wheel delta left over after scrolling whole lines */
+       WCHAR *cue_banner_text;
 	/*
 	 *	only for multi line controls
 	 */
@@ -2181,6 +2182,12 @@ static void EDIT_PaintLine(EDITSTATE *es, HDC dc, INT line, BOOL rev)
 		x += EDIT_PaintText(es, dc, x, y, line, e - li, li + ll - e, FALSE);
 	} else
 		x += EDIT_PaintText(es, dc, x, y, line, 0, ll, FALSE);
+
+       if (es->cue_banner_text && es->text_length == 0 && !(es->flags & EF_FOCUSED))
+       {
+	       SetTextColor(dc, GetSysColor(COLOR_GRAYTEXT));
+	       TextOutW(dc, x, y, es->cue_banner_text, strlenW(es->cue_banner_text));
+       }
 }
 
 
@@ -4152,6 +4159,55 @@ static LRESULT EDIT_EM_GetThumb(EDITSTATE *es)
                         EDIT_WM_HScroll(es, EM_GETTHUMB, 0));
 }
 
+static inline WCHAR *heap_strdupW(const WCHAR *str)
+{
+    int len = strlenW(str) + 1;
+    WCHAR *ret = heap_alloc(len * sizeof(WCHAR));
+    lstrcpynW(ret, str, len);
+    return ret;
+}
+
+/*********************************************************************
+ *
+ *	EM_SETCUEBANNER
+ *
+ */
+static BOOL EDIT_EM_SetCueBanner(EDITSTATE *es, const WCHAR *cue_text)
+{
+    if (es->style & ES_MULTILINE || !cue_text)
+        return FALSE;
+
+    heap_free(es->cue_banner_text);
+    es->cue_banner_text = heap_strdupW(cue_text);
+
+    return TRUE;
+}
+
+/*********************************************************************
+ *
+ *	EM_GETCUEBANNER
+ *
+ */
+static BOOL EDIT_EM_GetCueBanner(EDITSTATE *es, WCHAR *buf, DWORD size)
+{
+    if (!es->cue_banner_text)
+    {
+        if (!(es->style & ES_MULTILINE) && buf && size)
+            *buf = 0;
+        return FALSE;
+    }
+    else
+    {
+        if (buf && size)
+        {
+            if (size > strlenW(es->cue_banner_text))
+                size = strlenW(es->cue_banner_text) + 1;
+            lstrcpynW(buf, es->cue_banner_text, size);
+        }
+        return TRUE;
+    }
+}
+
 
 /********************************************************************
  *
@@ -4489,6 +4545,7 @@ static LRESULT EDIT_WM_NCDestroy(EDITSTATE *es)
 
     SetWindowLongPtrW( es->hwndSelf, 0, 0 );
     heap_free(es->undo_text);
+    heap_free(es->cue_banner_text);
     heap_free(es);
 
     return 0;
@@ -4703,6 +4760,14 @@ static LRESULT CALLBACK EDIT_WindowProc(HWND hwnd, UINT msg, WPARAM wParam, LPAR
         result = EDIT_EM_CharFromPos(es, (short)LOWORD(lParam), (short)HIWORD(lParam));
         break;
 
+    case EM_SETCUEBANNER:
+        result = EDIT_EM_SetCueBanner(es, (const WCHAR *)lParam);
+        break;
+
+    case EM_GETCUEBANNER:
+        result = EDIT_EM_GetCueBanner(es, (WCHAR *)wParam, (DWORD)lParam);
+        break;
+
     /* End of the EM_ messages which were in numerical order; what order
      * are these in?  vaguely alphabetical?
      */
diff --git a/dlls/comctl32/tests/edit.c b/dlls/comctl32/tests/edit.c
index a5097d1cbb..0d24ca7dcf 100644
--- a/dlls/comctl32/tests/edit.c
+++ b/dlls/comctl32/tests/edit.c
@@ -3061,6 +3061,74 @@ static const struct message killfocus_combined_seq[] =
     { 0 }
 };
 
+static void test_cue_banner(void)
+{
+    HWND hwnd_edit;
+    BOOL ret;
+    static WCHAR testW[] = {'T','e','s','t',0};
+    static WCHAR testcmp1W[] = {'T','e','s','t',0};
+    static WCHAR testcmp2W[] = {'T','e','s',0};
+    static WCHAR getcuetestW[5] = {'T',0};
+    static WCHAR emptyW[] = {0};
+
+    hwnd_edit = create_editcontrolW(ES_AUTOHSCROLL | ES_AUTOVSCROLL, 0);
+
+    ret = SendMessageW(hwnd_edit, EM_GETCUEBANNER, (WPARAM)getcuetestW, 5);
+    if (lstrcmpW(getcuetestW, emptyW) != 0)
+    {
+        win_skip("skipping for Win XP and 2003 Server.\n");
+        DestroyWindow(hwnd_edit);
+        return;
+    }
+    ok(lstrcmpW(getcuetestW, emptyW) == 0, "First char is %c\n", getcuetestW[0]);
+    ok(ret == FALSE, "EM_GETCUEBANNER should have returned FALSE.\n");
+
+    ret = SendMessageW(hwnd_edit, EM_GETCUEBANNER, 0, 0);
+    ok(ret == FALSE, "EM_GETCUEBANNER should have returned FALSE.\n");
+
+    ret = SendMessageW(hwnd_edit, EM_SETCUEBANNER, 0, 0);
+    ok(ret == FALSE, "EM_SETCUEBANNER should have returned FALSE.\n");
+
+    ret = SendMessageW(hwnd_edit, EM_GETCUEBANNER, 0, 0);
+    ok(ret == FALSE, "EM_GETCUEBANNER should have returned FALSE.\n");
+
+    ret = SendMessageW(hwnd_edit, EM_SETCUEBANNER, 0, (LPARAM)testW);
+    ok(ret == TRUE, "EM_SETCUEBANNER should have returned TRUE.\n");
+
+    ret = SendMessageW(hwnd_edit, EM_GETCUEBANNER, 0, 5);
+    ok(ret == TRUE, "EM_GETCUEBANNER should have returned FALSE.\n");
+
+    ret = SendMessageW(hwnd_edit, EM_GETCUEBANNER, (WPARAM)getcuetestW, 5);
+    ok(ret == TRUE, "EM_GETCUEBANNER should have returned TRUE.\n");
+    ok(lstrcmpW(getcuetestW, testW) == 0, "EM_GETCUEBANNER returned string %s.\n", wine_dbgstr_w(getcuetestW));
+
+    ret = SendMessageW(hwnd_edit, EM_SETCUEBANNER, 0, (LPARAM)emptyW);
+    ok(ret == TRUE, "EM_SETCUEBANNER should have returned TRUE.\n");
+
+    ret = SendMessageW(hwnd_edit, EM_GETCUEBANNER, (WPARAM)getcuetestW, 5);
+    ok(ret == TRUE, "EM_GETCUEBANNER should have returned TRUE.\n");
+    ok(lstrcmpW(getcuetestW, emptyW) == 0, "EM_GETCUEBANNER returned string %s.\n", wine_dbgstr_w(getcuetestW));
+
+    /* EM_GETCUEBANNER's buffer size includes null char */
+    ret = SendMessageW(hwnd_edit, EM_SETCUEBANNER, 0, (LPARAM)testW);
+    ok(ret == TRUE, "EM_SETCUEBANNER should have returned TRUE.\n");
+    memset(testW, 0, lstrlenW(testcmp1W)*sizeof(WCHAR));
+    ret = SendMessageW(hwnd_edit, EM_GETCUEBANNER, (WPARAM)testW, (LPARAM)lstrlenW(testcmp1W)+1);
+    ok(ret == TRUE, "EM_GETCUEBANNER should have returned TRUE.\n");
+    ok(lstrcmpW(testW, testcmp1W) == 0, "EM_GETCUEBANNER returned string %s.\n", wine_dbgstr_w(testW));
+    memset(testW, 0, lstrlenW(testcmp1W)*sizeof(WCHAR));
+    ret = SendMessageW(hwnd_edit, EM_GETCUEBANNER, (WPARAM)testW, (LPARAM)lstrlenW(testcmp1W));
+    ok(lstrcmpW(testW, testcmp2W) == 0, "EM_GETCUEBANNER returned string %s.\n", wine_dbgstr_w(testW));
+    DestroyWindow(hwnd_edit);
+
+    /* setting cue banner fails for multi-line edit controls */
+    hwnd_edit = create_editcontrolW(ES_AUTOHSCROLL | ES_AUTOVSCROLL | ES_MULTILINE, 0);
+    ret = SendMessageW(hwnd_edit, EM_SETCUEBANNER, 0, (LPARAM)testW);
+    ok(ret == FALSE, "EM_SETCUEBANNER.\n");
+
+    DestroyWindow(hwnd_edit);
+}
+
 static void test_change_focus(void)
 {
     HWND hwnd, parent_wnd;
@@ -3138,6 +3206,7 @@ START_TEST(edit)
     test_EM_GETLINE();
     test_wordbreak_proc();
     test_change_focus();
+    test_cue_banner();
 
     UnregisterWindowClasses();
 
-- 
2.17.1

[Nikolay Sivov]

On 10/24/2018 07:02 PM, Sergio Gómez Del Real wrote:
> +static inline WCHAR *heap_strdupW(const WCHAR *str)
> +{
> +    int len = strlenW(str) + 1;
> +    WCHAR *ret = heap_alloc(len * sizeof(WCHAR));
> +    lstrcpynW(ret, str, len);
> +    return ret;
> +}
> Now there will be two different functions under the same name (and you 
> don't need lstrcpynW if you know the length).
> +    if (!es->cue_banner_text)
> +    {
> +        if (!(es->style & ES_MULTILINE) && buf && size)
> +            *buf = 0;
> +        return FALSE;
> +    }
> I don't think tests cover that.
> +        if (buf && size)
> +        {
> +            if (size > strlenW(es->cue_banner_text))
> +                size = strlenW(es->cue_banner_text) + 1;
> +            lstrcpynW(buf, es->cue_banner_text, size);
> +        }
> Why this size fixup is necessary?
> +static void test_cue_banner(void)
> +{
> +    HWND hwnd_edit;
> +    BOOL ret;
> +    static WCHAR testW[] = {'T','e','s','t',0};
> +    static WCHAR testcmp1W[] = {'T','e','s','t',0};
> +    static WCHAR testcmp2W[] = {'T','e','s',0};
> +    static WCHAR getcuetestW[5] = {'T',0};
> +    static WCHAR emptyW[] = {0};
> One constant for text and one buffer to get it back is enough.
> +    ret = SendMessageW(hwnd_edit, EM_GETCUEBANNER, 0, 5);
> +    ok(ret == TRUE, "EM_GETCUEBANNER should have returned FALSE.\n");
> Typo.

[Sergio Gómez Del Real]

On 24/10/18 11:36 a. m., Nikolay Sivov wrote:
> On 10/24/2018 07:02 PM, Sergio Gómez Del Real wrote:
> > +static inline WCHAR *heap_strdupW(const WCHAR *str)
> > +{
> > +    int len = strlenW(str) + 1;
> > +    WCHAR *ret = heap_alloc(len * sizeof(WCHAR));
> > +    lstrcpynW(ret, str, len);
> > +    return ret;
> > +}
> > Now there will be two different functions under the same name (and you 
> > don't need lstrcpynW if you know the length).

There is a lot of duplication of this simple function; I found it 
copy-pasted in at least 10 places. Ideally that function should be in 
just on place, but I didn't find any gain in moving it to a header for 
just this case while it is being duplicated 10 times in other places.

What do you mean that I don't need lstrcpynW if I know the length? I use 
this function because it copies the null char.
> > +    if (!es->cue_banner_text)
> > +    {
> > +        if (!(es->style & ES_MULTILINE) && buf && size)
> > +            *buf = 0;
> > +        return FALSE;
> > +    }
> > I don't think tests cover that.

I added a test (line 3083 in the tests) for this case specifically.
> > +        if (buf && size)
> > +        {
> > +            if (size > strlenW(es->cue_banner_text))
> > +                size = strlenW(es->cue_banner_text) + 1;
> > +            lstrcpynW(buf, es->cue_banner_text, size);
> > +        }
> > Why this size fixup is necessary?

We could get a size for a buffer greater than the length of the banner 
text. It should be sized so we don't copy more than necessary.
> > +static void test_cue_banner(void)
> > +{
> > +    HWND hwnd_edit;
> > +    BOOL ret;
> > +    static WCHAR testW[] = {'T','e','s','t',0};
> > +    static WCHAR testcmp1W[] = {'T','e','s','t',0};
> > +    static WCHAR testcmp2W[] = {'T','e','s',0};
> > +    static WCHAR getcuetestW[5] = {'T',0};
> > +    static WCHAR emptyW[] = {0};
> > One constant for text and one buffer to get it back is enough.

The thing here is that I use testW in various ways; I memset it to 0 to 
test against testcmp1W and testcmp2W. getcuetestW is user for other 
cases. The reason for various strings is that they get modified in 
GETCUEBANNER, so I think it is easier just to use various. I could see 
how I could simplify it though.
> > +    ret = SendMessageW(hwnd_edit, EM_GETCUEBANNER, 0, 5);
> > +    ok(ret == TRUE, "EM_GETCUEBANNER should have returned FALSE.\n");
> > Typo.

Ok.

[Nikolay Sivov]

On 10/24/2018 07:51 PM, Sergio Gómez Del Real wrote:
> On 24/10/18 11:36 a. m., Nikolay Sivov wrote:
> > On 10/24/2018 07:02 PM, Sergio Gómez Del Real wrote:
> > > +static inline WCHAR *heap_strdupW(const WCHAR *str)
> > > +{
> > > +    int len = strlenW(str) + 1;
> > > +    WCHAR *ret = heap_alloc(len * sizeof(WCHAR));
> > > +    lstrcpynW(ret, str, len);
> > > +    return ret;
> > > +}
> > > Now there will be two different functions under the same name (and 
> > > you don't need lstrcpynW if you know the length).
>
> There is a lot of duplication of this simple function; I found it 
> copy-pasted in at least 10 places. Ideally that function should be in 
> just on place, but I didn't find any gain in moving it to a header for 
> just this case while it is being duplicated 10 times in other places.
>
> What do you mean that I don't need lstrcpynW if I know the length? I 
> use this function because it copies the null char.

So does strcpy, or memcpy.
> > > +    if (!es->cue_banner_text)
> > > +    {
> > > +        if (!(es->style & ES_MULTILINE) && buf && size)
> > > +            *buf = 0;
> > > +        return FALSE;
> > > +    }
> > > I don't think tests cover that.
>
> I added a test (line 3083 in the tests) for this case specifically.

There is no tests for buf != NULL && size == 0, or for multiline style.
> > > +        if (buf && size)
> > > +        {
> > > +            if (size > strlenW(es->cue_banner_text))
> > > +                size = strlenW(es->cue_banner_text) + 1;
> > > +            lstrcpynW(buf, es->cue_banner_text, size);
> > > +        }
> > > Why this size fixup is necessary?
>
> We could get a size for a buffer greater than the length of the banner 
> text. It should be sized so we don't copy more than necessary.

I think lstrcpynW is doing just that.

[Sergio Gómez Del Real]

On 24/10/18 12:39 p. m., Nikolay Sivov wrote:
> On 10/24/2018 07:51 PM, Sergio Gómez Del Real wrote:
> > On 24/10/18 11:36 a. m., Nikolay Sivov wrote:
> > > On 10/24/2018 07:02 PM, Sergio Gómez Del Real wrote:
> > > > +static inline WCHAR *heap_strdupW(const WCHAR *str)
> > > > +{
> > > > +    int len = strlenW(str) + 1;
> > > > +    WCHAR *ret = heap_alloc(len * sizeof(WCHAR));
> > > > +    lstrcpynW(ret, str, len);
> > > > +    return ret;
> > > > +}
> > > > Now there will be two different functions under the same name (and 
> > > > you don't need lstrcpynW if you know the length).
> >
> > There is a lot of duplication of this simple function; I found it 
> > copy-pasted in at least 10 places. Ideally that function should be in 
> > just on place, but I didn't find any gain in moving it to a header 
> > for just this case while it is being duplicated 10 times in other 
> > places.
> >
> > What do you mean that I don't need lstrcpynW if I know the length? I 
> > use this function because it copies the null char.
>
> So does strcpy, or memcpy.

Ok.
> > > +    if (!es->cue_banner_text)
> > > +    {
> > > +        if (!(es->style & ES_MULTILINE) && buf && size)
> > > +            *buf = 0;
> > > +        return FALSE;
> > > +    }
> > > I don't think tests cover that.
>
> I added a test (line 3083 in the tests) for this case specifically.
>
> There is no tests for buf != NULL && size == 0, or for multiline style.

Ok, I'll add tests for those cases.
> > > > +        if (buf && size)
> > > > +        {
> > > > +            if (size > strlenW(es->cue_banner_text))
> > > > +                size = strlenW(es->cue_banner_text) + 1;
> > > > +            lstrcpynW(buf, es->cue_banner_text, size);
> > > > +        }
> > > > Why this size fixup is necessary?
> >
> > We could get a size for a buffer greater than the length of the 
> > banner text. It should be sized so we don't copy more than necessary.
>
> I think lstrcpynW is doing just that.
> Ok.