[PATCH v10 0/2] MR5761: comclt32: Allow ctrl-tab and tab hotkeys for propsheets

[Fabian Maurer (＠DarkShadow44)]

Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=56103

--
  v10: comclt32: Allow hotkeys for propsheet navigation.

https://gitlab.winehq.org/wine/wine/-/merge_requests/5761

[PATCH v10 1/2] comctl32/tests: Add test for propsheet hotkey navigation.

[Fabian Maurer]

From: Fabian Maurer

---
 dlls/comctl32/tests/propsheet.c | 185 ++++++++++++++++++++++++++++++++
 1 file changed, 185 insertions(+)

diff --git a/dlls/comctl32/tests/propsheet.c b/dlls/comctl32/tests/propsheet.c
index 9bd1c08bd6a..a7953135992 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,186 @@ static void test_invalid_hpropsheetpage(void)
     DestroyWindow(hdlg);
 }

+#define WM_DIALOG_IDLE (WM_USER + 0x99)
+
+static WNDPROC old_hotkeys_dialog_proc;
+static HWND hotkey_dialog;
+static HANDLE hotkey_dialog_idle;
+
+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_IDLE, 0, 0);
+        break;
+
+    case WM_DIALOG_IDLE:
+        SetEvent(hotkey_dialog_idle);
+        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};
+    BOOL success;
+
+    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_idle = CreateEventW(NULL, FALSE, FALSE, NULL);
+    ok(hotkey_dialog_idle != NULL, "CreateEvent failed, error %lu\n", GetLastError());
+
+    thread = CreateThread(NULL, 0, hotkey_dialog_thread, NULL, 0, NULL);
+
+    WaitForSingleObject(hotkey_dialog_idle, INFINITE);
+    ResetEvent(active_page_changed);
+    success = SetForegroundWindow(hotkey_dialog);
+    ok(success, "SetForegroundWindow failed\n");
+    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, 1000);
+    todo_wine
+    ok(active_page == 1, "Got %ld\n", active_page);
+    PostMessageW(hotkey_dialog, WM_DIALOG_IDLE, 0, 0);
+    WaitForSingleObject(hotkey_dialog_idle, INFINITE);
+
+    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, 1000);
+    ok(active_page == 0, "Got %ld\n", active_page);
+    PostMessageW(hotkey_dialog, WM_DIALOG_IDLE, 0, 0);
+    WaitForSingleObject(hotkey_dialog_idle, INFINITE);
+
+    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, 1000);
+    todo_wine
+    ok(active_page == 1, "Got %ld\n", active_page);
+    PostMessageW(hotkey_dialog, WM_DIALOG_IDLE, 0, 0);
+    WaitForSingleObject(hotkey_dialog_idle, INFINITE);
+
+    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, 1000);
+    ok(active_page == 0, "Got %ld\n", active_page);
+    PostMessageW(hotkey_dialog, WM_DIALOG_IDLE, 0, 0);
+    WaitForSingleObject(hotkey_dialog_idle, INFINITE);
+
+    PostMessageA(hotkey_dialog, WM_CLOSE, 0, 0);
+    WaitForSingleObject(thread, INFINITE);
+    CloseHandle(thread);
+    CloseHandle(hotkey_dialog_idle);
+    CloseHandle(active_page_changed);
+    active_page_changed = NULL;
+}
+
 static void init_comctl32_functions(void)
 {
     HMODULE hComCtl32 = LoadLibraryA("comctl32.dll");
@@ -1442,6 +1626,7 @@ START_TEST(propsheet)
     test_CreatePropertySheetPage();
     test_page_dialog_texture();
     test_invalid_hpropsheetpage();
+    test_hotkey_navigation();

if (!load_v6_module(&ctx_cookie, &ctx))
         return;

[PATCH v10 2/2] comclt32: Allow hotkeys for propsheet navigation.

[Fabian Maurer]

From: Fabian Maurer

Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=56103
---
 dlls/comctl32/propsheet.c       | 108 ++++++++++++++++----------------
 dlls/comctl32/tests/propsheet.c |  14 ++---
 2 files changed, 58 insertions(+), 64 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 a7953135992..799532a6194 100644
--- a/dlls/comctl32/tests/propsheet.c
+++ b/dlls/comctl32/tests/propsheet.c
@@ -1512,10 +1512,8 @@ static void test_hotkey_navigation(void)
     inputs[3].ki.dwFlags = KEYEVENTF_KEYUP;
     SendInput(4, inputs, sizeof(INPUT));
     WaitForSingleObject(active_page_changed, 1000);
-    todo_wine
     ok(active_page == 1, "Got %ld\n", active_page);
-    PostMessageW(hotkey_dialog, WM_DIALOG_IDLE, 0, 0);
-    WaitForSingleObject(hotkey_dialog_idle, INFINITE);
+    SendMessageA(hotkey_dialog, WM_NULL, 0, 0);

inputs[0].ki.wVk = VK_CONTROL;
     inputs[0].ki.dwFlags = 0;
@@ -1528,8 +1526,7 @@ static void test_hotkey_navigation(void)
     SendInput(4, inputs, sizeof(INPUT));
     WaitForSingleObject(active_page_changed, 1000);
     ok(active_page == 0, "Got %ld\n", active_page);
-    PostMessageW(hotkey_dialog, WM_DIALOG_IDLE, 0, 0);
-    WaitForSingleObject(hotkey_dialog_idle, INFINITE);
+    SendMessageA(hotkey_dialog, WM_NULL, 0, 0);

inputs[0].ki.wVk = VK_CONTROL;
     inputs[0].ki.dwFlags = 0;
@@ -1541,10 +1538,8 @@ static void test_hotkey_navigation(void)
     inputs[3].ki.dwFlags = KEYEVENTF_KEYUP;
     SendInput(4, inputs, sizeof(INPUT));
     WaitForSingleObject(active_page_changed, 1000);
-    todo_wine
     ok(active_page == 1, "Got %ld\n", active_page);
-    PostMessageW(hotkey_dialog, WM_DIALOG_IDLE, 0, 0);
-    WaitForSingleObject(hotkey_dialog_idle, INFINITE);
+    SendMessageA(hotkey_dialog, WM_NULL, 0, 0);

inputs[0].ki.wVk = VK_CONTROL;
     inputs[0].ki.dwFlags = 0;
@@ -1561,8 +1556,7 @@ static void test_hotkey_navigation(void)
     SendInput(6, inputs, sizeof(INPUT));
     WaitForSingleObject(active_page_changed, 1000);
     ok(active_page == 0, "Got %ld\n", active_page);
-    PostMessageW(hotkey_dialog, WM_DIALOG_IDLE, 0, 0);
-    WaitForSingleObject(hotkey_dialog_idle, INFINITE);
+    SendMessageA(hotkey_dialog, WM_NULL, 0, 0);

PostMessageA(hotkey_dialog, WM_CLOSE, 0, 0);
     WaitForSingleObject(thread, INFINITE);

[Fabian Maurer (＠DarkShadow44)]

On Sat Jun 29 02:26:59 2024 +0000, Jinoh Kang wrote:
> > SendMessage should be handles immediately, without going through the
>
> message queue like that.
> Not if the window belongs to another thread. Pay special attention to
> the 4<sup>th</sup> paragraph of https://learn.microsoft.com/en-us/windows/win32/api/winuser/nf-winuser-sendm...:
> > If the specified window was created by the calling thread, the window
>
> procedure is called immediately as a subroutine. **If the specified
> window was created by a different thread, the system switches to that
> thread and calls the appropriate window procedure.** Messages sent
> between threads are processed only when the receiving thread executes
> message retrieval code.
> [emphasis mine]
> > I wonder why it doesn't work.
>
> Because sent messages (QS_SENDMESSAGE) take priority over hardware
> (QS_INPUT) messages. See server/queue.c, get_messages handler. Posted
> messages (QS_POSTMESSAGE) have the same problem.
> That is, we still need the WaitForSingleObject. My original proposal was
> to use SendMessage *in addition to* WaitForSingleObject, not replace it.

I see, so many tiny details.. So the only advantage SendMessage has over PostMessage is that we don't need the manual wait afterwards, right?

I pushed an update, is this good then?

[Zhiyi Zhang (＠zhiyi)]

> I pushed an update, is this good then?

Jinoh is right, sent messages have a higher priority than hardware messages so let's use the WM_DIALOG_IDLE message.