[PATCH v3] comctl32: Implement handling of EM_SETCUEBANNER/EM_GETCUEBANNER messages.
Sergio Gómez Del Real
sdelreal at codeweavers.com
Tue Oct 23 16:05:27 CDT 2018
On 23/10/18 11:07 a. m., 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.
However, since CUE BANNER only makes sense for single-line controls,
it'd be called only once (when painting the one and only line). For
multi-lines it will never be called. Could we just take advantage of
already having (x,y) and leave the code where it is?
>
>>
>>> @@ -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.
>
>
More information about the wine-devel
mailing list