New subject: [PATCH] comctl32/taskdialog: Add callback and tests

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

    

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

Nikolay Sivov

Fabian Maurer

tags

participants (2)