Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=56109
-- v9: 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.
From: Fabian Maurer dark.shadow4@web.de
--- dlls/comctl32/tests/button.c | 110 +++++++++++++++++++++++++++++++++++ 1 file changed, 110 insertions(+)
diff --git a/dlls/comctl32/tests/button.c b/dlls/comctl32/tests/button.c index 7c171af8af1..adfad6f4d0b 100644 --- a/dlls/comctl32/tests/button.c +++ b/dlls/comctl32/tests/button.c @@ -2462,6 +2462,115 @@ 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 withouth BS_NOTIFY", TRUE); + DestroyWindow(button); + } + + DestroyWindow(hwnd); +} + START_TEST(button) { BOOL (WINAPI * pIsThemeActive)(VOID); @@ -2500,6 +2609,7 @@ START_TEST(button) test_style(); test_visual(); test_getobject(); + test_radiobutton_focus();
uninit_winevent_hook();
From: Fabian Maurer dark.shadow4@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 adfad6f4d0b..089c6dde2b6 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 withouth BS_NOTIFY", TRUE); + ok_sequence(sequences, COMBINED_SEQ_INDEX, set_focus_without_notify_seq, "WM_SETFOCUS on radiobutton withouth BS_NOTIFY", FALSE); DestroyWindow(button); }
From: Fabian Maurer dark.shadow4@web.de
--- dlls/user32/tests/msg.c | 97 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 97 insertions(+)
diff --git a/dlls/user32/tests/msg.c b/dlls/user32/tests/msg.c index 590f50aa6b7..87bcf85fa2a 100644 --- a/dlls/user32/tests/msg.c +++ b/dlls/user32/tests/msg.c @@ -20526,6 +20526,102 @@ 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 withouth BS_NOTIFY", TRUE); + DestroyWindow(button); + } + + DestroyWindow(hwnd); +} + START_TEST(msg) { char **test_argv; @@ -20573,6 +20669,7 @@ START_TEST(msg) test_setparent_status(); test_InSendMessage(); test_SetFocus(); + test_radiobutton_focus(); test_SetParent(); test_PostMessage(); test_broadcast();
From: Fabian Maurer dark.shadow4@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 87bcf85fa2a..201da49ecf0 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 withouth BS_NOTIFY", TRUE); + ok_sequence(set_focus_without_notify_seq, "WM_SETFOCUS on radiobutton withouth BS_NOTIFY", FALSE); DestroyWindow(button); }
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=149809
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.
=== 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: input.c:4305: Test succeeded inside todo block: button_down_hwnd_todo 1: got MSG_TEST_WIN hwnd 0000000000A100E2, msg WM_LBUTTONDOWN, wparam 0x1, lparam 0x320032
On Wed Nov 20 18:34:27 2024 +0000, Fabian Maurer wrote:
changed this line in [version 9 of the diff](/wine/wine/-/merge_requests/5759/diffs?diff_id=144281&start_sha=a2225cc8819a5201fcf116708106367e3353d429#e7108b69c4f3a83370502f077a893b7cdca070aa_2552_2554)
Updated, thanks for all the reviewing!
On Wed Nov 20 18:34:27 2024 +0000, Fabian Maurer wrote:
changed this line in [version 9 of the diff](/wine/wine/-/merge_requests/5759/diffs?diff_id=144281&start_sha=a2225cc8819a5201fcf116708106367e3353d429#e7108b69c4f3a83370502f077a893b7cdca070aa_2520_2522)
Changed. For some reason, this causes Windows to send 4 WM_PAINT instead of just 1. Not sure why, but I adapted the tests.
Zhiyi Zhang (@zhiyi) commented about dlls/comctl32/button.c:
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);
"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?
-
Fabian Maurer -
Fabian Maurer (＠DarkShadow44)
-
Marvin -
Zhiyi Zhang (＠zhiyi)