[PATCH v3 0/2] MR5761: comclt32: Allow ctrl-tab and tab hotkeys for propsheets
Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=56103 -- v3: comclt32: Allow hotkeys for propsheet navigation https://gitlab.winehq.org/wine/wine/-/merge_requests/5761
From: Fabian Maurer <dark.shadow4(a)web.de> --- dlls/comctl32/tests/propsheet.c | 188 ++++++++++++++++++++++++++++++++ 1 file changed, 188 insertions(+) diff --git a/dlls/comctl32/tests/propsheet.c b/dlls/comctl32/tests/propsheet.c index 9bd1c08bd6a..11065910284 100644 --- a/dlls/comctl32/tests/propsheet.c +++ b/dlls/comctl32/tests/propsheet.c @@ -302,6 +302,8 @@ static void test_disableowner(void) DestroyWindow(parenthwnd); } +HANDLE active_page_changed; + static INT_PTR CALLBACK nav_page_proc(HWND hwnd, UINT msg, WPARAM wparam, LPARAM lparam) { switch(msg){ @@ -312,6 +314,8 @@ static INT_PTR CALLBACK nav_page_proc(HWND hwnd, UINT msg, WPARAM wparam, LPARAM case PSN_SETACTIVE: active_page = /* PropSheet_HwndToIndex(hdr->hwndFrom, hwnd); */ (int)SendMessageA(hdr->hwndFrom, PSM_HWNDTOINDEX, (WPARAM)hwnd, 0); + if (active_page_changed) + SetEvent(active_page_changed); return TRUE; case PSN_KILLACTIVE: /* prevent navigation away from the fourth page */ @@ -1388,6 +1392,189 @@ static void test_invalid_hpropsheetpage(void) DestroyWindow(hdlg); } +#define WM_DIALOG_LOADED (WM_USER + 0x99) + +static WNDPROC old_hotkeys_dialog_proc; +static HWND hotkey_dialog; +static HANDLE hotkey_dialog_loaded; + +static LRESULT CALLBACK new_hotkeys_dialog_proc(HWND hwnd, UINT msg, WPARAM wparam, LPARAM lparam) +{ + switch(msg) + { + case WM_INITDIALOG: + hotkey_dialog = hwnd; + PostMessageW(hwnd, WM_DIALOG_LOADED, 0, 0); + break; + + case WM_DIALOG_LOADED: + SetEvent(hotkey_dialog_loaded); + return TRUE; + } + + return CallWindowProcW(old_hotkeys_dialog_proc, hwnd, msg, wparam, lparam); +} + + +static LRESULT CALLBACK hook_proc_hotkeys(int code, WPARAM wparam, LPARAM lparam) +{ + static BOOL done; + if (code == HCBT_CREATEWND) + { + CBT_CREATEWNDW *c = (CBT_CREATEWNDW *)lparam; + + /* The first dialog created will be the parent dialog */ + if (!done && c->lpcs->lpszClass == (LPWSTR)WC_DIALOG) + { + old_hotkeys_dialog_proc = (WNDPROC)SetWindowLongPtrW((HWND)wparam, GWLP_WNDPROC, (LONG_PTR)new_hotkeys_dialog_proc); + done = TRUE; + } + } + + return CallNextHookEx(NULL, code, wparam, lparam); +} + +static DWORD WINAPI hotkey_dialog_thread(void *arg) +{ + HPROPSHEETPAGE hpsp[4]; + PROPSHEETPAGEA psp[4]; + PROPSHEETHEADERA psh; + HHOOK hook; + + /* set up a hook proc in order to subclass the main dialog early on */ + hook = SetWindowsHookExW(WH_CBT, hook_proc_hotkeys, NULL, GetCurrentThreadId()); + + /* create the property sheet pages */ + memset(psp, 0, sizeof(PROPSHEETPAGEA) * 4); + + psp[0].dwSize = sizeof(PROPSHEETPAGEA); + psp[0].hInstance = GetModuleHandleA(NULL); + psp[0].pszTemplate = (LPCSTR)MAKEINTRESOURCE(IDD_PROP_PAGE_INTRO); + psp[0].pfnDlgProc = nav_page_proc; + hpsp[0] = pCreatePropertySheetPageA(&psp[0]); + + psp[1].dwSize = sizeof(PROPSHEETPAGEA); + psp[1].hInstance = GetModuleHandleA(NULL); + psp[1].pszTemplate = (LPCSTR)MAKEINTRESOURCE(IDD_PROP_PAGE_RADIO); + psp[1].pfnDlgProc = nav_page_proc; + hpsp[1] = pCreatePropertySheetPageA(&psp[1]); + + psp[2].dwSize = sizeof(PROPSHEETPAGEA); + psp[2].hInstance = GetModuleHandleA(NULL); + psp[2].pszTemplate = (LPCSTR)MAKEINTRESOURCE(IDD_PROP_PAGE_RADIO); + psp[2].pfnDlgProc = nav_page_proc; + hpsp[2] = pCreatePropertySheetPageA(&psp[2]); + + psp[3].dwSize = sizeof(PROPSHEETPAGEA); + psp[3].hInstance = GetModuleHandleA(NULL); + psp[3].pszTemplate = (LPCSTR)MAKEINTRESOURCE(IDD_PROP_PAGE_EXIT); + psp[3].pfnDlgProc = nav_page_proc; + hpsp[3] = pCreatePropertySheetPageA(&psp[3]); + + /* set up the property sheet dialog */ + memset(&psh, 0, sizeof(psh)); + psh.dwSize = PROPSHEETHEADERA_V1_SIZE; + psh.dwFlags = PSH_WIZARD; + psh.pszCaption = "A Wizard"; + psh.nPages = 4; + psh.hwndParent = GetDesktopWindow(); + psh.phpage = hpsp; + pPropertySheetA(&psh); + + UnhookWindowsHookEx(hook); + + return 0; +} + +static void test_hotkey_navigation(void) +{ + HWND control; + HANDLE thread; + int i; + INPUT inputs[6] = {0}; + + for (i = 0; i < ARRAY_SIZE(inputs); i++) + { + inputs[i].type = INPUT_KEYBOARD; + } + + active_page_changed = CreateEventW(NULL, FALSE, FALSE, NULL); + ok(active_page_changed != NULL, "CreateEvent failed, error %lu\n", GetLastError()); + hotkey_dialog_loaded = CreateEventW(NULL, FALSE, FALSE, NULL); + ok(hotkey_dialog_loaded != NULL, "CreateEvent failed, error %lu\n", GetLastError()); + + thread = CreateThread(NULL, 0, hotkey_dialog_thread, NULL, 0, NULL); + + WaitForSingleObject(hotkey_dialog_loaded, INFINITE); + ResetEvent(active_page_changed); + SetFocus(hotkey_dialog); + + control = (HWND)SendMessageA(hotkey_dialog, PSM_GETCURRENTPAGEHWND, 0, 0); + active_page = (LONG)SendMessageA(hotkey_dialog, PSM_HWNDTOINDEX, (WPARAM)control, 0); + ok(active_page == 0, "Got %ld\n", active_page); + + inputs[0].ki.wVk = VK_CONTROL; + inputs[0].ki.dwFlags = 0; + inputs[1].ki.wVk = VK_NEXT; + inputs[1].ki.dwFlags = 0; + inputs[2].ki.wVk = VK_NEXT; + inputs[2].ki.dwFlags = KEYEVENTF_KEYUP; + inputs[3].ki.wVk = VK_CONTROL; + inputs[3].ki.dwFlags = KEYEVENTF_KEYUP; + SendInput(4, inputs, sizeof(INPUT)); + WaitForSingleObject(active_page_changed, 200); + todo_wine + ok(active_page == 1, "Got %ld\n", active_page); + + inputs[0].ki.wVk = VK_CONTROL; + inputs[0].ki.dwFlags = 0; + inputs[1].ki.wVk = VK_PRIOR; + inputs[1].ki.dwFlags = 0; + inputs[2].ki.wVk = VK_PRIOR; + inputs[2].ki.dwFlags = KEYEVENTF_KEYUP; + inputs[3].ki.wVk = VK_CONTROL; + inputs[3].ki.dwFlags = KEYEVENTF_KEYUP; + SendInput(4, inputs, sizeof(INPUT)); + WaitForSingleObject(active_page_changed, 200); + ok(active_page == 0, "Got %ld\n", active_page); + + inputs[0].ki.wVk = VK_CONTROL; + inputs[0].ki.dwFlags = 0; + inputs[1].ki.wVk = VK_TAB; + inputs[1].ki.dwFlags = 0; + inputs[2].ki.wVk = VK_TAB; + inputs[2].ki.dwFlags = KEYEVENTF_KEYUP; + inputs[3].ki.wVk = VK_CONTROL; + inputs[3].ki.dwFlags = KEYEVENTF_KEYUP; + SendInput(4, inputs, sizeof(INPUT)); + WaitForSingleObject(active_page_changed, 200); + todo_wine + ok(active_page == 1, "Got %ld\n", active_page); + + inputs[0].ki.wVk = VK_CONTROL; + inputs[0].ki.dwFlags = 0; + inputs[1].ki.wVk = VK_SHIFT; + inputs[1].ki.dwFlags = 0; + inputs[2].ki.wVk = VK_TAB; + inputs[2].ki.dwFlags = 0; + inputs[3].ki.wVk = VK_TAB; + inputs[3].ki.dwFlags = KEYEVENTF_KEYUP; + inputs[4].ki.wVk = VK_SHIFT; + inputs[4].ki.dwFlags = KEYEVENTF_KEYUP; + inputs[5].ki.wVk = VK_CONTROL; + inputs[5].ki.dwFlags = KEYEVENTF_KEYUP; + SendInput(6, inputs, sizeof(INPUT)); + WaitForSingleObject(active_page_changed, 200); + ok(active_page == 0, "Got %ld\n", active_page); + + PostMessageA(hotkey_dialog, WM_CLOSE, 0, 0); + WaitForSingleObject(thread, INFINITE); + CloseHandle(thread); + CloseHandle(hotkey_dialog_loaded); + CloseHandle(active_page_changed); + active_page_changed = NULL; +} + static void init_comctl32_functions(void) { HMODULE hComCtl32 = LoadLibraryA("comctl32.dll"); @@ -1442,6 +1629,7 @@ START_TEST(propsheet) test_CreatePropertySheetPage(); test_page_dialog_texture(); test_invalid_hpropsheetpage(); + test_hotkey_navigation(); if (!load_v6_module(&ctx_cookie, &ctx)) return; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5761
From: Fabian Maurer <dark.shadow4(a)web.de> Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=56103 --- dlls/comctl32/propsheet.c | 108 ++++++++++++++++---------------- dlls/comctl32/tests/propsheet.c | 2 - 2 files changed, 54 insertions(+), 56 deletions(-) diff --git a/dlls/comctl32/propsheet.c b/dlls/comctl32/propsheet.c index b530a1a8094..fe8a4c68333 100644 --- a/dlls/comctl32/propsheet.c +++ b/dlls/comctl32/propsheet.c @@ -2884,6 +2884,59 @@ static void PROPSHEET_CleanUp(HWND hwndDlg) GlobalFree(psInfo); } +/****************************************************************************** + * PROPSHEET_IsDialogMessage + */ +static BOOL PROPSHEET_IsDialogMessage(HWND hwnd, LPMSG lpMsg) +{ + PropSheetInfo* psInfo = GetPropW(hwnd, PropSheetInfoStr); + + TRACE("\n"); + if (!psInfo || (hwnd != lpMsg->hwnd && !IsChild(hwnd, lpMsg->hwnd))) + return FALSE; + + if (lpMsg->message == WM_KEYDOWN && (GetKeyState(VK_CONTROL) & 0x8000)) + { + int new_page = 0; + INT dlgCode = SendMessageW(lpMsg->hwnd, WM_GETDLGCODE, 0, (LPARAM)lpMsg); + + if (!(dlgCode & DLGC_WANTMESSAGE)) + { + switch (lpMsg->wParam) + { + case VK_TAB: + if (GetKeyState(VK_SHIFT) & 0x8000) + new_page = -1; + else + new_page = 1; + break; + + case VK_NEXT: new_page = 1; break; + case VK_PRIOR: new_page = -1; break; + } + } + + if (new_page) + { + if (PROPSHEET_CanSetCurSel(hwnd) != FALSE) + { + new_page += psInfo->active_page; + + if (new_page < 0) + new_page = psInfo->nPages - 1; + else if (new_page >= psInfo->nPages) + new_page = 0; + + PROPSHEET_SetCurSel(hwnd, new_page, 1, 0); + } + + return TRUE; + } + } + + return IsDialogMessageW(hwnd, lpMsg); +} + static INT do_loop(const PropSheetInfo *psInfo) { MSG msg = { 0 }; @@ -2896,7 +2949,7 @@ static INT do_loop(const PropSheetInfo *psInfo) if(ret == -1) break; - if(!IsDialogMessageW(hwnd, &msg)) + if(!PROPSHEET_IsDialogMessage(hwnd, &msg)) { TranslateMessage(&msg); DispatchMessageW(&msg); @@ -3234,59 +3287,6 @@ BOOL WINAPI DestroyPropertySheetPage(HPROPSHEETPAGE hpsp) return TRUE; } -/****************************************************************************** - * PROPSHEET_IsDialogMessage - */ -static BOOL PROPSHEET_IsDialogMessage(HWND hwnd, LPMSG lpMsg) -{ - PropSheetInfo* psInfo = GetPropW(hwnd, PropSheetInfoStr); - - TRACE("\n"); - if (!psInfo || (hwnd != lpMsg->hwnd && !IsChild(hwnd, lpMsg->hwnd))) - return FALSE; - - if (lpMsg->message == WM_KEYDOWN && (GetKeyState(VK_CONTROL) & 0x8000)) - { - int new_page = 0; - INT dlgCode = SendMessageW(lpMsg->hwnd, WM_GETDLGCODE, 0, (LPARAM)lpMsg); - - if (!(dlgCode & DLGC_WANTMESSAGE)) - { - switch (lpMsg->wParam) - { - case VK_TAB: - if (GetKeyState(VK_SHIFT) & 0x8000) - new_page = -1; - else - new_page = 1; - break; - - case VK_NEXT: new_page = 1; break; - case VK_PRIOR: new_page = -1; break; - } - } - - if (new_page) - { - if (PROPSHEET_CanSetCurSel(hwnd) != FALSE) - { - new_page += psInfo->active_page; - - if (new_page < 0) - new_page = psInfo->nPages - 1; - else if (new_page >= psInfo->nPages) - new_page = 0; - - PROPSHEET_SetCurSel(hwnd, new_page, 1, 0); - } - - return TRUE; - } - } - - return IsDialogMessageW(hwnd, lpMsg); -} - /****************************************************************************** * PROPSHEET_DoCommand */ diff --git a/dlls/comctl32/tests/propsheet.c b/dlls/comctl32/tests/propsheet.c index 11065910284..d9a1ac817a1 100644 --- a/dlls/comctl32/tests/propsheet.c +++ b/dlls/comctl32/tests/propsheet.c @@ -1523,7 +1523,6 @@ static void test_hotkey_navigation(void) inputs[3].ki.dwFlags = KEYEVENTF_KEYUP; SendInput(4, inputs, sizeof(INPUT)); WaitForSingleObject(active_page_changed, 200); - todo_wine ok(active_page == 1, "Got %ld\n", active_page); inputs[0].ki.wVk = VK_CONTROL; @@ -1548,7 +1547,6 @@ static void test_hotkey_navigation(void) inputs[3].ki.dwFlags = KEYEVENTF_KEYUP; SendInput(4, inputs, sizeof(INPUT)); WaitForSingleObject(active_page_changed, 200); - todo_wine ok(active_page == 1, "Got %ld\n", active_page); inputs[0].ki.wVk = VK_CONTROL; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5761
On Wed Jun 5 22:22:58 2024 +0000, Fabian Maurer wrote:
No problem. Turns out it's a bit more difficult testing this than I thought, but I think I have a properly working approach now. I'll push an update the coming days. The tests are pushed, is that approach alright?
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/5761#note_72987
Zhiyi Zhang (@zhiyi) commented about dlls/comctl32/tests/propsheet.c:
+ psp[1].hInstance = GetModuleHandleA(NULL); + psp[1].pszTemplate = (LPCSTR)MAKEINTRESOURCE(IDD_PROP_PAGE_RADIO); + psp[1].pfnDlgProc = nav_page_proc; + hpsp[1] = pCreatePropertySheetPageA(&psp[1]); + + psp[2].dwSize = sizeof(PROPSHEETPAGEA); + psp[2].hInstance = GetModuleHandleA(NULL); + psp[2].pszTemplate = (LPCSTR)MAKEINTRESOURCE(IDD_PROP_PAGE_RADIO); + psp[2].pfnDlgProc = nav_page_proc; + hpsp[2] = pCreatePropertySheetPageA(&psp[2]); + + psp[3].dwSize = sizeof(PROPSHEETPAGEA); + psp[3].hInstance = GetModuleHandleA(NULL); + psp[3].pszTemplate = (LPCSTR)MAKEINTRESOURCE(IDD_PROP_PAGE_EXIT); + psp[3].pfnDlgProc = nav_page_proc; + hpsp[3] = pCreatePropertySheetPageA(&psp[3]); Let's use a loop for this. And do you really need 4 pages? It seems to me 2 pages are enough for this test.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/5761#note_73044
Zhiyi Zhang (@zhiyi) commented about dlls/comctl32/tests/propsheet.c:
DestroyWindow(parenthwnd); }
Could you add a dot at the the patch commit subject message? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5761#note_73045
Zhiyi Zhang (@zhiyi) commented about dlls/comctl32/tests/propsheet.c:
+ { + case WM_INITDIALOG: + hotkey_dialog = hwnd; + PostMessageW(hwnd, WM_DIALOG_LOADED, 0, 0); + break; + + case WM_DIALOG_LOADED: + SetEvent(hotkey_dialog_loaded); + return TRUE; + } + + return CallWindowProcW(old_hotkeys_dialog_proc, hwnd, msg, wparam, lparam); +} + + +static LRESULT CALLBACK hook_proc_hotkeys(int code, WPARAM wparam, LPARAM lparam) It seems to me that all this hooking is to get the dialog window handle and wait for it to be initialized. I think you can achieve the same thing with PROPSHEETHEADER.pfnCallback and wait for PSCB_INITIALIZED. See sheet_callback() in comctl32/tests/propsheet.c for example.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/5761#note_73046
On Thu Jun 13 07:26:33 2024 +0000, Zhiyi Zhang wrote:
Could you add a dot at the the patch commit subject message? Sure, done
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/5761#note_73077
On Thu Jun 13 07:28:29 2024 +0000, Zhiyi Zhang wrote:
It seems to me that all this hooking is to get the dialog window handle and wait for it to be initialized. I think you can achieve the same thing with PROPSHEETHEADER.pfnCallback and wait for PSCB_INITIALIZED. See sheet_callback() in comctl32/tests/propsheet.c for example. Yes, it only works when we're in that message loop. I tried the callback, but it's too early, it simply doesn't process the inputs properly. That's why I have that work around where I sent a message myself, and once it's received, we must be processing messages.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/5761#note_73078
On Thu Jun 13 16:56:51 2024 +0000, Fabian Maurer wrote:
Yes, it only works when we're in that message loop. I tried the callback, but it's too early, it simply doesn't process the inputs properly. That's why I have that work around where I sent a message myself, and once it's received, we must be processing messages. Okay. I see. Let's keep it as it is then.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/5761#note_73171
On Fri Jun 14 09:06:42 2024 +0000, Zhiyi Zhang wrote:
Okay. I see. Let's keep it as it is then. Hi, could you push an update addressing the comments?
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/5761#note_73822
participants (3)
-
Fabian Maurer -
Fabian Maurer (＠DarkShadow44)
-
Zhiyi Zhang (＠zhiyi)