Re: [PATCH] comctl32/taskdialog: Add callback and tests

[Fabian Maurer]

Hello,

sorry I thought I already sent this, but it apparently didn't work.
> This looks suspicious, documentation is not strict on this, but it seems
> TDN_CREATED corresponds for to WM_SHOWWINDOW stage.

I'm not sure, I'll look into it.
> > -
> >    /* Custom messages*/
>
> What do you mean by "custom"?

I meant taskdialog specific messages. Guess I'll change it to "Taskdialog 
messages"
> > - if (verification_flag_checked) *verification_flag_checked = TRUE;
> > - if (verification_flag_checked) *verification_flag_checked = FALSE;
>
> Why do you need this flag value change?

Because that's what the tests told me windows behaves like.
> >  return S_OK;
> >
> > }
> >
> > diff --git a/dlls/comctl32/tests/taskdialog.c
> > b/dlls/comctl32/tests/taskdialog.c index c937127897..867361617c 100644
> > --- a/dlls/comctl32/tests/taskdialog.c
> > +++ b/dlls/comctl32/tests/taskdialog.c
> > @@ -24,13 +24,250 @@
> >
> > #include "winuser.h"
> >  #include "commctrl.h"
> >
> > +#include "wine/list.h"
> >
> > #include "wine/test.h"
> >  #include "v6util.h"
> >
> > +#include "msg.h"
> > -
> >
> > +#define WM_TD_CALLBACK (WM_APP) /* Custom dummy message to wrap callback
> > notifications */ +
> > +#define NUM_MSG_SEQUENCES     1
> > +#define TASKDIALOG_SEQ_INDEX  0
> > -
> >
> > +#define TEST_NUM_BUTTONS 20 /* Number of custom buttons to test with */
> > -
> >
> > +#define ID_START 20 /* Lower IDs might be used by the system */
> > +#define ID_START_BUTTON (ID_START + 0)
> >
> > static HRESULT (WINAPI *pTaskDialogIndirect)(const TASKDIALOGCONFIG *,
> >  int *, int *, BOOL *); static HRESULT (WINAPI *pTaskDialog)(HWND,
> >  HINSTANCE, const WCHAR *, const WCHAR *, const WCHAR *,>  
> >          TASKDIALOG_COMMON_BUTTON_FLAGS, const WCHAR *, int *);
> >
> > +static struct msg_sequence *sequences[NUM_MSG_SEQUENCES];
> > -
> >
> > +/* Message lists to test against */
> > -
> >
> > +struct message_send_info
> > +{
> > - UINT send_message;
> > - WPARAM send_wparam;
> > - LPARAM send_lparam;
> > -
> > - BOOL post; /* post instead of send */
> > - const CHAR *title_target; /* control text, 0 means it's send to the
> >
> > dialog form instead */ +};
> > -
> >
> > +struct message_info
> > +{
> > - UINT recv_message; /* Message the callback receives */
> > - WPARAM recv_wparam;
> > - LPARAM recv_lparam;
> > -
> > - HRESULT ret; /* Value the callback should return */
> > -
> > - struct message_send_info send[9];  /* Message to send to trigger the
> >
> > next callback message */ +};
> > -
> >
> > +static const struct message_info *current_message_info;
> > -
> >
> > +static const struct message_info mes_return_press_ok[] = {
> > - { TDN_CREATED, 0, 0, S_OK, {
> > -
> >    { WM_KEYDOWN, VK_RETURN, 0, TRUE },
> > -
> >    { 0 }}},
> > - { TDN_BUTTON_CLICKED, IDOK, 0, S_OK, {{ 0 }}},
> > - { 0 }
> >
> > +};
> > -
> >
> > +static const struct message_info mes_cancel_button_press[] = {
> > - { TDN_CREATED, 0, 0, S_OK, {
> > -
> >    { TDM_CLICK_BUTTON, IDOK, 0, TRUE },
> > -
> >    { TDM_CLICK_BUTTON, IDOK, 0, TRUE },
> > -
> >    { TDM_CLICK_BUTTON, IDOK, 0, TRUE },
> > -
> >    { 0 }}},
> > - { TDN_BUTTON_CLICKED, IDOK, 0, S_FALSE, {{ 0 }}},
> > - { TDN_BUTTON_CLICKED, IDOK, 0, 0xFF, {{ 0 }}}, /* Random return value
> >
> > tested here */ +    { TDN_BUTTON_CLICKED, IDOK, 0, S_OK, {{ 0 }}},
> > - { 0 }
> >
> > +};
> > -
> >
> > +static const struct message_info mes_send_button_directly[] = {
> > - { TDN_CREATED, 0, 0, S_OK, {
> > -
> >    { WM_LBUTTONDOWN, MK_LBUTTON, 0, TRUE, "02" },
> > -
> >    { WM_LBUTTONUP,   0, 0, TRUE, "02" },
> > -
> >    { 0 }}},
> > - { TDN_BUTTON_CLICKED, ID_START + 2, 0, S_OK, {{ 0 }}},
> > - { 0 }
> >
> > +};
>
> This looks too complicated, I'd just check notifications callback
> received, using 'struct message' data, same way WM_NOTIFY tests work.

I have more tests currently available, like cancelling tests. They're not part 
of this patch, but I figured I'll do it right from the beginning.
> > +static HWND get_child_from_title(HWND hwnd_parent, const CHAR *title)
> > +{
> > - taskdialog_child = NULL;
> > - EnumChildWindows(hwnd_parent, enum_taskdialog_children_proc,
> >
> > (LPARAM)title); +    return taskdialog_child;
> > +}
>
> I don't think we need to get that deep.

Are you sure? I figured it would make sense to actually test for simulated 
mouse clicks instead of just sending the taskdialog specific click messages.
> I think for first iteration test should simply check
> creation/destruction notifications and callback user data value.

I'll take care of the issues and resend the patch as simplified version.

Regards
Fabian Maurer