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

[Nikolay Sivov]

On 10/22/2018 09:24 PM, Sergio Gómez Del Real wrote:

> Signed-off-by: Sergio Gómez Del Real <sdelreal at codeweavers.com>
> ---
>   dlls/comctl32/edit.c       | 62 ++++++++++++++++++++++++++++++++++++++
>   dlls/comctl32/tests/edit.c | 62 ++++++++++++++++++++++++++++++++++++++
>   2 files changed, 124 insertions(+)
>
> diff --git a/dlls/comctl32/edit.c b/dlls/comctl32/edit.c
> index f0180adfbe..2796e6e53c 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->flags & EF_FOCUSED) && es->cue_banner_text && es->text_length == 0)
> +       {
> +	       SetTextColor(dc, GetSysColor(COLOR_GRAYTEXT));
> +	       TextOutW(dc, x, y, es->cue_banner_text, strlenW(es->cue_banner_text));
> +       }
>   }

Now looking again, it seems it would be better to move this to WM_PAINT, 
because it does not have to be called for every line. Does this 
complicate (x,y) calculation a lot?

>   
>   
> @@ -4152,6 +4159,52 @@ static LRESULT EDIT_EM_GetThumb(EDITSTATE *es)
>                           EDIT_WM_HScroll(es, EM_GETTHUMB, 0));
>   }
>   
> +/*********************************************************************
> + *
> + *	EM_SETCUEBANNER
> + *
> + */
> +static BOOL EDIT_EM_SetCueBanner(EDITSTATE *es, const WCHAR *cue_text)
> +{
> +    SIZE_T str_size;
> +
> +    if (es->style & ES_MULTILINE || !cue_text)
> +        return FALSE;
> +
> +    str_size = strlenW(cue_text);
> +
> +    if (es->cue_banner_text)
> +        heap_free(es->cue_banner_text);
> +
> +    es->cue_banner_text = heap_alloc((str_size + 1) * sizeof(WCHAR));
> +    if (!es->cue_banner_text)
> +        return FALSE;
> +
> +    memcpy(es->cue_banner_text, cue_text, str_size * sizeof(WCHAR));
> +    es->cue_banner_text[str_size] = 0;
> +
> +    return TRUE;
> +}

I think you can reuse heap_strdupW() from propsheet.c for that, first 
moving it to a header, and changing Alloc() -> heap_alloc(). No need to 
null check before releasing old text by the way.

> +
> +/*********************************************************************
> + *
> + *	EM_GETCUEBANNER
> + *
> + */
> +static BOOL EDIT_EM_GetCueBanner(EDITSTATE *es, WCHAR *buf, DWORD size)
> +{
> +    if (!es->cue_banner_text)
> +        return FALSE;
> +    else
> +    {
> +        if (size > strlenW(es->cue_banner_text))
> +            size = strlenW(es->cue_banner_text) + 1;
> +        memcpy(buf, es->cue_banner_text, (size - 1) * sizeof(WCHAR));
> +        buf[size-1] = 0;
> +        return TRUE;
> +    }
> +}
> +

Could you use lstrcpynW() for this? Also it seems inaccurate for null 
text case - for ES_MULTILINE it shouldn't touch output buffer, and for 
single line case it write empty string, that's according to my quick 
testing, we'll need some tests for that. Also is it supposed to crash on 
null destination?

>   
>
> @@ -4703,6 +4757,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;

So does wParam work or not?
>   
> +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];
> +    static WCHAR emptyW[] = {0};
> +
> +    hwnd_edit = create_editcontrolW(ES_AUTOHSCROLL | ES_AUTOVSCROLL, 0);
> +
> +    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, (WPARAM)getcuetestW, 5);
> +    if (ret == FALSE)
> +    {
> +        win_skip("skipping for Win XP and 2003 Server.\n");
> +        DestroyWindow(hwnd_edit);
> +        return;
> +    }

Move this up please. You can test that returned buffer is not null 
terminated as indication of unimplemented message on XP. No reason to 
run other tests before this skip if it's not going to work.