Re: user32/tests: Add some tests for dialog owner disabled state. (v3)
Hi Dmitry, On 04/06/16 13:15, Dmitry Timoshkov wrote:
Signed-off-by: Dmitry Timoshkov <dmitry(a)baikal.ru> --- 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
Jacek Caban <jacek(a)codeweavers.com> wrote:
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.
The thing which currently is being tested are the dialog styles that don't depend on the dialog parent. Regarding any possible additional tests or moving some logic around: as the patch subject says these tests don't pretend to be perfect or exhaustive set of tests, they simply test some things I was interested in, and as always with every test there are things that could be added or changed. -- Dmitry.
Dmitry Timoshkov <dmitry(a)baikal.ru> writes:
Regarding any possible additional tests or moving some logic around: as the patch subject says these tests don't pretend to be perfect or exhaustive set of tests, they simply test some things I was interested in, and as always with every test there are things that could be added or changed.
Of course, but could you please fix the few things that Jacek mentioned, and update the todos that succeed now? Thanks. -- Alexandre Julliard julliard(a)winehq.org
Alexandre Julliard <julliard(a)winehq.org> wrote:
Regarding any possible additional tests or moving some logic around: as the patch subject says these tests don't pretend to be perfect or exhaustive set of tests, they simply test some things I was interested in, and as always with every test there are things that could be added or changed.
Of course, but could you please fix the few things that Jacek mentioned, and update the todos that succeed now? Thanks.
I can resend with just todos removed if that's ok, since changing the things that Jacek pointed out (adding the tests after EndDialog, moving the tests into another place or removing a test that makes sure that window state has not really changed) has no influence on the things being actually tested. -- Dmitry.
Dmitry Timoshkov <dmitry(a)baikal.ru> writes:
Alexandre Julliard <julliard(a)winehq.org> wrote:
Regarding any possible additional tests or moving some logic around: as the patch subject says these tests don't pretend to be perfect or exhaustive set of tests, they simply test some things I was interested in, and as always with every test there are things that could be added or changed.
Of course, but could you please fix the few things that Jacek mentioned, and update the todos that succeed now? Thanks.
I can resend with just todos removed if that's ok, since changing the things that Jacek pointed out (adding the tests after EndDialog, moving the tests into another place or removing a test that makes sure that window state has not really changed) has no influence on the things being actually tested.
Maybe it would still be testing the same thing, but moving the tests out of WM_INITDIALOG would make things clearer. Avoiding changing the window creation of existing tests would be a good thing too. -- Alexandre Julliard julliard(a)winehq.org
Alexandre Julliard <julliard(a)winehq.org> wrote:
Regarding any possible additional tests or moving some logic around: as the patch subject says these tests don't pretend to be perfect or exhaustive set of tests, they simply test some things I was interested in, and as always with every test there are things that could be added or changed.
Of course, but could you please fix the few things that Jacek mentioned, and update the todos that succeed now? Thanks.
I can resend with just todos removed if that's ok, since changing the things that Jacek pointed out (adding the tests after EndDialog, moving the tests into another place or removing a test that makes sure that window state has not really changed) has no influence on the things being actually tested.
Maybe it would still be testing the same thing, but moving the tests out of WM_INITDIALOG would make things clearer.
That would complicate things for no reason and wouldn't chage anything. For instance moving the tests out of WM_INITDIALOG to WM_ENTERIDLE would make the tests actually test different behaviour than it was originally intended.
Avoiding changing the window creation of existing tests would be a good thing too.
Since I am an original author of existing tests I don't think that adding another hoop to jump trough (make the tests cope with NULL parent) would change anything for existing tests since this wouldn't change the dialog styles. -- Dmitry.
Dmitry Timoshkov <dmitry(a)baikal.ru> writes:
Alexandre Julliard <julliard(a)winehq.org> wrote:
Maybe it would still be testing the same thing, but moving the tests out of WM_INITDIALOG would make things clearer.
That would complicate things for no reason and wouldn't chage anything. For instance moving the tests out of WM_INITDIALOG to WM_ENTERIDLE would make the tests actually test different behaviour than it was originally intended.
Avoiding changing the window creation of existing tests would be a good thing too.
Since I am an original author of existing tests I don't think that adding another hoop to jump trough (make the tests cope with NULL parent) would change anything for existing tests since this wouldn't change the dialog styles.
Never mind then. Could you please resubmit at least with the todos fixed? -- Alexandre Julliard julliard(a)winehq.org
participants (3)
-
Alexandre Julliard -
Dmitry Timoshkov -
Jacek Caban