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

[Sergio Gómez Del Real]

On 23/10/18 1:47 a. m., Nikolay Sivov wrote:

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


I'll look into this. Hopefully retrieving (x,y) won't be complicated.

>
>>     @@ -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.
>
Ok.

>> +
>> +/*********************************************************************
>> + *
>> + *    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?
>
I'm not sure I understand. For "null text case" this will always return 
FALSE. If the control has ES_MULTILINE, then es->cue_banner_text will be 
null, so it won't touch output buffer. I included tests to Set/Get the 
empty string.

It shouldn't crash; I'll fix that.

>>
>> @@ -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?
It doesn't; it is always ignored on Windows. I included it in the 
previous version just in case, but I think it is best to remove it.
>>   +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.
>
Ok.