Re: user32/tests: Add some tests for dialog owner disabled state. (v3)

[Jacek Caban]

Hi Dmitry,

On 04/06/16 13:15, Dmitry Timoshkov wrote:
> Signed-off-by: Dmitry Timoshkov
>
> dlls/user32/tests/win.c | 98 ++++++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 96 insertions(+), 2 deletions(-)
>
> diff --git a/dlls/user32/tests/win.c b/dlls/user32/tests/win.c
> index 957e8a1d..528669e 100644
> --- a/dlls/user32/tests/win.c
> +++ b/dlls/user32/tests/win.c
> @@ -4258,6 +4258,78 @@ static INT_PTR WINAPI empty_dlg_proc(HWND hwnd, UINT msg, WPARAM wparam, LPARAM
>      return 0;
>  }
>
> +static INT_PTR WINAPI empty_dlg_proc3(HWND hwnd, UINT msg, WPARAM wparam, LPARAM lparam)
> +{
> - if (msg == WM_INITDIALOG)
> -
>    EndDialog(hwnd, 0);
> -
> - return 0;
>
> +}
> -
>
> +struct dialog_param
> +{
> - HWND parent, grand_parent;
> - DLGTEMPLATE *dlg_data;
>
> +};
> -
>
> +static INT_PTR WINAPI empty_dlg_proc2(HWND hwnd, UINT msg, WPARAM wparam, LPARAM lparam)
> +{
> - if (msg == WM_INITDIALOG)
> - {
> -
>    DWORD style = GetWindowLongA(hwnd, GWL_STYLE);
> -
>    struct dialog_param *param = (struct dialog_param *)lparam;
> -
>    BOOL parent_is_child;
> -
>    HWND disabled_hwnd;
> -
> -
>    parent_is_child = (GetWindowLongA(param->parent, GWL_STYLE) & (WS_POPUP | WS_CHILD)) == WS_CHILD;
> -
> -
>    ok(IsWindowEnabled(hwnd), "wrong state for %p\n", hwnd);
> -
>    if (parent_is_child)
> -
>    {
> -
>        ok(IsWindowEnabled(param->parent), "wrong state for %08x\n", style);
> -
>        disabled_hwnd = param->grand_parent;
> -
>    }
> -
>    else
> -
>    {
> -
>        ok(!IsWindowEnabled(param->parent), "wrong state for %08x\n", style);
> -
>        disabled_hwnd = param->parent;
> -
>    }
> -
> -
>    if (param->grand_parent)
> -
>    {
> -
>        if (parent_is_child)
> -
>            ok(!IsWindowEnabled(param->grand_parent), "wrong state for %08x\n", style);
> -
>        else
> -
>            ok(IsWindowEnabled(param->grand_parent), "wrong state for %08x\n", style);
> -
>    }
> -
> -
>    DialogBoxIndirectParamA(GetModuleHandleA(NULL), param->dlg_data, disabled_hwnd, empty_dlg_proc3, 0);
> -
>    todo_wine_if ((style & (WS_CHILD|WS_POPUP)) == WS_CHILD)
> -
>    ok(IsWindowEnabled(disabled_hwnd), "wrong state for %08x\n", style);

If you ensured that disabled_hwnd is enabled here, then testing it
later, after dialog is done, has very little value. Maybe you'd disable
it by EnableWindow() call here?
> -
> -
>    ok(IsWindowEnabled(hwnd), "wrong state for %p\n", hwnd);
> -
>    ok(IsWindowEnabled(param->parent), "wrong state for %p\n", param->parent);
> -
>    if (param->grand_parent)
> -
>    todo_wine_if ((style & (WS_CHILD|WS_POPUP)) == WS_CHILD)
> -
>        ok(IsWindowEnabled(param->grand_parent), "wrong state for %p (%08x)\n", param->grand_parent, style);
> -
> -
>    DialogBoxIndirectParamA(GetModuleHandleA(NULL), param->dlg_data, hwnd, empty_dlg_proc3, 0);
> -
>    ok(IsWindowEnabled(hwnd), "wrong state for %p\n", hwnd);
> -
>    ok(IsWindowEnabled(param->parent), "wrong state for %p\n", param->parent);
> -
>    if (param->grand_parent)
> -
>    todo_wine_if ((style & (WS_CHILD|WS_POPUP)) == WS_CHILD)
> -
>        ok(IsWindowEnabled(param->grand_parent), "wrong state for %p (%08x)\n", param->grand_parent, style);

Why do you put those tests inside WM_INITDIALOG? Wouldn't separated
tests be cleaner?
> -
>    param->dlg_data->style |= WS_CHILD;
> -
>    DialogBoxIndirectParamA(GetModuleHandleA(NULL), param->dlg_data, hwnd, empty_dlg_proc3, 0);
> -
>    todo_wine_if (!(style & (WS_CHILD|WS_POPUP)))
> -
>    ok(IsWindowEnabled(hwnd), "wrong state for %p (%08x)\n", hwnd, style);
> -
> -
>    EndDialog(hwnd, 0);

Testing that EndDialog enables window would be nice (given that it's
disabled as I said above).
> - }
> - return 0;
>
> +}
> -
>
> static void check_dialog_style(DWORD style_in, DWORD ex_style_in, DWORD style_out, DWORD ex_style_out)
>  {
>      struct
> @@ -4269,11 +4341,19 @@ static void check_dialog_style(DWORD style_in, DWORD ex_style_in, DWORD style_ou
>          WCHAR caption[1];
>      } dlg_data;
>      DWORD style, ex_style;
> - HWND hwnd, parent = 0;
> -
>
> HWND hwnd, grand_parent = 0, parent = 0;
> -
>
> struct dialog_param param;
>
> if (style_in & WS_CHILD)
> -
>    parent = CreateWindowExA(0, "static", NULL, WS_OVERLAPPEDWINDOW,
> - {
> -
>    grand_parent = CreateWindowExA(0, "static", NULL, WS_OVERLAPPEDWINDOW,
>                            0, 0, 0, 0, NULL, NULL, NULL, NULL);
> -
>    ok(grand_parent != 0, "grand_parent creation failed\n");
> - }
> -
> - parent = CreateWindowExA(0, "static", NULL, style_in,
> -
>                         0, 0, 0, 0, grand_parent, NULL, NULL, NULL);
> - ok(parent != 0, "parent creation failed, style %#x\n", style_in);

Always creating window here changes existing tests. It's not a big deal,
I wouldn't expect it to change much, but still, the case that's
currently tested will no longer be tested.

Jacek