Re: comctl32: Implementation the WM_GETTEXT processing of datetime control

[Nikolay Sivov]

Ilya Shpigor wrote:
> +static INT
> +DATETIME_GetText (DATETIME_INFO *infoPtr, INT count, LPWSTR dst)
> +{
> +    WCHAR buf[80];
> +    int i;
> +
> +    if(!count) return 0;
> +
> +    dst[0] = 0;
> +    for (i = 0; i < infoPtr->nrFields; i++)
> +    {
> +        DATETIME_ReturnTxt(infoPtr, i, buf, sizeof(buf)/sizeof(buf[0]));
> +        if ((strlenW(dst) + strlenW(buf)) <= count)
> +            strcatW(dst, buf);
> +        else break;
> +    }
> +    return strlenW(dst);
> +}

I don't think it's a right way. You probably should use window text 
instead updating it on every change,
see how GetWindowText is implemented.
> @@ -674,6 +675,10 @@ static void test_wm_set_get_text(void)
>      ret = SendMessage(hWnd, WM_GETTEXT, sizeof(buff), (LPARAM)buff);
>      ok(strcmp(buff, a_str) != 0, "Expected text not to change, got %s\n", buff);
>
> +    GetSystemTime(&stime);
> +    sprintf(time, "%d.%d.%d", stime.wDay, stime.wMonth, stime.wYear);
> +    ok(!strcmp(buff, time), "Expected %s, got %s\n", time, buff);
> +
>      DestroyWindow(hWnd);
>  }

Test is definitely wrong. You can't expect DD.MMMM.YYYY pattern here, 
it's locale dependent.

comctl32: Implementation the WM_GETTEXT processing of datetime control

[Ilya Shpigor]

Hi,

Thanks for reply.
> I don't think it's a right way. You probably should use window text
> instead updating it on every change,
> see how GetWindowText is implemented.

GetWindowText work with the server side text buffer. Are you offer to use it? 
I don't sure that control text and this kind of window text are same things. 

Datetime control have own interpretation of data in the self text buffers. 
Using window text will need some synchronization when this data change. Can I 
do it?

-- 
Best regards,
Ilya Shpigor.

comctl32: Implementation the WM_GETTEXT processing of datetime control

[Nikolay Sivov]

Ilya Shpigor wrote:
> GetWindowText work with the server side text buffer. Are you offer to use it? 
> I don't sure that control text and this kind of window text are same things.

Hmm, I'm not sure. Looks like setting text always goes to server call, 
maybe it's the same thing (synced somewhere?)...
Anyway GetWindowText should work, and Wine's implementation uses 
WM_GETTEXT only for same process windows,
so you shouldn't override default handling for this message.
> Datetime control have own interpretation of data in the self text buffers. 
> Using window text will need some synchronization when this data change. Can I 
> do it?

Or course you can, format a string every time contents change and store 
it with SetWindowText(). WM_GETTEXT
will work through default procedure (I've done it that way for IPAddress 
control).

P.S. Also don't forget comments regarding tests part.

Thanks.

comctl32: Implementation the WM_GETTEXT processing of datetime control

[Ilya Shpigor]

> Or course you can, format a string every time contents change and store
> it with SetWindowText(). WM_GETTEXT
> will work through default procedure (I've done it that way for IPAddress
> control).

This is good idea, but SetWindowText just send the WM_SETTEXT message. Your 
patch "comctl32/datetime: Block WM_SETTEXT message" don't allow to do it.
> Anyway GetWindowText should work, and Wine's implementation uses
> WM_GETTEXT only for same process windows,
> so you shouldn't override default handling for this message.

This possible to test: is the datetime window procedure process WM_GETTEXT? 
This message must be send to control directly and be processed through 
CallWindowProc with DefWindowProc parameter. Is this test give right results?
> P.S. Also don't forget comments regarding tests part.

The second try of test have been send.

-- 
Best regards,
Ilya Shpigor.

comctl32: Implementation the WM_GETTEXT processing of datetime control

[Nikolay Sivov]

Ilya Shpigor wrote:
> > Or course you can, format a string every time contents change and store
> > it with SetWindowText(). WM_GETTEXT
> > will work through default procedure (I've done it that way for IPAddress
> > control).
>
> This is good idea, but SetWindowText just send the WM_SETTEXT message. Your 
> patch "comctl32/datetime: Block WM_SETTEXT message" don't allow to do it.

Ah, forgot about it, sorry. Now I think your way is good enough, maybe 
WM_SETTEXT is still forwarded to
default procedure and WM_GETTEXT not, I don't know.
> > Anyway GetWindowText should work, and Wine's implementation uses
> > WM_GETTEXT only for same process windows,
> > so you shouldn't override default handling for this message.
>
> This possible to test: is the datetime window procedure process WM_GETTEXT? 
> This message must be send to control directly and be processed through 
> CallWindowProc with DefWindowProc parameter. Is this test give right results?

It's possible but such test will tell nothing, WM_GETTEXT definitely 
works for this control class and returns
current content. Blocked WM_SETTEXT makes me think that WM_GETTEXT is 
overridden as you've done.

To test that you could subclass control, catch WM_GETTEXT in subclass 
proc and forward it to default procedure directly
bypassing control handling. If it returns nothing or some creation time 
passed text then your patch is ok.

All of that comes to be extra testing not related to directly visible 
control behavior, treat this as additional checks.