Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=56103
-- v6: comclt32: Allow hotkeys for propsheet navigation.
From: Fabian Maurer dark.shadow4@web.de
--- dlls/comctl32/tests/propsheet.c | 174 ++++++++++++++++++++++++++++++++ 1 file changed, 174 insertions(+)
diff --git a/dlls/comctl32/tests/propsheet.c b/dlls/comctl32/tests/propsheet.c index 9bd1c08bd6a..599583007ec 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,175 @@ 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[2]; + PROPSHEETPAGEA psp[2]; + PROPSHEETHEADERA psh; + HHOOK hook; + int i; + + /* 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) * ARRAY_SIZE(psp)); + + for (i = 0; i < ARRAY_SIZE(psp); i++) + { + psp[i].dwSize = sizeof(PROPSHEETPAGEA); + psp[i].hInstance = GetModuleHandleA(NULL); + psp[i].pszTemplate = (LPCSTR)MAKEINTRESOURCE(IDD_PROP_PAGE_RADIO); + psp[i].pfnDlgProc = nav_page_proc; + hpsp[i] = pCreatePropertySheetPageA(&psp[i]); + } + + /* 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 = ARRAY_SIZE(psp); + 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 +1615,7 @@ START_TEST(propsheet) test_CreatePropertySheetPage(); test_page_dialog_texture(); test_invalid_hpropsheetpage(); + test_hotkey_navigation();
if (!load_v6_module(&ctx_cookie, &ctx)) return;
From: Fabian Maurer dark.shadow4@web.de
Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=56103 --- dlls/comctl32/propsheet.c | 108 ++++++++++++++++---------------- dlls/comctl32/tests/propsheet.c | 10 ++- 2 files changed, 58 insertions(+), 60 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 599583007ec..a44909f420f 100644 --- a/dlls/comctl32/tests/propsheet.c +++ b/dlls/comctl32/tests/propsheet.c @@ -1508,8 +1508,7 @@ static void test_hotkey_navigation(void) inputs[3].ki.wVk = VK_CONTROL; inputs[3].ki.dwFlags = KEYEVENTF_KEYUP; SendInput(4, inputs, sizeof(INPUT)); - WaitForSingleObject(active_page_changed, 200); - todo_wine + WaitForSingleObject(active_page_changed, 150); ok(active_page == 1, "Got %ld\n", active_page);
inputs[0].ki.wVk = VK_CONTROL; @@ -1521,7 +1520,7 @@ static void test_hotkey_navigation(void) inputs[3].ki.wVk = VK_CONTROL; inputs[3].ki.dwFlags = KEYEVENTF_KEYUP; SendInput(4, inputs, sizeof(INPUT)); - WaitForSingleObject(active_page_changed, 200); + WaitForSingleObject(active_page_changed, 150); ok(active_page == 0, "Got %ld\n", active_page);
inputs[0].ki.wVk = VK_CONTROL; @@ -1533,8 +1532,7 @@ static void test_hotkey_navigation(void) inputs[3].ki.wVk = VK_CONTROL; inputs[3].ki.dwFlags = KEYEVENTF_KEYUP; SendInput(4, inputs, sizeof(INPUT)); - WaitForSingleObject(active_page_changed, 200); - todo_wine + WaitForSingleObject(active_page_changed, 150); ok(active_page == 1, "Got %ld\n", active_page);
inputs[0].ki.wVk = VK_CONTROL; @@ -1550,7 +1548,7 @@ static void test_hotkey_navigation(void) inputs[5].ki.wVk = VK_CONTROL; inputs[5].ki.dwFlags = KEYEVENTF_KEYUP; SendInput(6, inputs, sizeof(INPUT)); - WaitForSingleObject(active_page_changed, 200); + WaitForSingleObject(active_page_changed, 150); ok(active_page == 0, "Got %ld\n", active_page);
PostMessageA(hotkey_dialog, WM_CLOSE, 0, 0);
On Mon Jun 24 22:24:39 2024 +0000, Zhiyi Zhang wrote:
There is a test failure on Windows 10 21H1 Arabic. See https://testbot.winehq.org/JobDetails.pl?Key=146448#k309. Please take a look at how it should work on Arabic locale on Windows.
I reran the tests manually by uploading comctl32_test to the testbot, and there were no issues. I also restarted the pipeline by making a simple change, and now all is green.. I think that test failure isn't related to Arabic, it looks more like a spurious failure.
Either the test I made is inherently flaky or it just ran into the timeout, although 200ms seemed plenty to me... I can't reproduce it locally either, even when running the test hundreds of times.
I'd like to fix the test though, can you advice me on how to approach that?
On Mon Jun 24 22:25:05 2024 +0000, Fabian Maurer wrote:
I reran the tests manually by uploading comctl32_test to the testbot, and there were no issues. I also restarted the pipeline by making a simple change, and now all is green.. I think that test failure isn't related to Arabic, it looks more like a spurious failure. Either the test I made is inherently flaky or it just ran into the timeout, although 200ms seemed plenty to me... I can't reproduce it locally either, even when running the test hundreds of times. I'd like to fix the test though, can you advice me on how to approach that?
Did you try with a larger timeout or Windows 10 21H1 in Arabic (Egypt) locale?
I would also try making the dialog foreground to ensure the input is not sent to other windows. If all that fails, you can add a flaky_if(in_arabic_locale).
On Tue Jun 25 01:02:26 2024 +0000, Zhiyi Zhang wrote:
Did you try with a larger timeout or Windows 10 21H1 in Arabic (Egypt) locale? I would also try making the dialog foreground to ensure the input is not sent to other windows. If all that fails, you can add a flaky_if(in_arabic_locale).
I reduced the timeout to 150ms, and the tests work everywhere: https://testbot.winehq.org/JobDetails.pl?Key=146528
As noted, I don't think it's Arabic only. But I don't feel comfortable merging it with a higher timeout unless I know what caused it. Problem is that I can't really reproduce the issue, so I don't know how to fix it either. Maybe making it foreground would help, but how would I know?
On Tue Jun 25 17:55:32 2024 +0000, Fabian Maurer wrote:
I reduced the timeout to 150ms, and the tests work everywhere: https://testbot.winehq.org/JobDetails.pl?Key=146528 As noted, I don't think it's Arabic only. But I don't feel comfortable merging it with a higher timeout unless I know what caused it. Problem is that I can't really reproduce the issue, so I don't know how to fix it either. Maybe making it foreground would help, but how would I know?
If you run it on TestBots multiple times ( >=5 ) and can't reproduce the issue anymore, let's consider it fixed. If it still appears in future runs, let's add a flaky.
-
Fabian Maurer -
Fabian Maurer (＠DarkShadow44)
-
Zhiyi Zhang (＠zhiyi)