Re: [v3 04/12] comctl32: Extend TaskDialog, add simple callback and tests

[Nikolay Sivov]

On 10.03.2017 21:21, Fabian Maurer wrote:
> v3: Rewrite to implement Nikolay Sivov's suggestions
> Signed-off-by: Fabian Maurer
>
> dlls/comctl32/taskdialog.c       | 29 +++++++++++++++++++++++--
>  dlls/comctl32/tests/taskdialog.c | 46 +++++++++++++++++++++++++++++++++-------
>  2 files changed, 65 insertions(+), 10 deletions(-)
>
> diff --git a/dlls/comctl32/taskdialog.c b/dlls/comctl32/taskdialog.c
> index 2166ce1898..c5f410716e 100644
> --- a/dlls/comctl32/taskdialog.c
> +++ b/dlls/comctl32/taskdialog.c
> @@ -147,17 +147,41 @@ static void controls_add(struct list *controls, WORD id, const WCHAR *class, con
>      list_add_tail(controls, &data->entry);
>  }
>
> +/* FIXME: Make thread safe */
> +static const TASKDIALOGCONFIG *task_config = 0;
> +static HRESULT callback(HWND hwnd, UINT uNotification, WPARAM wParam, LPARAM lParam)
> +{
> - if(task_config->pfCallback)
> -
>    return task_config->pfCallback(hwnd, uNotification, wParam, lParam, task_config->lpCallbackData);
> - return S_OK;
>
> +}
> -

There's no reason to introduce global variable for that, just to remove
it two patches after.
> static INT_PTR CALLBACK DialogProc(HWND hwndDlg, UINT uMsg, WPARAM wParam, LPARAM lParam)
>  {
> - HRESULT ret_callback;
> - switch (uMsg)
>    {
> -
>    case WM_INITDIALOG:
> -
>        callback(hwndDlg, TDN_DIALOG_CONSTRUCTED, 0, 0);
> -
>        callback(hwndDlg, TDN_CREATED, 0, 0);
> -
>        return TRUE;
>    case WM_COMMAND:
> -
>        if(HIWORD(wParam) == BN_CLICKED && LOWORD(wParam) == IDOK)
> -
>        if(HIWORD(wParam) == BN_CLICKED)
>        {
> -
>            EndDialog(hwndDlg, 0);
> -
>            WORD command_id = LOWORD(wParam);
> -
> -
>            ret_callback = callback(hwndDlg, TDN_BUTTON_CLICKED, command_id, 0);
> -
>            if(ret_callback == S_OK) /* FIXME */
> -
>            {
> -
>                EndDialog(hwndDlg, command_id);
> -
>            }
>            return TRUE;
>        }
>        break;
> -
>    case WM_DESTROY:
> -
>        callback(hwndDlg, TDN_DESTROYED, 0, 0);
> -
>        break;
> }
>    return FALSE;

For minimal working replacement of MessageBox callback support is not
needed, right?