[PATCH] comctl32/taskdialog: Add callback and tests
Nikolay Sivov
bunglehead at gmail.com
Wed Aug 2 05:02:11 CDT 2017
On 28.07.2017 19:53, Fabian Maurer wrote:
>
> +static HRESULT callback(struct taskdialog_info *dialog_info, UINT uNotification, WPARAM wParam, LPARAM lParam)
> +{
> + if(dialog_info->callback)
> + return dialog_info->callback(dialog_info->hwnd, uNotification, wParam, lParam, dialog_info->callback_data);
> + return S_OK;
> +}
Please use consistent naming and formatting, like space after 'if' for
example, and uNotification -> notification.
> +
> static INT_PTR CALLBACK taskdialog_proc(HWND hwnd, UINT msg, WPARAM wParam, LPARAM lParam)
> {
> + static const WCHAR taskdialog_info_propnameW[] = {'T','a','s','k','D','i','a','l','o','g','I','n','f','o',0};
> + struct taskdialog_info *dialog_info;
> +
> TRACE("hwnd=%p msg=0x%04x wparam=%lx lparam=%lx\n", hwnd, msg, wParam, lParam);
>
> + if(msg != WM_INITDIALOG && msg != WM_NCDESTROY)
> + dialog_info = GetPropW(hwnd, taskdialog_info_propnameW);
> +
Why WM_NCDESTROY is a special case?
> switch (msg)
> {
> + case WM_INITDIALOG:
> + dialog_info = (struct taskdialog_info *)lParam;
> + dialog_info->hwnd = hwnd;
> + SetPropW(hwnd, taskdialog_info_propnameW, dialog_info);
> +
> + callback(dialog_info, TDN_DIALOG_CONSTRUCTED, 0, 0);
> + callback(dialog_info, TDN_CREATED, 0, 0);
> + break;
This looks suspicious, documentation is not strict on this, but it seems
TDN_CREATED corresponds for to WM_SHOWWINDOW stage.
> case WM_COMMAND:
> if (HIWORD(wParam) == BN_CLICKED)
> {
> WORD command_id = LOWORD(wParam);
> - EndDialog(hwnd, command_id);
> + click_button(dialog_info, command_id);
> return TRUE;
> }
> break;
> + case WM_DESTROY:
> + callback(dialog_info, TDN_DESTROYED, 0, 0);
> + RemovePropW(hwnd, taskdialog_info_propnameW);
> + break;
> +
> + /* Custom messages*/
What do you mean by "custom"?
> +
> + case TDM_CLICK_BUTTON:
> + click_button(dialog_info, wParam);
> + break;
> }
> return FALSE;
> }
> @@ -507,17 +556,24 @@ HRESULT WINAPI TaskDialogIndirect(const TASKDIALOGCONFIG *taskconfig, int *butto
> int *radio_button, BOOL *verification_flag_checked)
> {
> DLGTEMPLATE *template;
> + struct taskdialog_info dialog_info;
> INT ret;
>
> TRACE("%p, %p, %p, %p\n", taskconfig, button, radio_button, verification_flag_checked);
>
> + if(!taskconfig || taskconfig->cbSize != sizeof(TASKDIALOGCONFIG))
> + return E_INVALIDARG;
Tests you added only check cbSize == 0 case.
> +
> + dialog_info.callback = taskconfig->pfCallback;
> + dialog_info.callback_data = taskconfig->lpCallbackData;
> +
> template = create_taskdialog_template(taskconfig);
> - ret = DialogBoxIndirectParamW(taskconfig->hInstance, template, taskconfig->hwndParent, taskdialog_proc, 0);
> + ret = DialogBoxIndirectParamW(taskconfig->hInstance, template, taskconfig->hwndParent, taskdialog_proc, (LPARAM)&dialog_info);
> Free(template);
>
> if (button) *button = ret;
> if (radio_button) *radio_button = taskconfig->nDefaultButton;
> - if (verification_flag_checked) *verification_flag_checked = TRUE;
> + if (verification_flag_checked) *verification_flag_checked = FALSE;
Why do you need this flag value change?
>
> 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.
> +
> +/* Create a message to test against */
> +static struct message create_test_message(UINT message, WPARAM wParam, LPARAM lParam)
> +{
> + struct message mes;
> +
> + mes.message = WM_TD_CALLBACK;
> + mes.id = message;
> + mes.flags = sent|wparam|lparam|id;
> + mes.wParam = wParam;
> + mes.lParam = lParam;
> +
> + return mes;
> +}
> +
> +/* Our only way to get a button handle, since GetDlgItem and FindWindowEx don't work for the official taskdialog */
> +
> +static HWND taskdialog_child;
> +BOOL CALLBACK enum_taskdialog_children_proc(HWND hwnd, LPARAM lParam)
> +{
> + CHAR text[100];
> + const CHAR *title = (const CHAR *)lParam;
> +
> + GetWindowTextA(hwnd, text, sizeof(text));
> +
> + if(lstrcmpA(text, title) == 0)
> + {
> + taskdialog_child = hwnd;
> + return FALSE;
> + }
> +
> + return TRUE;
> +}
> +
> +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.
>
> + init_msg_sequences(sequences, NUM_MSG_SEQUENCES);
> + test_TaskDialogIndirect();
This name is too generic, in a test function like that you can test
invalid parameter handling for example. Then have another function for
callback notifications, and another one for TDM_* messages.
I think for first iteration test should simply check
creation/destruction notifications and callback user data value.
More information about the wine-devel
mailing list