[PATCH v6 0/4] MR5759: comctl32 & user32: Mark radio buttons at selected when focused
Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=56109 -- v6: user32: Send parent BN_CLICKED notification when a radio button get focused user32/tests: Add tests for radio button WM_SETFOCUS comctl32: Send parent BN_CLICKED notification when a radio button get focused comctl32/tests: Add tests for radio button WM_SETFOCUS https://gitlab.winehq.org/wine/wine/-/merge_requests/5759
From: Fabian Maurer <dark.shadow4(a)web.de> --- dlls/comctl32/tests/button.c | 108 +++++++++++++++++++++++++++++++++++ 1 file changed, 108 insertions(+) diff --git a/dlls/comctl32/tests/button.c b/dlls/comctl32/tests/button.c index 7c171af8af1..81fc91fcfad 100644 --- a/dlls/comctl32/tests/button.c +++ b/dlls/comctl32/tests/button.c @@ -2462,6 +2462,113 @@ static void test_getobject(void) DestroyWindow(hwnd); } +static void test_radiobutton_focus(void) +{ + HWND hwnd, button; + MSG msg; + int i; + DWORD types[] = { BS_RADIOBUTTON, BS_AUTORADIOBUTTON }; + + static const struct message set_focus1[] = + { + { WM_SETFOCUS, sent }, + { WM_COMMAND, sent|parent|wparam, MAKEWPARAM(ID_BUTTON, BN_SETFOCUS) }, + { WM_COMMAND, sent|parent|wparam, MAKEWPARAM(ID_BUTTON, BN_CLICKED) }, + { WM_PAINT, sent }, + { 0 } + }; + + static const struct message set_focus2[] = + { + { WM_SETFOCUS, sent }, + { WM_COMMAND, sent|parent|wparam, MAKEWPARAM(ID_BUTTON, BN_SETFOCUS) }, + { WM_PAINT, sent }, + { 0 } + }; + + static const struct message set_focus4[] = + { + { WM_LBUTTONDOWN, sent|wparam|lparam, 0, 0 }, + { WM_KILLFOCUS, sent|parent }, + { WM_IME_SETCONTEXT, sent|optional|parent }, + { WM_IME_SETCONTEXT, sent|optional|defwinproc }, + { EVENT_OBJECT_FOCUS, winevent_hook|wparam|lparam, OBJID_CLIENT, 0 }, + { WM_SETFOCUS, sent|defwinproc }, + { WM_COMMAND, sent|parent }, + { BM_SETSTATE, sent|wparam|defwinproc, TRUE }, + { EVENT_OBJECT_STATECHANGE, winevent_hook|wparam|lparam, OBJID_CLIENT, 0 }, + { WM_PAINT, sent }, + { 0 } + }; + + static const struct message set_focus5[] = + { + { WM_SETFOCUS, sent }, + { WM_COMMAND, sent|parent|wparam, ID_BUTTON }, + { WM_PAINT, sent }, + { 0 } + }; + + hwnd = CreateWindowExA(0, "TestParentClass", "Test parent", WS_OVERLAPPEDWINDOW | WS_VISIBLE, + 100, 100, 200, 200, 0, 0, 0, NULL); + ok(hwnd != 0, "Failed to create parent window\n"); + + for (i = 0; i < ARRAY_SIZE(types); i++) + { + /* Test default button */ + button = create_button(types[i] | WS_VISIBLE, hwnd); + while (PeekMessageA(&msg, 0, 0, 0, PM_REMOVE)) DispatchMessageA(&msg); + flush_sequences(sequences, NUM_MSG_SEQUENCES); + SendMessageA(button, WM_SETFOCUS, 0, 0); + while (PeekMessageA(&msg, 0, 0, 0, PM_REMOVE)) DispatchMessageA(&msg); + ok_sequence(sequences, COMBINED_SEQ_INDEX, set_focus1, "WM_SETFOCUS on a radiobutton 1", TRUE); + DestroyWindow(button); + + /* Test already checked button */ + button = create_button(types[i] | WS_VISIBLE, hwnd); + SendMessageA(button, BM_SETCHECK, BST_CHECKED, 0); + while (PeekMessageA(&msg, 0, 0, 0, PM_REMOVE)) DispatchMessageA(&msg); + flush_sequences(sequences, NUM_MSG_SEQUENCES); + SendMessageA(button, WM_SETFOCUS, 0, 0); + while (PeekMessageA(&msg, 0, 0, 0, PM_REMOVE)) DispatchMessageA(&msg); + ok_sequence(sequences, COMBINED_SEQ_INDEX, set_focus2, "WM_SETFOCUS on a radiobutton 2", FALSE); + DestroyWindow(button); + + /* Test already focused button */ + button = create_button(types[i] | WS_VISIBLE, hwnd); + SendMessageA(button, WM_SETFOCUS, 0, 0); + SendMessageA(button, BM_SETCHECK, BST_UNCHECKED, 0); + while (PeekMessageA(&msg, 0, 0, 0, PM_REMOVE)) DispatchMessageA(&msg); + flush_sequences(sequences, NUM_MSG_SEQUENCES); + SendMessageA(button, WM_SETFOCUS, 0, 0); + while (PeekMessageA(&msg, 0, 0, 0, PM_REMOVE)) DispatchMessageA(&msg); + ok_sequence(sequences, COMBINED_SEQ_INDEX, set_focus1, "WM_SETFOCUS on a radiobutton 3", TRUE); + DestroyWindow(button); + + /* Test WM_LBUTTONDOWN */ + button = create_button(types[i] | WS_VISIBLE, hwnd); + while (PeekMessageA(&msg, 0, 0, 0, PM_REMOVE)) DispatchMessageA(&msg); + flush_sequences(sequences, NUM_MSG_SEQUENCES); + SendMessageA(button, WM_LBUTTONDOWN, 0, 0); + while (PeekMessageA(&msg, 0, 0, 0, PM_REMOVE)) DispatchMessageA(&msg); + ok_sequence(sequences, COMBINED_SEQ_INDEX, set_focus4, "WM_SETFOCUS on a radiobutton 4", FALSE); + DestroyWindow(button); + + /* Test without BS_NOTIFY */ + button = CreateWindowExA(0, WC_BUTTONA, "test", types[i] | WS_VISIBLE | WS_CHILD, 0, 0, 50, 14, hwnd, (HMENU)ID_BUTTON, 0, NULL); + ok(hwnd != NULL, "failed to create a button, 0x%08lx, %p\n", types[i] | WS_VISIBLE | WS_CHILD, hwnd); + pSetWindowSubclass(button, button_subclass_proc, 0, 0); + while (PeekMessageA(&msg, 0, 0, 0, PM_REMOVE)) DispatchMessageA(&msg); + flush_sequences(sequences, NUM_MSG_SEQUENCES); + SendMessageA(button, WM_SETFOCUS, 0, 0); + while (PeekMessageA(&msg, 0, 0, 0, PM_REMOVE)) DispatchMessageA(&msg); + ok_sequence(sequences, COMBINED_SEQ_INDEX, set_focus5, "WM_SETFOCUS on a radiobutton 5", TRUE); + DestroyWindow(button); + } + + DestroyWindow(hwnd); +} + START_TEST(button) { BOOL (WINAPI * pIsThemeActive)(VOID); @@ -2500,6 +2607,7 @@ START_TEST(button) test_style(); test_visual(); test_getobject(); + test_radiobutton_focus(); uninit_winevent_hook(); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5759
From: Fabian Maurer <dark.shadow4(a)web.de> We need to make sure here that the button is marked pressed before it gets the focus. Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=56109 --- dlls/comctl32/button.c | 11 ++++++++++- dlls/comctl32/tests/button.c | 6 +++--- 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/dlls/comctl32/button.c b/dlls/comctl32/button.c index 11a9760e3e2..a1de6a2ec4c 100644 --- a/dlls/comctl32/button.c +++ b/dlls/comctl32/button.c @@ -628,15 +628,18 @@ static LRESULT CALLBACK BUTTON_WindowProc(HWND hWnd, UINT uMsg, WPARAM wParam, L } /* fall through */ case WM_LBUTTONDOWN: + infoPtr->state |= BUTTON_BTNPRESSED; SetFocus( hWnd ); if ((btn_type == BS_SPLITBUTTON || btn_type == BS_DEFSPLITBUTTON) && !(infoPtr->split_style & BCSS_NOSPLIT) && notify_split_button_dropdown(infoPtr, &pt, hWnd)) + { + infoPtr->state &= ~BUTTON_BTNPRESSED; break; + } SetCapture( hWnd ); - infoPtr->state |= BUTTON_BTNPRESSED; SendMessageW( hWnd, BM_SETSTATE, TRUE, 0 ); break; @@ -867,6 +870,12 @@ static LRESULT CALLBACK BUTTON_WindowProc(HWND hWnd, UINT uMsg, WPARAM wParam, L if (style & BS_NOTIFY) BUTTON_NOTIFY_PARENT(hWnd, BN_SETFOCUS); + + if (((btn_type == BS_RADIOBUTTON) || (btn_type == BS_AUTORADIOBUTTON)) && + !(infoPtr->state & (BST_CHECKED | BUTTON_BTNPRESSED))) + { + BUTTON_NOTIFY_PARENT(hWnd, BN_CLICKED); + } break; case WM_KILLFOCUS: diff --git a/dlls/comctl32/tests/button.c b/dlls/comctl32/tests/button.c index 81fc91fcfad..512cb8473dc 100644 --- a/dlls/comctl32/tests/button.c +++ b/dlls/comctl32/tests/button.c @@ -2521,7 +2521,7 @@ static void test_radiobutton_focus(void) flush_sequences(sequences, NUM_MSG_SEQUENCES); SendMessageA(button, WM_SETFOCUS, 0, 0); while (PeekMessageA(&msg, 0, 0, 0, PM_REMOVE)) DispatchMessageA(&msg); - ok_sequence(sequences, COMBINED_SEQ_INDEX, set_focus1, "WM_SETFOCUS on a radiobutton 1", TRUE); + ok_sequence(sequences, COMBINED_SEQ_INDEX, set_focus1, "WM_SETFOCUS on a radiobutton 1", FALSE); DestroyWindow(button); /* Test already checked button */ @@ -2542,7 +2542,7 @@ static void test_radiobutton_focus(void) flush_sequences(sequences, NUM_MSG_SEQUENCES); SendMessageA(button, WM_SETFOCUS, 0, 0); while (PeekMessageA(&msg, 0, 0, 0, PM_REMOVE)) DispatchMessageA(&msg); - ok_sequence(sequences, COMBINED_SEQ_INDEX, set_focus1, "WM_SETFOCUS on a radiobutton 3", TRUE); + ok_sequence(sequences, COMBINED_SEQ_INDEX, set_focus1, "WM_SETFOCUS on a radiobutton 3", FALSE); DestroyWindow(button); /* Test WM_LBUTTONDOWN */ @@ -2562,7 +2562,7 @@ static void test_radiobutton_focus(void) flush_sequences(sequences, NUM_MSG_SEQUENCES); SendMessageA(button, WM_SETFOCUS, 0, 0); while (PeekMessageA(&msg, 0, 0, 0, PM_REMOVE)) DispatchMessageA(&msg); - ok_sequence(sequences, COMBINED_SEQ_INDEX, set_focus5, "WM_SETFOCUS on a radiobutton 5", TRUE); + ok_sequence(sequences, COMBINED_SEQ_INDEX, set_focus5, "WM_SETFOCUS on a radiobutton 5", FALSE); DestroyWindow(button); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5759
From: Fabian Maurer <dark.shadow4(a)web.de> --- dlls/user32/tests/msg.c | 98 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 98 insertions(+) diff --git a/dlls/user32/tests/msg.c b/dlls/user32/tests/msg.c index 590f50aa6b7..63513717d29 100644 --- a/dlls/user32/tests/msg.c +++ b/dlls/user32/tests/msg.c @@ -20526,6 +20526,103 @@ static void test_hook_changing_window_proc(void) DestroyWindow( hwnd ); } +static void test_radiobutton_focus(void) +{ + HWND hwnd, button; + DWORD style; + MSG msg; + int i; + DWORD types[] = { BS_RADIOBUTTON, BS_AUTORADIOBUTTON }; + + static const struct message set_focus1[] = + { + { WM_COMMAND, sent|parent|wparam, MAKEWPARAM(ID_BUTTON, BN_SETFOCUS) }, + { WM_COMMAND, sent|parent|wparam, MAKEWPARAM(ID_BUTTON, BN_CLICKED) }, + { 0 } + }; + + static const struct message set_focus2[] = + { + { WM_COMMAND, sent|parent|wparam, MAKEWPARAM(ID_BUTTON, BN_SETFOCUS) }, + { 0 } + }; + + static const struct message set_focus4[] = + { + { WM_KILLFOCUS, sent|parent }, + { WM_IME_SETCONTEXT, sent|optional|parent }, + { WM_IME_SETCONTEXT, sent|optional|defwinproc }, + { WM_COMMAND, sent|parent }, + { 0 } + }; + + static const struct message set_focus5[] = + { + { WM_COMMAND, sent|parent|wparam, ID_BUTTON }, + { 0 } + }; + + hwnd = CreateWindowExA(0, "TestParentClass", "Test parent", WS_OVERLAPPEDWINDOW | WS_VISIBLE, + 100, 100, 200, 200, 0, 0, 0, NULL); + ok(hwnd != 0, "Failed to create parent window\n"); + + for (i = 0; i < ARRAY_SIZE(types); i++) + { + /* Test default button */ + style = types[i] | WS_CHILD | WS_VISIBLE | BS_NOTIFY; + button = CreateWindowExA(0, WC_BUTTONA, "test", style, 0, 0, 50, 14, hwnd, (HMENU)ID_BUTTON, 0, NULL); + ok(button != NULL, "failed to create a button, 0x%08lx, %p\n", style, hwnd); + flush_events(); + flush_sequence(); + SendMessageA(button, WM_SETFOCUS, 0, 0); + flush_events(); + ok_sequence(set_focus1, "WM_SETFOCUS on a radiobutton 1", TRUE); + DestroyWindow(button); + + /* Test already checked button */ + button = CreateWindowExA(0, WC_BUTTONA, "test", style, 0, 0, 50, 14, hwnd, (HMENU)ID_BUTTON, 0, NULL); + SendMessageA(button, BM_SETCHECK, BST_CHECKED, 0); + flush_events(); + flush_sequence(); + SendMessageA(button, WM_SETFOCUS, 0, 0); + flush_events(); + ok_sequence(set_focus2, "WM_SETFOCUS on a radiobutton 2", FALSE); + DestroyWindow(button); + + /* Test already focused button */ + button = CreateWindowExA(0, WC_BUTTONA, "test", style, 0, 0, 50, 14, hwnd, (HMENU)ID_BUTTON, 0, NULL); + SendMessageA(button, WM_SETFOCUS, 0, 0); + SendMessageA(button, BM_SETCHECK, BST_UNCHECKED, 0); + flush_events(); + flush_sequence(); + SendMessageA(button, WM_SETFOCUS, 0, 0); + flush_events(); + ok_sequence(set_focus1, "WM_SETFOCUS on a radiobutton 3", TRUE); + DestroyWindow(button); + + /* Test WM_LBUTTONDOWN */ + button = CreateWindowExA(0, WC_BUTTONA, "test", style, 0, 0, 50, 14, hwnd, (HMENU)ID_BUTTON, 0, NULL); + flush_events(); + flush_sequence(); + SendMessageA(button, WM_LBUTTONDOWN, 0, 0); + while (PeekMessageA(&msg, 0, 0, 0, PM_REMOVE)) DispatchMessageA(&msg); + ok_sequence(set_focus4, "WM_SETFOCUS on a radiobutton 4", FALSE); + DestroyWindow(button); + + /* Test without BS_NOTIFY */ + style = types[i] | WS_CHILD | WS_VISIBLE; + button = CreateWindowExA(0, WC_BUTTONA, "test", style, 0, 0, 50, 14, hwnd, (HMENU)ID_BUTTON, 0, NULL); + flush_events(); + flush_sequence(); + SendMessageA(button, WM_SETFOCUS, 0, 0); + flush_events(); + ok_sequence(set_focus5, "WM_SETFOCUS on a radiobutton 5", TRUE); + DestroyWindow(button); + } + + DestroyWindow(hwnd); +} + START_TEST(msg) { char **test_argv; @@ -20573,6 +20670,7 @@ START_TEST(msg) test_setparent_status(); test_InSendMessage(); test_SetFocus(); + test_radiobutton_focus(); test_SetParent(); test_PostMessage(); test_broadcast(); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5759
From: Fabian Maurer <dark.shadow4(a)web.de> Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=56109 --- dlls/user32/button.c | 8 +++++++- dlls/user32/tests/msg.c | 6 +++--- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/dlls/user32/button.c b/dlls/user32/button.c index 61e34f99246..2a889223143 100644 --- a/dlls/user32/button.c +++ b/dlls/user32/button.c @@ -257,8 +257,8 @@ LRESULT ButtonWndProc_common(HWND hWnd, UINT uMsg, WPARAM wParam, LPARAM lParam, /* fall through */ case WM_LBUTTONDOWN: NtUserSetCapture( hWnd ); - NtUserSetFocus( hWnd ); set_button_state( hWnd, get_button_state( hWnd ) | BUTTON_BTNPRESSED ); + NtUserSetFocus( hWnd ); SendMessageW( hWnd, BM_SETSTATE, TRUE, 0 ); break; @@ -381,6 +381,12 @@ LRESULT ButtonWndProc_common(HWND hWnd, UINT uMsg, WPARAM wParam, LPARAM lParam, paint_button( hWnd, btn_type, ODA_FOCUS ); if (style & BS_NOTIFY) BUTTON_NOTIFY_PARENT(hWnd, BN_SETFOCUS); + + if (((btn_type == BS_RADIOBUTTON) || (btn_type == BS_AUTORADIOBUTTON)) && + !(get_button_state(hWnd) & (BST_CHECKED | BUTTON_BTNPRESSED))) + { + BUTTON_NOTIFY_PARENT(hWnd, BN_CLICKED); + } break; case WM_KILLFOCUS: diff --git a/dlls/user32/tests/msg.c b/dlls/user32/tests/msg.c index 63513717d29..47e93ba1ccc 100644 --- a/dlls/user32/tests/msg.c +++ b/dlls/user32/tests/msg.c @@ -20576,7 +20576,7 @@ static void test_radiobutton_focus(void) flush_sequence(); SendMessageA(button, WM_SETFOCUS, 0, 0); flush_events(); - ok_sequence(set_focus1, "WM_SETFOCUS on a radiobutton 1", TRUE); + ok_sequence(set_focus1, "WM_SETFOCUS on a radiobutton 1", FALSE); DestroyWindow(button); /* Test already checked button */ @@ -20597,7 +20597,7 @@ static void test_radiobutton_focus(void) flush_sequence(); SendMessageA(button, WM_SETFOCUS, 0, 0); flush_events(); - ok_sequence(set_focus1, "WM_SETFOCUS on a radiobutton 3", TRUE); + ok_sequence(set_focus1, "WM_SETFOCUS on a radiobutton 3", FALSE); DestroyWindow(button); /* Test WM_LBUTTONDOWN */ @@ -20616,7 +20616,7 @@ static void test_radiobutton_focus(void) flush_sequence(); SendMessageA(button, WM_SETFOCUS, 0, 0); flush_events(); - ok_sequence(set_focus5, "WM_SETFOCUS on a radiobutton 5", TRUE); + ok_sequence(set_focus5, "WM_SETFOCUS on a radiobutton 5", FALSE); DestroyWindow(button); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5759
Hi, It looks like your patch introduced the new failures shown below. Please investigate and fix them before resubmitting your patch. If they are not new, fixing them anyway would help a lot. Otherwise please ask for the known failures list to be updated. The tests also ran into some preexisting test failures. If you know how to fix them that would be helpful. See the TestBot job for the details: The full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=149755 Your paranoid android. === w7pro64 (64 bit report) === user32: msg.c:6267: Test failed: MouseMove2: 3: the msg sequence is not complete: expected msg 0000 - actual msg 0020 === debian11 (32 bit report) === user32: win.c:4070: Test failed: Expected active window 00D40110, got 00000000. win.c:4071: Test failed: Expected focus window 00D40110, got 00000000. === debian11 (32 bit fr report) === user32: msg.c:6995: Test failed: SetFocus(hwnd) on a button: 3: the winevent_hook 0x8005 was expected, but got hook 0x0005 instead msg.c:6995: Test failed: SetFocus(hwnd) on a button: 3: the winevent_hook 0x8005 was expected, but got winevent_hook 0x0003 instead msg.c:6995: Test failed: SetFocus(hwnd) on a button: 3: the winevent_hook 0x8005 was expected, but got msg 0x030f instead msg.c:6995: Test failed: SetFocus(hwnd) on a button: 3: the winevent_hook 0x8005 was expected, but got msg 0x001c instead msg.c:6995: Test failed: SetFocus(hwnd) on a button: 3: the winevent_hook 0x8005 was expected, but got msg 0x0086 instead msg.c:6995: Test failed: SetFocus(hwnd) on a button: 3: the winevent_hook 0x8005 was expected, but got msg 0x0006 instead msg.c:6995: Test failed: SetFocus(hwnd) on a button: 3: the winevent_hook 0x8005 was expected, but got hook 0x0009 instead === debian11b (64 bit WoW report) === user32: win.c:4070: Test failed: Expected active window 0000000003030152, got 0000000000000000. win.c:4071: Test failed: Expected focus window 0000000003030152, got 0000000000000000. winmm: mci: Timeout
On Fri Nov 15 20:27:51 2024 +0000, Fabian Maurer wrote:
Thanks, I must have missed that - I'm on it but it's not ready yet. Updated
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/5759#note_87958
Zhiyi Zhang (@zhiyi) commented about dlls/comctl32/tests/button.c:
+ { BM_SETSTATE, sent|wparam|defwinproc, TRUE }, + { EVENT_OBJECT_STATECHANGE, winevent_hook|wparam|lparam, OBJID_CLIENT, 0 }, + { WM_PAINT, sent }, + { 0 } + }; + + static const struct message set_focus5[] = + { + { WM_SETFOCUS, sent }, + { WM_COMMAND, sent|parent|wparam, ID_BUTTON }, + { WM_PAINT, sent }, + { 0 } + }; + + hwnd = CreateWindowExA(0, "TestParentClass", "Test parent", WS_OVERLAPPEDWINDOW | WS_VISIBLE, + 100, 100, 200, 200, 0, 0, 0, NULL); Keep this line aligned.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/5759#note_87988
Zhiyi Zhang (@zhiyi) commented about dlls/comctl32/tests/button.c:
DestroyWindow(hwnd); }
+static void test_radiobutton_focus(void) +{ + HWND hwnd, button; + MSG msg; + int i; + DWORD types[] = { BS_RADIOBUTTON, BS_AUTORADIOBUTTON }; + + static const struct message set_focus1[] =
Let's use more descriptive names for these message sequences, for example, set_focus_default_seq, set_focus_checked_seq, and WM_LBUTTONDOWN_seq. Also, the description used in ok_sequence() can use some detailed descriptions. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5759#note_87987
participants (4)
-
Fabian Maurer -
Fabian Maurer (＠DarkShadow44)
-
Marvin -
Zhiyi Zhang (＠zhiyi)