[PATCH v3 0/2] MR6146: comctl32: Create all prop sheet pages on initialization.
Fixes bug 54861. The UK Kalender program was crashing when a new event was created as stated in the bug report. The fix is to create all the property sheet pages from the beginning instead of when they are selected. -- v3: comctl32: Create all prop sheet pages on initialization. comctl32/tests: Add test for propsheet page creation when propsheet gets initialized. https://gitlab.winehq.org/wine/wine/-/merge_requests/6146
From: Jacob Czekalla <jczekalla(a)codeweavers.com> --- dlls/comctl32/tests/propsheet.c | 60 +++++++++++++++++++++++++++++++++ 1 file changed, 60 insertions(+) diff --git a/dlls/comctl32/tests/propsheet.c b/dlls/comctl32/tests/propsheet.c index 9bd1c08bd6a..e4011231ecc 100644 --- a/dlls/comctl32/tests/propsheet.c +++ b/dlls/comctl32/tests/propsheet.c @@ -1388,6 +1388,65 @@ static void test_invalid_hpropsheetpage(void) DestroyWindow(hdlg); } +static void test_init_page_creation(void) +{ + PROPSHEETPAGEA psp[3]; + PROPSHEETHEADERA psh; + HWND hp; + HWND hwnd_page; + + memset(&psh, 0, sizeof(psh)); + memset(psp, 0, sizeof(psp)); + 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) page_dlg_proc; + psp[0].pszTitle = "page title"; + 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) page_dlg_proc; + psp[1].pszTitle = "page title"; + psp[1].lParam = 0; + + psp[2].dwSize = sizeof(PROPSHEETPAGEA); + psp[2].dwFlags = PSP_USETITLE | PSP_PREMATURE; + psp[2].hInstance = GetModuleHandleA(NULL); + psp[2].pszTemplate = (const char*)MAKEINTRESOURCE(IDD_PROP_PAGE_EDIT); + psp[2].pszIcon = NULL; + psp[2].pfnDlgProc = (DLGPROC) page_dlg_proc; + psp[2].pszTitle = "page title"; + psp[2].lParam = 0; + + psh.dwSize = sizeof(PROPSHEETHEADERA); + psh.dwFlags = PSH_PROPSHEETPAGE | PSH_MODELESS; + psh.hwndParent = GetDesktopWindow(); + psh.pszCaption = "test caption"; + psh.nPages = sizeof(psp) / sizeof(PROPSHEETPAGEA); + psh.ppsp = (LPCPROPSHEETPAGEA)&psp; + + hp = (HWND)pPropertySheetA(&psh); + + hwnd_page = (HWND)SendMessageA(hp, PSM_INDEXTOHWND, 0, 0); + todo_wine ok(hwnd_page != NULL, "Page 0 not created.\n"); + + hwnd_page = (HWND)SendMessageA(hp, PSM_INDEXTOHWND, 1, 0); + todo_wine ok(hwnd_page == NULL, "Page 1 not created.\n"); + + hwnd_page = (HWND)SendMessageA(hp, PSM_INDEXTOHWND, 2, 0); + todo_wine ok(hwnd_page != NULL, "Page 2 not created.\n"); + + DestroyWindow(hp); + + return; +} + static void init_comctl32_functions(void) { HMODULE hComCtl32 = LoadLibraryA("comctl32.dll"); @@ -1442,6 +1501,7 @@ START_TEST(propsheet) test_CreatePropertySheetPage(); test_page_dialog_texture(); test_invalid_hpropsheetpage(); + test_init_page_creation(); if (!load_v6_module(&ctx_cookie, &ctx)) return; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6146
From: Jacob Czekalla <jczekalla(a)codeweavers.com> --- dlls/comctl32/propsheet.c | 11 +++++++++++ dlls/comctl32/tests/propsheet.c | 6 +++--- 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/dlls/comctl32/propsheet.c b/dlls/comctl32/propsheet.c index b530a1a8094..4f0c4170816 100644 --- a/dlls/comctl32/propsheet.c +++ b/dlls/comctl32/propsheet.c @@ -3625,6 +3625,17 @@ PROPSHEET_DialogProc(HWND hwnd, UINT uMsg, WPARAM wParam, LPARAM lParam) psInfo->ppshheader.pszCaption); } + for (int index = 0; index < psInfo->nPages;) + { + if (HPSP_get_flags(psInfo->proppage[index].hpage) & PSP_PREMATURE) + { + if (!PROPSHEET_CreatePage(hwnd, index, psInfo, psInfo->proppage[index].hpage)) + PROPSHEET_RemovePage(hwnd, index, NULL); + else + index++; + }else + index++; + } if (psInfo->useCallback) (*(psInfo->ppshheader.pfnCallback))(hwnd, PSCB_INITIALIZED, 0); diff --git a/dlls/comctl32/tests/propsheet.c b/dlls/comctl32/tests/propsheet.c index e4011231ecc..0e0ef8fabee 100644 --- a/dlls/comctl32/tests/propsheet.c +++ b/dlls/comctl32/tests/propsheet.c @@ -1434,13 +1434,13 @@ static void test_init_page_creation(void) hp = (HWND)pPropertySheetA(&psh); hwnd_page = (HWND)SendMessageA(hp, PSM_INDEXTOHWND, 0, 0); - todo_wine ok(hwnd_page != NULL, "Page 0 not created.\n"); + ok(hwnd_page != NULL, "Page 0 not created.\n"); hwnd_page = (HWND)SendMessageA(hp, PSM_INDEXTOHWND, 1, 0); - todo_wine ok(hwnd_page == NULL, "Page 1 not created.\n"); + ok(hwnd_page == NULL, "Page 1 not created.\n"); hwnd_page = (HWND)SendMessageA(hp, PSM_INDEXTOHWND, 2, 0); - todo_wine ok(hwnd_page != NULL, "Page 2 not created.\n"); + ok(hwnd_page != NULL, "Page 2 not created.\n"); DestroyWindow(hp); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6146
Zhiyi Zhang (@zhiyi) commented about dlls/comctl32/tests/propsheet.c:
+ 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) page_dlg_proc; + psp[1].pszTitle = "page title"; + psp[1].lParam = 0; + + psp[2].dwSize = sizeof(PROPSHEETPAGEA); + psp[2].dwFlags = PSP_USETITLE | PSP_PREMATURE; + psp[2].hInstance = GetModuleHandleA(NULL); + psp[2].pszTemplate = (const char*)MAKEINTRESOURCE(IDD_PROP_PAGE_EDIT); + psp[2].pszIcon = NULL; + psp[2].pfnDlgProc = (DLGPROC) page_dlg_proc; + psp[2].pszTitle = "page title"; + psp[2].lParam = 0; Let's use a loop for this.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/6146#note_77412
Zhiyi Zhang (@zhiyi) commented about dlls/comctl32/tests/propsheet.c:
+ psh.dwFlags = PSH_PROPSHEETPAGE | PSH_MODELESS; + psh.hwndParent = GetDesktopWindow(); + psh.pszCaption = "test caption"; + psh.nPages = sizeof(psp) / sizeof(PROPSHEETPAGEA); + psh.ppsp = (LPCPROPSHEETPAGEA)&psp; + + hp = (HWND)pPropertySheetA(&psh); + + hwnd_page = (HWND)SendMessageA(hp, PSM_INDEXTOHWND, 0, 0); + todo_wine ok(hwnd_page != NULL, "Page 0 not created.\n"); + + hwnd_page = (HWND)SendMessageA(hp, PSM_INDEXTOHWND, 1, 0); + todo_wine ok(hwnd_page == NULL, "Page 1 not created.\n"); + + hwnd_page = (HWND)SendMessageA(hp, PSM_INDEXTOHWND, 2, 0); + todo_wine ok(hwnd_page != NULL, "Page 2 not created.\n"); Same here.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/6146#note_77413
Zhiyi Zhang (@zhiyi) commented about dlls/comctl32/tests/propsheet.c:
+ psh.ppsp = (LPCPROPSHEETPAGEA)&psp; + + hp = (HWND)pPropertySheetA(&psh); + + hwnd_page = (HWND)SendMessageA(hp, PSM_INDEXTOHWND, 0, 0); + todo_wine ok(hwnd_page != NULL, "Page 0 not created.\n"); + + hwnd_page = (HWND)SendMessageA(hp, PSM_INDEXTOHWND, 1, 0); + todo_wine ok(hwnd_page == NULL, "Page 1 not created.\n"); + + hwnd_page = (HWND)SendMessageA(hp, PSM_INDEXTOHWND, 2, 0); + todo_wine ok(hwnd_page != NULL, "Page 2 not created.\n"); + + DestroyWindow(hp); + + return; You don't need this return here.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/6146#note_77414
Zhiyi Zhang (@zhiyi) commented about dlls/comctl32/propsheet.c:
psInfo->ppshheader.pszCaption); }
+ for (int index = 0; index < psInfo->nPages;) + { + if (HPSP_get_flags(psInfo->proppage[index].hpage) & PSP_PREMATURE)
It's cleaner if you write it like ``` DWORD premature = HPSP_get_flags(psInfo->proppage[index].hpage) & PSP_PREMATURE; if (premature && !PROPSHEET_CreatePage(hwnd, index, psInfo, psInfo->proppage[index].hpage)) PROPSHEET_RemovePage(hwnd, index, NULL); else index++; ``` -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6146#note_77415
Zhiyi Zhang (@zhiyi) commented about dlls/comctl32/tests/propsheet.c:
hp = (HWND)pPropertySheetA(&psh);
The commit message can be improved. It can be something like "comctl32/propsheet: Create pages with PSP_PREMATURE on initialization." -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6146#note_77416
participants (3)
-
Jacob Czekalla -
Jacob Czekalla (＠JacobCzekalla)
-
Zhiyi Zhang (＠zhiyi)