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

Sergio Gómez Del Real sdelreal at codeweavers.com
Tue Oct 23 11:56:49 CDT 2018


On 23/10/18 11:35 a. m., Nikolay Sivov wrote:

>
>
> On 10/23/2018 07:07 PM, Sergio Gómez Del Real wrote:
>> 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.
>
> I mean buff != NULL && size != 0 && cue_banner_text == NULL, what 
> happens to buff.
>
Your are right; it is writing empty string in the case you are 
considering. I'll update tests for this case.

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



More information about the wine-devel mailing list