[PATCH v4 0/2] MR6145: comctl32: Implement PSN_QUERYINITIALFOCUS

[Jacob Czekalla (＠JacobCzekalla)]

Adding PSN_QUERYINITIALFOCUS helped fix some focus issues with the property sheet in bug 54862. Previously the listview in the tab control did not get focus from the start when it should have.

--
  v4: comctl32/tests: Add test for PSN_QUERYINITIALFOCUS for the propsheet.
      comctl32: Add handling for PSN_QUERYINITIALFOCUS in prop.c.
 
https://gitlab.winehq.org/wine/wine/-/merge_requests/6145

[PATCH v4 1/2] comctl32: Add handling for PSN_QUERYINITIALFOCUS in prop.c.

[Jacob Czekalla]

From: Jacob Czekalla <jczekalla(a)codeweavers.com>

---
 dlls/comctl32/propsheet.c | 33 +++++++++++++++++++++++++++------
 1 file changed, 27 insertions(+), 6 deletions(-)

diff --git a/dlls/comctl32/propsheet.c b/dlls/comctl32/propsheet.c
index b530a1a8094..6bd4b34a4d3 100644
--- a/dlls/comctl32/propsheet.c
+++ b/dlls/comctl32/propsheet.c
@@ -37,7 +37,6 @@
  *     o WM_CONTEXTMENU
  *   - Notifications:
  *     o PSN_GETOBJECT
- *     o PSN_QUERYINITIALFOCUS
  *     o PSN_TRANSLATEACCELERATOR
  *   - Styles:
  *     o PSH_RTLREADING
@@ -582,6 +581,30 @@ static void HPSP_draw_text(HPROPSHEETPAGE hpsp, HDC hdc, BOOL title, RECT *r, UI
 }
 
 #define add_flag(a) if (dwFlags & a) {strcat(string, #a );strcat(string," ");}
+
+static void notify_QueryInitialFocus(HWND hwndDlg, int index, PropSheetInfo* psInfo)
+{
+  PSHNOTIFY psn;
+  HWND focusable_item = NULL;
+  HWND initial_focus = NULL;
+
+  focusable_item = GetNextDlgTabItem(psInfo->proppage[index].hwndPage, NULL, FALSE);
+
+  if (!focusable_item)
+      return;
+
+  psn.hdr.code = PSN_QUERYINITIALFOCUS;
+  psn.hdr.hwndFrom = hwndDlg;
+  psn.hdr.idFrom = 0;
+  psn.lParam = 0;
+  initial_focus = (HWND) SendMessageW(psInfo->proppage[index].hwndPage, WM_NOTIFY, 0, (LPARAM) &psn);
+
+  if (initial_focus)
+      SetFocus(initial_focus);
+  else if (focusable_item)
+      SetFocus(focusable_item);
+}
+
 /******************************************************************************
  *            PROPSHEET_UnImplementedFlags
  *
@@ -1655,7 +1678,6 @@ static BOOL PROPSHEET_ShowPage(HWND hwndDlg, int index, PropSheetInfo * psInfo)
 {
   HWND hwndTabCtrl;
   HWND hwndLineHeader;
-  HWND control;
 
   TRACE("active_page %d, index %d\n", psInfo->active_page, index);
   if (index == psInfo->active_page)
@@ -1674,10 +1696,6 @@ static BOOL PROPSHEET_ShowPage(HWND hwndDlg, int index, PropSheetInfo * psInfo)
   {
      PROPSHEET_SetTitleW(hwndDlg, psInfo->ppshheader.dwFlags,
                          psInfo->proppage[index].pszText);
-
-     control = GetNextDlgTabItem(psInfo->proppage[index].hwndPage, NULL, FALSE);
-     if(control != NULL)
-         SetFocus(control);
   }
 
   if (psInfo->active_page != -1)
@@ -1693,6 +1711,8 @@ static BOOL PROPSHEET_ShowPage(HWND hwndDlg, int index, PropSheetInfo * psInfo)
   psInfo->active_page = index;
   psInfo->activeValid = TRUE;
 
+  notify_QueryInitialFocus(hwndDlg, index, psInfo);
+
   if (psInfo->ppshheader.dwFlags & (PSH_WIZARD97_OLD | PSH_WIZARD97_NEW) )
   {
       hwndLineHeader = GetDlgItem(hwndDlg, IDC_SUNKEN_LINEHEADER);
@@ -1891,6 +1911,7 @@ static BOOL PROPSHEET_Apply(HWND hwndDlg, LPARAM lParam)
      psn.lParam   = 0;
      hwndPage = psInfo->proppage[psInfo->active_page].hwndPage;
      SendMessageW(hwndPage, WM_NOTIFY, 0, (LPARAM) &psn);
+     notify_QueryInitialFocus(hwndPage, psInfo->active_page, psInfo);
   }
 
   return TRUE;
-- 
GitLab


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

[PATCH v4 2/2] comctl32/tests: Add test for PSN_QUERYINITIALFOCUS for the propsheet.

[Jacob Czekalla]

From: Jacob Czekalla <jczekalla(a)codeweavers.com>

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

diff --git a/dlls/comctl32/tests/propsheet.c b/dlls/comctl32/tests/propsheet.c
index 9bd1c08bd6a..262c6f9fc0d 100644
--- a/dlls/comctl32/tests/propsheet.c
+++ b/dlls/comctl32/tests/propsheet.c
@@ -1388,6 +1388,110 @@ static void test_invalid_hpropsheetpage(void)
     DestroyWindow(hdlg);
 }
 
+int query_initial_focus = 0;
+LRESULT CALLBACK TestDlgProc(HWND hdlg, UINT uMessage, WPARAM wParam, LPARAM lParam)
+{
+    LPNMHDR lpnmhdr;
+switch (uMessage)
+   {
+   case WM_NOTIFY:
+        lpnmhdr = (NMHDR *)lParam;
+        switch (lpnmhdr->code)
+        {
+            case PSN_QUERYINITIALFOCUS:
+            {
+                query_initial_focus = 1;
+                break;
+            }
+            default:
+                break;
+        }
+    break;
+
+   default:
+      break;
+   }
+
+   return FALSE;
+
+}
+
+static void test_QueryInitialFocus(void)
+{
+    PROPSHEETPAGEA psp[2];
+    PROPSHEETHEADERA psh;
+    HWND tab_ctrl = NULL;
+    HWND hp;
+    RECT rc;
+    LPARAM lp;
+
+    memset(&psh, 0, sizeof(psh));
+    memset(psp, 0, sizeof(psp[0]) * 2);
+    psp[0].dwSize = sizeof(PROPSHEETPAGEA);
+    psp[0].dwFlags = PSP_USETITLE;
+    psp[0].hInstance = GetModuleHandleA(NULL);;
+    psp[0].pszTemplate = (const char*) MAKEINTRESOURCE(IDD_PROP_PAGE_EDIT);
+    psp[0].pszIcon = NULL;
+    psp[0].pfnDlgProc = (DLGPROC) TestDlgProc;
+    psp[0].pszTitle = "Page1";
+    psp[0].lParam = 0;
+
+    psp[1].dwSize = sizeof(PROPSHEETPAGEA);
+    psp[1].dwFlags = PSP_USETITLE;
+    psp[1].hInstance = GetModuleHandleA(NULL);
+    psp[1].pszTemplate = (const char*) MAKEINTRESOURCE(IDD_PROP_PAGE_EDIT);
+    psp[1].pszIcon = NULL;
+    psp[1].pfnDlgProc = (DLGPROC) TestDlgProc;
+    psp[1].pszTitle = "Page2";
+    psp[1].lParam = 0;
+
+    psh.dwSize = sizeof(PROPSHEETHEADERA);
+    psh.dwFlags = PSH_PROPSHEETPAGE | PSH_MODELESS;
+    psh.hwndParent = GetDesktopWindow();
+    psh.pszCaption = "Modeless Property Sheet";
+    psh.nPages = sizeof(psp) / sizeof(PROPSHEETPAGEA);
+    psh.ppsp = (LPCPROPSHEETPAGEA)&psp;
+
+    hp = (HWND) pPropertySheetA(&psh);
+    tab_ctrl = (HWND)SendMessageA(hp, PSM_GETTABCONTROL, 0, 0);
+
+    //Test PSN_QUERYINITIALFOCUS gets sent on start
+    ok(query_initial_focus, "Did not recieve PSN_QUERYINITIALFOCUS on start.\n");
+    query_initial_focus = 0;
+
+    //Test changing of tabs
+    ok(tab_ctrl != 0, "Could not test changing tabs. No tab control found.\n");
+    if(tab_ctrl)
+    {
+        SendMessageA(tab_ctrl, TCM_GETITEMRECT, 1, (LPARAM) &rc);
+        lp = MAKELPARAM(rc.left + ((rc.right-rc.left)/2), rc.top + ((rc.bottom-rc.top)/2));
+        SendMessageA(tab_ctrl, WM_LBUTTONDOWN, MK_LBUTTON, lp);
+        ok(query_initial_focus, "Did not recieve PSN_QUERYINITIALFOCUS from changing tabs.\n");
+        query_initial_focus = 0;
+    }
+
+    //Test Apply Button
+    SendMessageA(hp, PSM_PRESSBUTTON, PSBTN_APPLYNOW, 0);
+    ok(query_initial_focus, "Did not recieve PSN_QUERYINITIALFOCUS from apply button.\n");
+    query_initial_focus = 0;
+
+    DestroyWindow(hp);
+
+    psh.dwFlags = PSH_PROPSHEETPAGE | PSH_USECALLBACK | PSH_WIZARD | PSH_MODELESS;
+    hp = (HWND) pPropertySheetA(&psh);
+
+    //Test PSN_QUERYINITIALFOCUS gets sent on start
+    ok(query_initial_focus, "Did not recieve PSN_QUERYINITIALFOCUS on start.\n");
+    query_initial_focus = 0;
+
+    //Test Next Button
+    SendMessageA(hp, PSM_PRESSBUTTON, PSBTN_NEXT, 0);
+    ok(query_initial_focus, "Did not recieve PSN_QUERYINITIALFOCUS from next button.\n");
+    query_initial_focus = 0;
+
+    DestroyWindow(hp);
+}
+
 static void init_comctl32_functions(void)
 {
     HMODULE hComCtl32 = LoadLibraryA("comctl32.dll");
@@ -1442,6 +1546,7 @@ START_TEST(propsheet)
     test_CreatePropertySheetPage();
     test_page_dialog_texture();
     test_invalid_hpropsheetpage();
+    test_QueryInitialFocus();
 
     if (!load_v6_module(&ctx_cookie, &ctx))
         return;
-- 
GitLab

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

[PATCH v4 2/2] comctl32/tests: Add test for PSN_QUERYINITIALFOCUS for the propsheet.

[Marvin]

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=147353

Your paranoid android.


=== debian11b (64 bit WoW report) ===

Report validation errors:
dxgi:dxgi has unaccounted for todo messages
dxgi:dxgi has unaccounted for skip messages
The report seems to have been truncated

[Zhiyi Zhang (＠zhiyi)]

Zhiyi Zhang (@zhiyi) commented about dlls/comctl32/propsheet.c:
> }
>
> #define add_flag(a) if (dwFlags & a) {strcat(string, #a );strcat(string," ");}
> +
> +static void notify_QueryInitialFocus(HWND hwndDlg, int index, PropSheetInfo* psInfo)
> Please rename the function to PROPSHEET_QueryInitialFocus() so that it's consistent with other functions in the file.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/6145#note_77105

[Zhiyi Zhang (＠zhiyi)]

Zhiyi Zhang (@zhiyi) commented about dlls/comctl32/tests/propsheet.c:
> +            case PSN_QUERYINITIALFOCUS:
> +            {
> +                query_initial_focus = 1;
> +                break;
> +            }
> +            default:
> +                break;
> +        }
> +    break;
> +
> +   default:
> +      break;
> +   }
> +
> +   return FALSE;
> +
> Delete this empty line.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/6145#note_77108

[Zhiyi Zhang (＠zhiyi)]

Zhiyi Zhang (@zhiyi) commented about dlls/comctl32/tests/propsheet.c:
> DestroyWindow(hdlg);
>  }
>
> +int query_initial_focus = 0;
> +LRESULT CALLBACK TestDlgProc(HWND hdlg, UINT uMessage, WPARAM wParam, LPARAM lParam)
> +{
> +    LPNMHDR lpnmhdr;

Please add a new line after variable declaration.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/6145#note_77106

[Zhiyi Zhang (＠zhiyi)]

Zhiyi Zhang (@zhiyi) commented about dlls/comctl32/tests/propsheet.c:
> DestroyWindow(hdlg);
>  }
>
> +int query_initial_focus = 0;
> +LRESULT CALLBACK TestDlgProc(HWND hdlg, UINT uMessage, WPARAM wParam, LPARAM lParam)
> +{
> +    LPNMHDR lpnmhdr;
> +switch (uMessage)

Add four spaces before switch

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/6145#note_77107

[Zhiyi Zhang (＠zhiyi)]

Zhiyi Zhang (@zhiyi) commented about dlls/comctl32/tests/propsheet.c:
> +    psp[1].pszIcon = NULL;
> +    psp[1].pfnDlgProc = (DLGPROC) TestDlgProc;
> +    psp[1].pszTitle = "Page2";
> +    psp[1].lParam = 0;
> +
> +    psh.dwSize = sizeof(PROPSHEETHEADERA);
> +    psh.dwFlags = PSH_PROPSHEETPAGE | PSH_MODELESS;
> +    psh.hwndParent = GetDesktopWindow();
> +    psh.pszCaption = "Modeless Property Sheet";
> +    psh.nPages = sizeof(psp) / sizeof(PROPSHEETPAGEA);
> +    psh.ppsp = (LPCPROPSHEETPAGEA)&psp;
> +
> +    hp = (HWND) pPropertySheetA(&psh);
> +    tab_ctrl = (HWND)SendMessageA(hp, PSM_GETTABCONTROL, 0, 0);
> +
> +    //Test PSN_QUERYINITIALFOCUS gets sent on start
> Please use /**/ instead of //

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/6145#note_77109

[Zhiyi Zhang (＠zhiyi)]

Zhiyi Zhang (@zhiyi) commented about dlls/comctl32/tests/propsheet.c:
> DestroyWindow(hdlg);
>  }
>
> +int query_initial_focus = 0;
> +LRESULT CALLBACK TestDlgProc(HWND hdlg, UINT uMessage, WPARAM wParam, LPARAM lParam)
> +{
> +    LPNMHDR lpnmhdr;
> +switch (uMessage)
> +   {
> +   case WM_NOTIFY:
> +        lpnmhdr = (NMHDR *)lParam;
> +        switch (lpnmhdr->code)
> +        {
> +            case PSN_QUERYINITIALFOCUS:
> +            {
> +                query_initial_focus = 1;

So handling PSN_QUERYINITIALFOCUS only sets query_initial_focus to 1. What about the message result? Please test that PSN_QUERYINITIALFOCUS works besides getting sent.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/6145#note_77110

[Zhiyi Zhang (＠zhiyi)]

Zhiyi Zhang (@zhiyi) commented about dlls/comctl32/tests/propsheet.c:
> +
> +   return FALSE;
> +
> +}
> +
> +static void test_QueryInitialFocus(void)
> +{
> +    PROPSHEETPAGEA psp[2];
> +    PROPSHEETHEADERA psh;
> +    HWND tab_ctrl = NULL;
> +    HWND hp;
> +    RECT rc;
> +    LPARAM lp;
> +
> +    memset(&psh, 0, sizeof(psh));
> +    memset(psp, 0, sizeof(psp[0]) * 2);
> You can juse use memset(psp, 0, sizeof(psp));

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/6145#note_77111

[Zhiyi Zhang (＠zhiyi)]

Zhiyi Zhang (@zhiyi) commented about dlls/comctl32/tests/propsheet.c:
> +    psh.dwFlags = PSH_PROPSHEETPAGE | PSH_MODELESS;
> +    psh.hwndParent = GetDesktopWindow();
> +    psh.pszCaption = "Modeless Property Sheet";
> +    psh.nPages = sizeof(psp) / sizeof(PROPSHEETPAGEA);
> +    psh.ppsp = (LPCPROPSHEETPAGEA)&psp;
> +
> +    hp = (HWND) pPropertySheetA(&psh);
> +    tab_ctrl = (HWND)SendMessageA(hp, PSM_GETTABCONTROL, 0, 0);
> +
> +    //Test PSN_QUERYINITIALFOCUS gets sent on start
> +    ok(query_initial_focus, "Did not recieve PSN_QUERYINITIALFOCUS on start.\n");
> +    query_initial_focus = 0;
> +
> +    //Test changing of tabs
> +    ok(tab_ctrl != 0, "Could not test changing tabs. No tab control found.\n");
> +    if(tab_ctrl)
> You can remove this if(tab_ctrl) check. ok(tab_ctrl != 0, ...) already checks for this.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/6145#note_77112

[Zhiyi Zhang (＠zhiyi)]

Zhiyi Zhang (@zhiyi) commented about dlls/comctl32/tests/propsheet.c:
> +    psh.nPages = sizeof(psp) / sizeof(PROPSHEETPAGEA);
> +    psh.ppsp = (LPCPROPSHEETPAGEA)&psp;
> +
> +    hp = (HWND) pPropertySheetA(&psh);
> +    tab_ctrl = (HWND)SendMessageA(hp, PSM_GETTABCONTROL, 0, 0);
> +
> +    //Test PSN_QUERYINITIALFOCUS gets sent on start
> +    ok(query_initial_focus, "Did not recieve PSN_QUERYINITIALFOCUS on start.\n");
> +    query_initial_focus = 0;
> +
> +    //Test changing of tabs
> +    ok(tab_ctrl != 0, "Could not test changing tabs. No tab control found.\n");
> +    if(tab_ctrl)
> +    {
> +        SendMessageA(tab_ctrl, TCM_GETITEMRECT, 1, (LPARAM) &rc);
> +        lp = MAKELPARAM(rc.left + ((rc.right-rc.left)/2), rc.top + ((rc.bottom-rc.top)/2));
> Add spaces before and after '-' and '/'

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/6145#note_77113

[Zhiyi Zhang (＠zhiyi)]

Zhiyi Zhang (@zhiyi) commented about dlls/comctl32/tests/propsheet.c:
> test_CreatePropertySheetPage();
>      test_page_dialog_texture();
>      test_invalid_hpropsheetpage();
> +    test_QueryInitialFocus();

Please add the tests in a commit first, and use todo_wine to mark the tests failing on Wine. Then remove those todo_wines after you implement PSN_QUERYINITIALFOCUS.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/6145#note_77114

[Zhiyi Zhang (＠zhiyi)]

Please add Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=54862 at the end of the commit message body.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/6145#note_77115