[PATCH] comctl32: Send EVENT_OBJECT_STATECHANGE when checking a checkbox
The NVDA screen reader needs this. Signed-off-by: Alex Henrie <alexhenrie24(a)gmail.com> --- dlls/comctl32/button.c | 1 + dlls/user32/tests/msg.c | 18 +++++++++--------- 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/dlls/comctl32/button.c b/dlls/comctl32/button.c index 88b48a4a5c0..faf0212b256 100644 --- a/dlls/comctl32/button.c +++ b/dlls/comctl32/button.c @@ -656,6 +656,7 @@ static LRESULT CALLBACK BUTTON_WindowProc(HWND hWnd, UINT uMsg, WPARAM wParam, L break; } SendMessageW( hWnd, BM_SETSTATE, FALSE, 0 ); + NotifyWinEvent( EVENT_OBJECT_STATECHANGE, hWnd, OBJID_CLIENT, 0 ); GetClientRect( hWnd, &rect ); if (uMsg == WM_KEYUP || PtInRect( &rect, pt )) { diff --git a/dlls/user32/tests/msg.c b/dlls/user32/tests/msg.c index f0e0d0fba76..cb8d4f07fe7 100644 --- a/dlls/user32/tests/msg.c +++ b/dlls/user32/tests/msg.c @@ -1774,7 +1774,7 @@ static const struct message WmModalDialogSeq[] = { { EVENT_OBJECT_FOCUS, winevent_hook|wparam|lparam, OBJID_CLIENT, 0 }, { WM_KILLFOCUS, sent|parent }, { WM_IME_SETCONTEXT, sent|parent|wparam|optional, 0 }, - { EVENT_OBJECT_STATECHANGE, winevent_hook|wparam|lparam|winevent_hook_todo, 0, 0 }, + { EVENT_OBJECT_STATECHANGE, winevent_hook|wparam|lparam, 0, 0 }, { WM_ENABLE, sent|parent|wparam, 0 }, { HCBT_CREATEWND, hook }, { EVENT_OBJECT_REORDER, winevent_hook|wparam|lparam|optional, 0, 0 }, /* Not sent on Win10. */ @@ -1829,7 +1829,7 @@ static const struct message WmModalDialogSeq[] = { { WM_ENTERIDLE, sent|parent|optional }, { WM_ENTERIDLE, sent|parent|optional }, { WM_TIMER, sent }, - { EVENT_OBJECT_STATECHANGE, winevent_hook|wparam|lparam|winevent_hook_todo, 0, 0 }, + { EVENT_OBJECT_STATECHANGE, winevent_hook|wparam|lparam, 0, 0 }, { WM_ENABLE, sent|parent|wparam, 1 }, { WM_WINDOWPOSCHANGING, sent|wparam, SWP_HIDEWINDOW|SWP_NOSIZE|SWP_NOMOVE|SWP_NOACTIVATE }, { EVENT_OBJECT_HIDE, winevent_hook|wparam|lparam|winevent_hook_todo, 0, 0 }, @@ -1860,14 +1860,14 @@ static const struct message WmModalDialogSeq_2[] = { { EVENT_OBJECT_FOCUS, winevent_hook|wparam|lparam, OBJID_CLIENT, 0 }, { WM_KILLFOCUS, sent }, { WM_IME_SETCONTEXT, sent|parent|wparam|optional, 0 }, - { EVENT_OBJECT_STATECHANGE, winevent_hook|wparam|lparam|winevent_hook_todo, 0, 0 }, + { EVENT_OBJECT_STATECHANGE, winevent_hook|wparam|lparam, 0, 0 }, { WM_ENABLE, sent|wparam, 0 }, { HCBT_CREATEWND, hook }, { EVENT_OBJECT_REORDER, winevent_hook|wparam|lparam|optional, 0, 0 }, { EVENT_OBJECT_CREATE, winevent_hook|wparam|lparam|winevent_hook_todo, 0, 0 }, { WM_SETFONT, sent }, { WM_INITDIALOG, sent }, - { EVENT_OBJECT_STATECHANGE, winevent_hook|wparam|lparam|winevent_hook_todo, 0, 0 }, + { EVENT_OBJECT_STATECHANGE, winevent_hook|wparam|lparam, 0, 0 }, { WM_CHANGEUISTATE, sent|optional }, { WM_UPDATEUISTATE, sent|optional }, { WM_ENABLE, sent|wparam, 1 }, @@ -7297,24 +7297,24 @@ static const struct message auto_radio_button_VK_DOWN_dialog[] = { EVENT_SYSTEM_CAPTURESTART, winevent_hook|wparam|lparam|winevent_hook_todo, 0, 0 }, { BM_SETSTATE, sent|wparam|lparam|defwinproc, BST_CHECKED, ID_RADIO1 }, { WM_CTLCOLORSTATIC, sent|parent }, - { EVENT_OBJECT_STATECHANGE, winevent_hook|wparam|lparam|winevent_hook_todo, OBJID_CLIENT, 0 }, + { EVENT_OBJECT_STATECHANGE, winevent_hook|wparam|lparam, OBJID_CLIENT, 0 }, { WM_LBUTTONUP, sent|wparam|lparam|defwinproc, 0, 0 }, { BM_SETSTATE, sent|wparam|lparam|defwinproc, BST_UNCHECKED, ID_RADIO1 }, { WM_CTLCOLORSTATIC, sent|parent }, - { EVENT_OBJECT_STATECHANGE, winevent_hook|wparam|lparam|winevent_hook_todo, OBJID_CLIENT, 0 }, + { EVENT_OBJECT_STATECHANGE, winevent_hook|wparam|lparam, OBJID_CLIENT, 0 }, { WM_GETDLGCODE, sent|wparam|lparam|defwinproc, ID_RADIO1, 0 }, { BM_SETCHECK, sent|wparam|lparam|defwinproc, BST_CHECKED, ID_RADIO1 }, { WM_CTLCOLORSTATIC, sent|parent }, - { EVENT_OBJECT_STATECHANGE, winevent_hook|wparam|lparam|winevent_hook_todo, OBJID_CLIENT, 0 }, + { EVENT_OBJECT_STATECHANGE, winevent_hook|wparam|lparam, OBJID_CLIENT, 0 }, { WM_GETDLGCODE, sent|wparam|lparam|defwinproc, ID_RADIO3, 0 }, { BM_SETCHECK, sent|wparam|lparam|defwinproc, BST_UNCHECKED, ID_RADIO3 }, { WM_CTLCOLORSTATIC, sent|parent }, - { EVENT_OBJECT_STATECHANGE, winevent_hook|wparam|lparam|winevent_hook_todo, OBJID_CLIENT, 0 }, + { EVENT_OBJECT_STATECHANGE, winevent_hook|wparam|lparam, OBJID_CLIENT, 0 }, { WM_GETDLGCODE, sent|wparam|lparam|defwinproc, ID_TEXT, 0 }, { WM_GETDLGCODE, sent|wparam|lparam|defwinproc, ID_RADIO2, 0 }, { BM_SETCHECK, sent|wparam|lparam|defwinproc, BST_UNCHECKED, ID_RADIO2 }, { WM_CTLCOLORSTATIC, sent|parent }, - { EVENT_OBJECT_STATECHANGE, winevent_hook|wparam|lparam|winevent_hook_todo, OBJID_CLIENT, 0 }, + { EVENT_OBJECT_STATECHANGE, winevent_hook|wparam|lparam, OBJID_CLIENT, 0 }, { EVENT_SYSTEM_CAPTUREEND, winevent_hook|wparam|lparam|winevent_hook_todo, 0, 0 }, { WM_CAPTURECHANGED, sent|wparam|lparam|defwinproc, 0, 0 }, { WM_COMMAND, sent|wparam|parent, MAKEWPARAM(ID_RADIO1, BN_CLICKED) }, -- 2.35.1
On 2/8/22 09:17, Alex Henrie wrote:
The NVDA screen reader needs this.
Signed-off-by: Alex Henrie <alexhenrie24(a)gmail.com> --- dlls/comctl32/button.c | 1 + dlls/user32/tests/msg.c | 18 +++++++++--------- 2 files changed, 10 insertions(+), 9 deletions(-)
diff --git a/dlls/comctl32/button.c b/dlls/comctl32/button.c index 88b48a4a5c0..faf0212b256 100644 --- a/dlls/comctl32/button.c +++ b/dlls/comctl32/button.c @@ -656,6 +656,7 @@ static LRESULT CALLBACK BUTTON_WindowProc(HWND hWnd, UINT uMsg, WPARAM wParam, L break; } SendMessageW( hWnd, BM_SETSTATE, FALSE, 0 ); + NotifyWinEvent( EVENT_OBJECT_STATECHANGE, hWnd, OBJID_CLIENT, 0 ); GetClientRect( hWnd, &rect ); if (uMsg == WM_KEYUP || PtInRect( &rect, pt )) { diff --git a/dlls/user32/tests/msg.c b/dlls/user32/tests/msg.c index f0e0d0fba76..cb8d4f07fe7 100644 --- a/dlls/user32/tests/msg.c +++ b/dlls/user32/tests/msg.c @@ -1774,7 +1774,7 @@ static const struct message WmModalDialogSeq[] = { { EVENT_OBJECT_FOCUS, winevent_hook|wparam|lparam, OBJID_CLIENT, 0 }, { WM_KILLFOCUS, sent|parent }, { WM_IME_SETCONTEXT, sent|parent|wparam|optional, 0 }, - { EVENT_OBJECT_STATECHANGE, winevent_hook|wparam|lparam|winevent_hook_todo, 0, 0 }, + { EVENT_OBJECT_STATECHANGE, winevent_hook|wparam|lparam, 0, 0 }, { WM_ENABLE, sent|parent|wparam, 0 }, { HCBT_CREATEWND, hook }, { EVENT_OBJECT_REORDER, winevent_hook|wparam|lparam|optional, 0, 0 }, /* Not sent on Win10. */
Doesn't this still pass in wine, without comctl32 change, with or without winevent_hook_todo? I see we have winetest_platform checks around winevent_hook_todo.
On Mon, Feb 7, 2022 at 11:27 PM Nikolay Sivov <nsivov(a)codeweavers.com> wrote:
--- a/dlls/user32/tests/msg.c +++ b/dlls/user32/tests/msg.c @@ -1774,7 +1774,7 @@ static const struct message WmModalDialogSeq[] = { { EVENT_OBJECT_FOCUS, winevent_hook|wparam|lparam, OBJID_CLIENT, 0 }, { WM_KILLFOCUS, sent|parent }, { WM_IME_SETCONTEXT, sent|parent|wparam|optional, 0 }, - { EVENT_OBJECT_STATECHANGE, winevent_hook|wparam|lparam|winevent_hook_todo, 0, 0 }, + { EVENT_OBJECT_STATECHANGE, winevent_hook|wparam|lparam, 0, 0 }, { WM_ENABLE, sent|parent|wparam, 0 }, { HCBT_CREATEWND, hook }, { EVENT_OBJECT_REORDER, winevent_hook|wparam|lparam|optional, 0, 0 }, /* Not sent on Win10. */
Doesn't this still pass in wine, without comctl32 change, with or without winevent_hook_todo? I see we have winetest_platform checks around winevent_hook_todo.
I don't think the platform check has anything to do with it, but I just noticed that all of these tests pass todo=TRUE to ok_sequence, which means that they do indeed pass whether or not EVENT_OBJECT_STATECHANGE is specifically marked winevent_hook_todo. So you're right, these are not good tests for the feature I'm trying to implement. -Alex
On Mon, Feb 7, 2022 at 11:48 PM Alex Henrie <alexhenrie24(a)gmail.com> wrote:
On Mon, Feb 7, 2022 at 11:27 PM Nikolay Sivov <nsivov(a)codeweavers.com> wrote:
--- a/dlls/user32/tests/msg.c +++ b/dlls/user32/tests/msg.c @@ -1774,7 +1774,7 @@ static const struct message WmModalDialogSeq[] = { { EVENT_OBJECT_FOCUS, winevent_hook|wparam|lparam, OBJID_CLIENT, 0 }, { WM_KILLFOCUS, sent|parent }, { WM_IME_SETCONTEXT, sent|parent|wparam|optional, 0 }, - { EVENT_OBJECT_STATECHANGE, winevent_hook|wparam|lparam|winevent_hook_todo, 0, 0 }, + { EVENT_OBJECT_STATECHANGE, winevent_hook|wparam|lparam, 0, 0 }, { WM_ENABLE, sent|parent|wparam, 0 }, { HCBT_CREATEWND, hook }, { EVENT_OBJECT_REORDER, winevent_hook|wparam|lparam|optional, 0, 0 }, /* Not sent on Win10. */
Doesn't this still pass in wine, without comctl32 change, with or without winevent_hook_todo? I see we have winetest_platform checks around winevent_hook_todo.
I don't think the platform check has anything to do with it, but I just noticed that all of these tests pass todo=TRUE to ok_sequence, which means that they do indeed pass whether or not EVENT_OBJECT_STATECHANGE is specifically marked winevent_hook_todo. So you're right, these are not good tests for the feature I'm trying to implement.
This must be what you were looking at: /* * silently drop hook messages if there is no support for them, mark * winevent todo's. */ else if ((expected->flags & optional) || ((expected->flags & hook) && !hCBT_hook) || ((expected->flags & winevent_hook) && !hEvent_hook) || ((expected->flags & kbd_hook) && !hKBD_hook) || ((expected->flags & winevent_hook_todo) && !strcmp(winetest_platform, "wine"))) Apparently ok_sequence treats all hooks as optional? That doesn't seem helpful... -Alex
On Tue, Feb 8, 2022 at 12:08 AM Alex Henrie <alexhenrie24(a)gmail.com> wrote:
On Mon, Feb 7, 2022 at 11:48 PM Alex Henrie <alexhenrie24(a)gmail.com> wrote:
On Mon, Feb 7, 2022 at 11:27 PM Nikolay Sivov <nsivov(a)codeweavers.com> wrote:
--- a/dlls/user32/tests/msg.c +++ b/dlls/user32/tests/msg.c @@ -1774,7 +1774,7 @@ static const struct message WmModalDialogSeq[] = { { EVENT_OBJECT_FOCUS, winevent_hook|wparam|lparam, OBJID_CLIENT, 0 }, { WM_KILLFOCUS, sent|parent }, { WM_IME_SETCONTEXT, sent|parent|wparam|optional, 0 }, - { EVENT_OBJECT_STATECHANGE, winevent_hook|wparam|lparam|winevent_hook_todo, 0, 0 }, + { EVENT_OBJECT_STATECHANGE, winevent_hook|wparam|lparam, 0, 0 }, { WM_ENABLE, sent|parent|wparam, 0 }, { HCBT_CREATEWND, hook }, { EVENT_OBJECT_REORDER, winevent_hook|wparam|lparam|optional, 0, 0 }, /* Not sent on Win10. */
Doesn't this still pass in wine, without comctl32 change, with or without winevent_hook_todo? I see we have winetest_platform checks around winevent_hook_todo.
I don't think the platform check has anything to do with it, but I just noticed that all of these tests pass todo=TRUE to ok_sequence, which means that they do indeed pass whether or not EVENT_OBJECT_STATECHANGE is specifically marked winevent_hook_todo. So you're right, these are not good tests for the feature I'm trying to implement.
This must be what you were looking at:
/* * silently drop hook messages if there is no support for them, mark * winevent todo's. */ else if ((expected->flags & optional) || ((expected->flags & hook) && !hCBT_hook) || ((expected->flags & winevent_hook) && !hEvent_hook) || ((expected->flags & kbd_hook) && !hKBD_hook) || ((expected->flags & winevent_hook_todo) && !strcmp(winetest_platform, "wine")))
Apparently ok_sequence treats all hooks as optional? That doesn't seem helpful...
It took me a minute but I think I get it: If we're expecting a hook and don't get one, it's OK. But if we do get a hook, it had better be the right one. More importantly, I missed the fact that there are two (entirely?) separate checkbox implementations in Windows: One in user32 and one in comctl32. The one that NVDA uses is definitely in comctl32, but comctl32's checkbox tests will have to be expanded to test hooks. -Alex
On 2/8/22 11:23, Alex Henrie wrote:
On Tue, Feb 8, 2022 at 12:08 AM Alex Henrie <alexhenrie24(a)gmail.com> wrote:
On Mon, Feb 7, 2022 at 11:48 PM Alex Henrie <alexhenrie24(a)gmail.com> wrote:
On Mon, Feb 7, 2022 at 11:27 PM Nikolay Sivov <nsivov(a)codeweavers.com> wrote:
--- a/dlls/user32/tests/msg.c +++ b/dlls/user32/tests/msg.c @@ -1774,7 +1774,7 @@ static const struct message WmModalDialogSeq[] = { { EVENT_OBJECT_FOCUS, winevent_hook|wparam|lparam, OBJID_CLIENT, 0 }, { WM_KILLFOCUS, sent|parent }, { WM_IME_SETCONTEXT, sent|parent|wparam|optional, 0 }, - { EVENT_OBJECT_STATECHANGE, winevent_hook|wparam|lparam|winevent_hook_todo, 0, 0 }, + { EVENT_OBJECT_STATECHANGE, winevent_hook|wparam|lparam, 0, 0 }, { WM_ENABLE, sent|parent|wparam, 0 }, { HCBT_CREATEWND, hook }, { EVENT_OBJECT_REORDER, winevent_hook|wparam|lparam|optional, 0, 0 }, /* Not sent on Win10. */
Doesn't this still pass in wine, without comctl32 change, with or without winevent_hook_todo? I see we have winetest_platform checks around winevent_hook_todo. I don't think the platform check has anything to do with it, but I just noticed that all of these tests pass todo=TRUE to ok_sequence, which means that they do indeed pass whether or not EVENT_OBJECT_STATECHANGE is specifically marked winevent_hook_todo. So you're right, these are not good tests for the feature I'm trying to implement. This must be what you were looking at:
/* * silently drop hook messages if there is no support for them, mark * winevent todo's. */ else if ((expected->flags & optional) || ((expected->flags & hook) && !hCBT_hook) || ((expected->flags & winevent_hook) && !hEvent_hook) || ((expected->flags & kbd_hook) && !hKBD_hook) || ((expected->flags & winevent_hook_todo) && !strcmp(winetest_platform, "wine")))
Apparently ok_sequence treats all hooks as optional? That doesn't seem helpful... It took me a minute but I think I get it: If we're expecting a hook and don't get one, it's OK. But if we do get a hook, it had better be the right one.
More importantly, I missed the fact that there are two (entirely?) separate checkbox implementations in Windows: One in user32 and one in comctl32. The one that NVDA uses is definitely in comctl32, but comctl32's checkbox tests will have to be expanded to test hooks.
Yes, that's what caught my attention, why comctl32 button would affect user32 tests.
-Alex
Hi, While running your changed tests, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=107294 Your paranoid android. === w8 (32 bit report) === user32: msg.c:15241: Test failed: unexpected message 31f msg.c:15242: Test failed: bad wparam 1 msg.c:15248: Test failed: unicode WM_CHAR: 0: the msg sequence is not complete: expected 0102 - actual 0000 === debian11 (32 bit French report) === user32: msg.c:9209: Test failed: WaitForSingleObject failed 102 msg.c:9215: Test failed: destroy child on thread exit: 0: the msg 0x0082 was expected, but got msg 0x000f instead msg.c:9215: Test failed: destroy child on thread exit: 1: the msg 0x000f was expected, but got msg 0x0014 instead msg.c:9215: Test failed: destroy child on thread exit: 2: the msg sequence is not complete: expected 0014 - actual 0000
participants (3)
-
Alex Henrie -
Marvin -
Nikolay Sivov