[PATCH v10 0/4] MR5759: comctl32 & user32: Mark radio buttons at selected when focused
Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=56109 -- v10: 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 | 120 +++++++++++++++++++++++++++++++++++ 1 file changed, 120 insertions(+) diff --git a/dlls/comctl32/tests/button.c b/dlls/comctl32/tests/button.c index 7c171af8af1..9cfbe1b8646 100644 --- a/dlls/comctl32/tests/button.c +++ b/dlls/comctl32/tests/button.c @@ -2462,6 +2462,125 @@ static void test_getobject(void) DestroyWindow(hwnd); } +static void test_radiobutton_focus(void) +{ + HWND hwnd, button; + int i; + DWORD types[] = { BS_RADIOBUTTON, BS_AUTORADIOBUTTON }; + + static const struct message set_focus_default_seq[] = + { + { 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_focus_checked_seq[] = + { + { WM_SETFOCUS, sent }, + { WM_COMMAND, sent|parent|wparam, MAKEWPARAM(ID_BUTTON, BN_SETFOCUS) }, + { WM_PAINT, sent }, + { 0 } + }; + + static const struct message WM_LBUTTONDOWN_seq[] = + { + { WM_LBUTTONDOWN, sent|wparam|lparam, 0, 0x70007 }, + { 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 }, + { WM_PAINT, sent|optional }, /* Not sent by Wine */ + { WM_PAINT, sent|optional }, /* Not sent by Wine */ + { WM_PAINT, sent|optional }, /* Not sent by Wine */ + { 0 } + }; + + static const struct message set_focus_without_notify_seq[] = + { + { 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); + flush_events(); + flush_sequences(sequences, NUM_MSG_SEQUENCES); + SendMessageA(button, WM_SETFOCUS, 0, 0); + flush_events(); + ok_sequence(sequences, COMBINED_SEQ_INDEX, set_focus_default_seq, "WM_SETFOCUS on default radiobutton", TRUE); + DestroyWindow(button); + + /* Test already checked button */ + button = create_button(types[i] | WS_VISIBLE, hwnd); + SendMessageA(button, BM_SETCHECK, BST_CHECKED, 0); + flush_events(); + flush_sequences(sequences, NUM_MSG_SEQUENCES); + SendMessageA(button, WM_SETFOCUS, 0, 0); + flush_events(); + ok_sequence(sequences, COMBINED_SEQ_INDEX, set_focus_checked_seq, "WM_SETFOCUS on checked radiobutton", 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); + flush_events(); + flush_sequences(sequences, NUM_MSG_SEQUENCES); + SendMessageA(button, WM_SETFOCUS, 0, 0); + flush_events(); + ok_sequence(sequences, COMBINED_SEQ_INDEX, set_focus_default_seq, "WM_SETFOCUS on focused radiobutton", TRUE); + DestroyWindow(button); + + /* Test WM_LBUTTONDOWN */ + button = create_button(types[i] | WS_VISIBLE, hwnd); + flush_events(); + flush_sequences(sequences, NUM_MSG_SEQUENCES); + SendMessageA(button, WM_LBUTTONDOWN, 0, MAKELPARAM(7, 7)); + flush_events(); + ok_sequence(sequences, COMBINED_SEQ_INDEX, WM_LBUTTONDOWN_seq, "WM_LBUTTONDOWN on radiobutton", 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); + flush_events(); + flush_sequences(sequences, NUM_MSG_SEQUENCES); + SendMessageA(button, WM_SETFOCUS, 0, 0); + flush_events(); + ok_sequence(sequences, COMBINED_SEQ_INDEX, set_focus_without_notify_seq, "WM_SETFOCUS on radiobutton without BS_NOTIFY", TRUE); + DestroyWindow(button); + + /* Test disabled button */ + button = create_button(types[i] | WS_VISIBLE, hwnd); + EnableWindow(button, FALSE); + flush_events(); + flush_sequences(sequences, NUM_MSG_SEQUENCES); + SendMessageA(button, WM_SETFOCUS, 0, 0); + flush_events(); + ok_sequence(sequences, COMBINED_SEQ_INDEX, set_focus_default_seq, "WM_SETFOCUS on disabled radiobutton", TRUE); + DestroyWindow(button); + } + + DestroyWindow(hwnd); +} + START_TEST(button) { BOOL (WINAPI * pIsThemeActive)(VOID); @@ -2500,6 +2619,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 | 12 ++++++++++-- dlls/comctl32/tests/button.c | 8 ++++---- 2 files changed, 14 insertions(+), 6 deletions(-) diff --git a/dlls/comctl32/button.c b/dlls/comctl32/button.c index 11a9760e3e2..a0eb41fec20 100644 --- a/dlls/comctl32/button.c +++ b/dlls/comctl32/button.c @@ -25,7 +25,6 @@ * * Messages * - WM_CHAR: Checks a (manual or automatic) check box on '+' or '=', clears it on '-' key. - * - WM_SETFOCUS: For (manual or automatic) radio buttons, send the parent window BN_CLICKED * - WM_NCCREATE: Turns any BS_OWNERDRAW button into a BS_PUSHBUTTON button. * - WM_SYSKEYUP * @@ -628,15 +627,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 +869,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 9cfbe1b8646..676062f0132 100644 --- a/dlls/comctl32/tests/button.c +++ b/dlls/comctl32/tests/button.c @@ -2523,7 +2523,7 @@ static void test_radiobutton_focus(void) flush_sequences(sequences, NUM_MSG_SEQUENCES); SendMessageA(button, WM_SETFOCUS, 0, 0); flush_events(); - ok_sequence(sequences, COMBINED_SEQ_INDEX, set_focus_default_seq, "WM_SETFOCUS on default radiobutton", TRUE); + ok_sequence(sequences, COMBINED_SEQ_INDEX, set_focus_default_seq, "WM_SETFOCUS on default radiobutton", FALSE); DestroyWindow(button); /* Test already checked button */ @@ -2544,7 +2544,7 @@ static void test_radiobutton_focus(void) flush_sequences(sequences, NUM_MSG_SEQUENCES); SendMessageA(button, WM_SETFOCUS, 0, 0); flush_events(); - ok_sequence(sequences, COMBINED_SEQ_INDEX, set_focus_default_seq, "WM_SETFOCUS on focused radiobutton", TRUE); + ok_sequence(sequences, COMBINED_SEQ_INDEX, set_focus_default_seq, "WM_SETFOCUS on focused radiobutton", FALSE); DestroyWindow(button); /* Test WM_LBUTTONDOWN */ @@ -2564,7 +2564,7 @@ static void test_radiobutton_focus(void) flush_sequences(sequences, NUM_MSG_SEQUENCES); SendMessageA(button, WM_SETFOCUS, 0, 0); flush_events(); - ok_sequence(sequences, COMBINED_SEQ_INDEX, set_focus_without_notify_seq, "WM_SETFOCUS on radiobutton without BS_NOTIFY", TRUE); + ok_sequence(sequences, COMBINED_SEQ_INDEX, set_focus_without_notify_seq, "WM_SETFOCUS on radiobutton without BS_NOTIFY", FALSE); DestroyWindow(button); /* Test disabled button */ @@ -2574,7 +2574,7 @@ static void test_radiobutton_focus(void) flush_sequences(sequences, NUM_MSG_SEQUENCES); SendMessageA(button, WM_SETFOCUS, 0, 0); flush_events(); - ok_sequence(sequences, COMBINED_SEQ_INDEX, set_focus_default_seq, "WM_SETFOCUS on disabled radiobutton", TRUE); + ok_sequence(sequences, COMBINED_SEQ_INDEX, set_focus_default_seq, "WM_SETFOCUS on disabled radiobutton", 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 | 108 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 108 insertions(+) diff --git a/dlls/user32/tests/msg.c b/dlls/user32/tests/msg.c index 590f50aa6b7..9b0dab69196 100644 --- a/dlls/user32/tests/msg.c +++ b/dlls/user32/tests/msg.c @@ -20526,6 +20526,113 @@ static void test_hook_changing_window_proc(void) DestroyWindow( hwnd ); } +static void test_radiobutton_focus(void) +{ + HWND hwnd, button; + DWORD style; + int i; + DWORD types[] = { BS_RADIOBUTTON, BS_AUTORADIOBUTTON }; + + static const struct message set_focus_default_seq[] = + { + { 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_focus_checked_seq[] = + { + { WM_COMMAND, sent|parent|wparam, MAKEWPARAM(ID_BUTTON, BN_SETFOCUS) }, + { 0 } + }; + + static const struct message WM_LBUTTONDOWN_seq[] = + { + { 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_focus_without_notify_seq[] = + { + { 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_focus_default_seq, "WM_SETFOCUS on default radiobutton", 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_focus_checked_seq, "WM_SETFOCUS on checked radiobutton", 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_focus_default_seq, "WM_SETFOCUS on focused radiobutton", 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, MAKELPARAM(7, 7)); + flush_events(); + ok_sequence(WM_LBUTTONDOWN_seq, "WM_LBUTTONDOWN on radiobutton", 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_focus_without_notify_seq, "WM_SETFOCUS on radiobutton without BS_NOTIFY", TRUE); + DestroyWindow(button); + + /* Test disabled 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); + EnableWindow(button, FALSE); + flush_events(); + flush_sequence(); + SendMessageA(button, WM_SETFOCUS, 0, 0); + flush_events(); + ok_sequence(set_focus_default_seq, "WM_SETFOCUS on disabled radiobutton", TRUE); + DestroyWindow(button); + } + + DestroyWindow(hwnd); +} + START_TEST(msg) { char **test_argv; @@ -20573,6 +20680,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 | 9 +++++++-- dlls/user32/tests/msg.c | 8 ++++---- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/dlls/user32/button.c b/dlls/user32/button.c index 61e34f99246..ab6787899cb 100644 --- a/dlls/user32/button.c +++ b/dlls/user32/button.c @@ -25,7 +25,6 @@ * * Messages * - WM_CHAR: Checks a (manual or automatic) check box on '+' or '=', clears it on '-' key. - * - WM_SETFOCUS: For (manual or automatic) radio buttons, send the parent window BN_CLICKED * - WM_NCCREATE: Turns any BS_OWNERDRAW button into a BS_PUSHBUTTON button. * - WM_SYSKEYUP * @@ -257,8 +256,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 +380,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 9b0dab69196..becf6a164b4 100644 --- a/dlls/user32/tests/msg.c +++ b/dlls/user32/tests/msg.c @@ -20575,7 +20575,7 @@ static void test_radiobutton_focus(void) flush_sequence(); SendMessageA(button, WM_SETFOCUS, 0, 0); flush_events(); - ok_sequence(set_focus_default_seq, "WM_SETFOCUS on default radiobutton", TRUE); + ok_sequence(set_focus_default_seq, "WM_SETFOCUS on default radiobutton", FALSE); DestroyWindow(button); /* Test already checked button */ @@ -20596,7 +20596,7 @@ static void test_radiobutton_focus(void) flush_sequence(); SendMessageA(button, WM_SETFOCUS, 0, 0); flush_events(); - ok_sequence(set_focus_default_seq, "WM_SETFOCUS on focused radiobutton", TRUE); + ok_sequence(set_focus_default_seq, "WM_SETFOCUS on focused radiobutton", FALSE); DestroyWindow(button); /* Test WM_LBUTTONDOWN */ @@ -20615,7 +20615,7 @@ static void test_radiobutton_focus(void) flush_sequence(); SendMessageA(button, WM_SETFOCUS, 0, 0); flush_events(); - ok_sequence(set_focus_without_notify_seq, "WM_SETFOCUS on radiobutton without BS_NOTIFY", TRUE); + ok_sequence(set_focus_without_notify_seq, "WM_SETFOCUS on radiobutton without BS_NOTIFY", FALSE); DestroyWindow(button); /* Test disabled button */ @@ -20626,7 +20626,7 @@ static void test_radiobutton_focus(void) flush_sequence(); SendMessageA(button, WM_SETFOCUS, 0, 0); flush_events(); - ok_sequence(set_focus_default_seq, "WM_SETFOCUS on disabled radiobutton", TRUE); + ok_sequence(set_focus_default_seq, "WM_SETFOCUS on disabled radiobutton", 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=149844 Your paranoid android. === debian11 (32 bit report) === user32: input.c:4305: Test succeeded inside todo block: button_down_hwnd_todo 1: got MSG_TEST_WIN hwnd 009B0084, msg WM_LBUTTONDOWN, wparam 0x1, lparam 0x320032 win.c:4070: Test failed: Expected active window 00D40110, got 00000000. win.c:4071: Test failed: Expected focus window 00D40110, got 00000000. === debian11b (64 bit WoW report) === user32: input.c:4305: Test succeeded inside todo block: button_down_hwnd_todo 1: got MSG_TEST_WIN hwnd 0000000000E700E4, msg WM_LBUTTONDOWN, wparam 0x1, lparam 0x320032 win.c:4070: Test failed: Expected active window 0000000005A20172, got 0000000000000000. win.c:4071: Test failed: Expected focus window 0000000005A20172, got 0000000000000000. winmm: mci: Timeout
On Thu Nov 21 08:39:29 2024 +0000, Zhiyi Zhang wrote:
"A disabled button does not send a BN_CLICKED notification code to its parent window." from [MSDN](https://learn.microsoft.com/en-us/windows/win32/controls/bn-clicked). Could you add tests for it? I added a test, but it doesn't seem to be true, or at least not in this case.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/5759#note_88369
This merge request was approved by Zhiyi Zhang. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5759
participants (4)
-
Fabian Maurer -
Fabian Maurer (＠DarkShadow44)
-
Marvin -
Zhiyi Zhang (＠zhiyi)