[PATCH v2 0/5] MR932: cocmtl32: propsheet tests
-- v2: comtl32: Don't pass PROPSHEETPAGE to SetWindowSubclass in PROPSHEET_CreatePage. comctl32/tests: Test passing PropertySheetPage as HPROPSHEETPAGE. comctl32/tests: Add more PROPSHEETPAGE tests inside callback. comctl32/tests: Test passing extra data in PROPSHEETPAGE structure. comctl32/tests: Test PROPERTYSHEETPAGE passed in WM_INITDIALOG. https://gitlab.winehq.org/wine/wine/-/merge_requests/932
From: Piotr Caban <piotr(a)codeweavers.com> --- dlls/comctl32/tests/propsheet.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/dlls/comctl32/tests/propsheet.c b/dlls/comctl32/tests/propsheet.c index 8b2c282c72e..2132158bd93 100644 --- a/dlls/comctl32/tests/propsheet.c +++ b/dlls/comctl32/tests/propsheet.c @@ -112,10 +112,18 @@ static INT_PTR CALLBACK page_dlg_proc(HWND hwnd, UINT msg, WPARAM wparam, { case WM_INITDIALOG: { + PROPSHEETPAGEA *psp = (PROPSHEETPAGEA *)lparam; HWND sheet = GetParent(hwnd); char caption[256]; + GetWindowTextA(sheet, caption, sizeof(caption)); ok(!strcmp(caption,"test caption"), "caption: %s\n", caption); + + if (psp->dwFlags & PSP_USETITLE) + { + todo_wine ok(!strcmp(psp->pszTitle, "page title"), "psp->pszTitle = %s\n", + wine_dbgstr_a(psp->pszTitle)); + } return TRUE; } @@ -149,8 +157,9 @@ static void test_title(void) memset(&psp, 0, sizeof(psp)); psp.dwSize = sizeof(psp); - psp.dwFlags = 0; + psp.dwFlags = PSP_USETITLE; psp.hInstance = GetModuleHandleA(NULL); + psp.pszTitle = "page title"; U(psp).pszTemplate = "prop_page1"; U2(psp).pszIcon = NULL; psp.pfnDlgProc = page_dlg_proc; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/932
From: Piotr Caban <piotr(a)codeweavers.com> --- dlls/comctl32/tests/propsheet.c | 23 +++++++++++++++++++++-- 1 file changed, 21 insertions(+), 2 deletions(-) diff --git a/dlls/comctl32/tests/propsheet.c b/dlls/comctl32/tests/propsheet.c index 2132158bd93..eee525a19d3 100644 --- a/dlls/comctl32/tests/propsheet.c +++ b/dlls/comctl32/tests/propsheet.c @@ -1035,6 +1035,7 @@ struct custom_proppage PROPSHEETPAGEA pageA; PROPSHEETPAGEW pageW; } u; + DWORD extra_data; unsigned int addref_called; unsigned int release_called; }; @@ -1050,6 +1051,12 @@ static UINT CALLBACK proppage_callback_a(HWND hwnd, UINT msg, PROPSHEETPAGEA *ps psp->lParam, psp); ok(psp_orig->pszTitle == psp->pszTitle, "Expected same page title pointer\n"); ok(!lstrcmpA(psp_orig->pszTitle, psp->pszTitle), "Expected same page title string\n"); + if (psp->dwSize >= FIELD_OFFSET(struct custom_proppage, addref_called)) + { + struct custom_proppage *extra_data = (struct custom_proppage *)psp; + todo_wine ok(extra_data->extra_data == 0x1234, "Expected extra_data to be preserved, got %lx\n", + extra_data->extra_data); + } switch (msg) { @@ -1078,6 +1085,12 @@ static UINT CALLBACK proppage_callback_w(HWND hwnd, UINT msg, PROPSHEETPAGEW *ps psp->lParam, psp); ok(psp_orig->pszTitle == psp->pszTitle, "Expected same page title pointer\n"); ok(!lstrcmpW(psp_orig->pszTitle, psp->pszTitle), "Expected same page title string\n"); + if (psp->dwSize >= FIELD_OFFSET(struct custom_proppage, addref_called)) + { + struct custom_proppage *extra_data = (struct custom_proppage *)psp; + todo_wine ok(extra_data->extra_data == 0x4321, "Expected extra_data to be preserved, got %lx\n", + extra_data->extra_data); + } switch (msg) { @@ -1110,8 +1123,11 @@ static void test_CreatePropertySheetPage(void) page.u.pageA.pszTitle = "Title"; /* Only minimal size validation is performed */ - for (page.u.pageA.dwSize = PROPSHEETPAGEA_V1_SIZE - 1; page.u.pageA.dwSize <= PROPSHEETPAGEA_V4_SIZE + 1; page.u.pageA.dwSize++) + for (page.u.pageA.dwSize = PROPSHEETPAGEA_V1_SIZE - 1; + page.u.pageA.dwSize <= FIELD_OFFSET(struct custom_proppage, addref_called); + page.u.pageA.dwSize++) { + page.extra_data = 0x1234; page.addref_called = 0; hpsp = pCreatePropertySheetPageA(&page.u.pageA); @@ -1139,8 +1155,11 @@ static void test_CreatePropertySheetPage(void) page.u.pageW.lParam = (LPARAM)&page; page.u.pageW.pszTitle = L"Title"; - for (page.u.pageW.dwSize = PROPSHEETPAGEW_V1_SIZE - 1; page.u.pageW.dwSize <= PROPSHEETPAGEW_V4_SIZE + 1; page.u.pageW.dwSize++) + for (page.u.pageW.dwSize = PROPSHEETPAGEW_V1_SIZE - 1; + page.u.pageW.dwSize <= FIELD_OFFSET(struct custom_proppage, addref_called); + page.u.pageW.dwSize++) { + page.extra_data = 0x4321; page.addref_called = 0; hpsp = pCreatePropertySheetPageW(&page.u.pageW); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/932
From: Piotr Caban <piotr(a)codeweavers.com> --- dlls/comctl32/tests/propsheet.c | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/dlls/comctl32/tests/propsheet.c b/dlls/comctl32/tests/propsheet.c index eee525a19d3..ca99e11df36 100644 --- a/dlls/comctl32/tests/propsheet.c +++ b/dlls/comctl32/tests/propsheet.c @@ -1049,7 +1049,10 @@ static UINT CALLBACK proppage_callback_a(HWND hwnd, UINT msg, PROPSHEETPAGEA *ps ok(psp->lParam && psp->lParam != (LPARAM)psp, "Expected newly allocated page description, got %Ix, %p\n", psp->lParam, psp); - ok(psp_orig->pszTitle == psp->pszTitle, "Expected same page title pointer\n"); + if (psp->dwFlags & PSP_USETITLE) + todo_wine ok(psp_orig->pszTitle != psp->pszTitle, "Expected different page title pointer\n"); + else + ok(psp_orig->pszTitle == psp->pszTitle, "Expected same page title pointer\n"); ok(!lstrcmpA(psp_orig->pszTitle, psp->pszTitle), "Expected same page title string\n"); if (psp->dwSize >= FIELD_OFFSET(struct custom_proppage, addref_called)) { @@ -1148,6 +1151,17 @@ static void test_CreatePropertySheetPage(void) } } + page.u.pageA.dwSize = sizeof(PROPSHEETPAGEA); + page.u.pageA.dwFlags |= PSP_USETITLE; + page.addref_called = 0; + hpsp = pCreatePropertySheetPageA(&page.u.pageA); + ok(hpsp != NULL, "Failed to create a page, size %lu\n", page.u.pageA.dwSize); + ok(page.addref_called == 1, "Expected ADDREF callback message\n"); + page.release_called = 0; + ret = pDestroyPropertySheetPage(hpsp); + ok(ret, "Failed to destroy a page\n"); + ok(page.release_called == 1, "Expected RELEASE callback message\n"); + memset(&page.u.pageW, 0, sizeof(page.u.pageW)); page.u.pageW.dwFlags = PSP_USECALLBACK; page.u.pageW.pfnDlgProc = page_dlg_proc_messages; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/932
From: Piotr Caban <piotr(a)codeweavers.com> --- dlls/comctl32/tests/propsheet.c | 85 +++++++++++++++++++++++++++++++++ 1 file changed, 85 insertions(+) diff --git a/dlls/comctl32/tests/propsheet.c b/dlls/comctl32/tests/propsheet.c index ca99e11df36..84fd5e51e28 100644 --- a/dlls/comctl32/tests/propsheet.c +++ b/dlls/comctl32/tests/propsheet.c @@ -42,6 +42,7 @@ static HPROPSHEETPAGE (WINAPI *pCreatePropertySheetPageA)(const PROPSHEETPAGEA * static HPROPSHEETPAGE (WINAPI *pCreatePropertySheetPageW)(const PROPSHEETPAGEW *desc); static BOOL (WINAPI *pDestroyPropertySheetPage)(HPROPSHEETPAGE proppage); static INT_PTR (WINAPI *pPropertySheetA)(const PROPSHEETHEADERA *header); +static INT_PTR (WINAPI *pPropertySheetW)(const PROPSHEETHEADERW *header); static BOOL (WINAPI *pIsThemeActive)(void); static BOOL (WINAPI *pIsThemeDialogTextureEnabled)(HWND); @@ -1301,6 +1302,88 @@ static void test_page_dialog_texture(void) DestroyWindow(hdlg); } +static void test_invalid_hpropsheetpage(void) +{ + BYTE buf_psp[sizeof(ULONG_PTR) + sizeof(PROPSHEETPAGEA)]; + BYTE buf_pspW[sizeof(ULONG_PTR) + sizeof(PROPSHEETPAGEW)]; + HPROPSHEETPAGE hpsp[1]; + PROPSHEETHEADERW pshW; + PROPSHEETHEADERA psh; + PROPSHEETPAGEA *psp; + PROPSHEETPAGEW *pspW; + LRESULT ret; + HWND hdlg; + + /* LocalSize throws exception on misaligned pointers (x86_64). */ + psp = (PROPSHEETPAGEA*)((ULONG_PTR)buf_psp & 0xf ? buf_psp : buf_psp + sizeof(ULONG_PTR)); + pspW = (PROPSHEETPAGEW*)((ULONG_PTR)buf_pspW & 0xf ? buf_pspW : buf_pspW + sizeof(ULONG_PTR)); + + memset(psp, 0, sizeof(*psp)); + psp->dwSize = sizeof(*psp); + psp->hInstance = GetModuleHandleA(NULL); + U(psp)->pszTemplate = "prop_page1"; + U2(psp)->pszIcon = NULL; + psp->pfnDlgProc = page_dlg_proc; + psp->lParam = 0; + + hpsp[0] = (HPROPSHEETPAGE)psp; + + memset(&psh, 0, sizeof(psh)); + psh.dwSize = PROPSHEETHEADERA_V1_SIZE; + psh.dwFlags = PSH_MODELESS | PSH_USECALLBACK; + psh.pszCaption = "test caption"; + psh.nPages = 1; + psh.hwndParent = GetDesktopWindow(); + U3(psh).phpage = hpsp; + psh.pfnCallback = sheet_callback; + + hdlg = (HWND)pPropertySheetA(&psh); + ok(hdlg != INVALID_HANDLE_VALUE, "got invalid handle value %p\n", hdlg); + + ret = SendMessageA(hdlg, PSM_INDEXTOPAGE, 0, 0); + ok(ret, "page was not created\n"); + todo_wine ok((HPROPSHEETPAGE)ret != hpsp[0], "invalid HPROPSHEETPAGE was preserved\n"); + DestroyWindow(hdlg); + + memset(pspW, 0, sizeof(*pspW)); + pspW->dwSize = sizeof(*pspW); + pspW->hInstance = GetModuleHandleA(NULL); + U(pspW)->pszTemplate = L"prop_page1"; + U2(pspW)->pszIcon = NULL; + pspW->pfnDlgProc = page_dlg_proc; + pspW->lParam = 0; + + hpsp[0] = (HPROPSHEETPAGE)pspW; + + hdlg = (HWND)pPropertySheetA(&psh); + ok(hdlg != INVALID_HANDLE_VALUE, "got invalid handle value %p\n", hdlg); + + ok(!SendMessageA(hdlg, PSM_INDEXTOPAGE, 0, 0), "there should be no pages\n"); + ok(!SendMessageW(hdlg, PSM_ADDPAGE, 0, (LPARAM)pspW), "PSM_ADDPAGE succeeded\n"); + ok(SendMessageW(hdlg, PSM_ADDPAGE, 0, (LPARAM)psp), "PSM_ADDPAGE failed\n"); + ok(SendMessageA(hdlg, PSM_INDEXTOPAGE, 0, 0), "no pages after PSM_ADDPAGE\n"); + DestroyWindow(hdlg); + + hpsp[0] = (HPROPSHEETPAGE)pspW; + + memset(&pshW, 0, sizeof(pshW)); + pshW.dwSize = PROPSHEETHEADERA_V1_SIZE; + pshW.dwFlags = PSH_MODELESS | PSH_USECALLBACK; + pshW.pszCaption = L"test caption"; + pshW.nPages = 1; + pshW.hwndParent = GetDesktopWindow(); + U3(pshW).phpage = hpsp; + pshW.pfnCallback = sheet_callback; + + hdlg = (HWND)pPropertySheetW(&pshW); + ok(hdlg != INVALID_HANDLE_VALUE, "got invalid handle value %p\n", hdlg); + + ret = SendMessageA(hdlg, PSM_INDEXTOPAGE, 0, 0); + todo_wine ok(ret, "page was not created\n"); + ok((HPROPSHEETPAGE)ret != hpsp[0], "invalid HPROPSHEETPAGE was preserved\n"); + DestroyWindow(hdlg); +} + static void init_comctl32_functions(void) { HMODULE hComCtl32 = LoadLibraryA("comctl32.dll"); @@ -1310,6 +1393,7 @@ static void init_comctl32_functions(void) X(CreatePropertySheetPageW); X(DestroyPropertySheetPage); X(PropertySheetA); + X(PropertySheetW); #undef X } @@ -1353,6 +1437,7 @@ START_TEST(propsheet) test_PSM_INSERTPAGE(); test_CreatePropertySheetPage(); test_page_dialog_texture(); + test_invalid_hpropsheetpage(); if (!load_v6_module(&ctx_cookie, &ctx)) return; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/932
From: Piotr Caban <piotr(a)codeweavers.com> --- dlls/comctl32/propsheet.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/dlls/comctl32/propsheet.c b/dlls/comctl32/propsheet.c index a512ca9f361..884a0638abb 100644 --- a/dlls/comctl32/propsheet.c +++ b/dlls/comctl32/propsheet.c @@ -1461,8 +1461,7 @@ static BOOL PROPSHEET_CreatePage(HWND hwndParent, (psInfo->ppshheader.dwFlags & PSH_WATERMARK) && (ppshpage->dwFlags & PSP_HIDEHEADER)) { - SetWindowSubclass(hwndPage, PROPSHEET_WizardSubclassProc, 1, - (DWORD_PTR)ppshpage); + SetWindowSubclass(hwndPage, PROPSHEET_WizardSubclassProc, 1, 0); } if (!(psInfo->ppshheader.dwFlags & INTRNL_ANY_WIZARD)) EnableThemeDialogTexture (hwndPage, ETDT_ENABLETAB); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/932
Zhiyi Zhang (@zhiyi) commented about dlls/comctl32/tests/propsheet.c:
}
+static void test_invalid_hpropsheetpage(void) +{ + BYTE buf_psp[sizeof(ULONG_PTR) + sizeof(PROPSHEETPAGEA)]; + BYTE buf_pspW[sizeof(ULONG_PTR) + sizeof(PROPSHEETPAGEW)]; + HPROPSHEETPAGE hpsp[1]; + PROPSHEETHEADERW pshW; + PROPSHEETHEADERA psh; + PROPSHEETPAGEA *psp; + PROPSHEETPAGEW *pspW; + LRESULT ret; + HWND hdlg; + + /* LocalSize throws exception on misaligned pointers (x86_64). */ + psp = (PROPSHEETPAGEA*)((ULONG_PTR)buf_psp & 0xf ? buf_psp : buf_psp + sizeof(ULONG_PTR)); Please add a space before *.
And could you add some comments documenting what invalid configuration are you testing? It's not obvious to me, to be honest. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/932#note_9502
Zhiyi Zhang (@zhiyi) commented about dlls/comctl32/propsheet.c:
(psInfo->ppshheader.dwFlags & PSH_WATERMARK) && (ppshpage->dwFlags & PSP_HIDEHEADER))
Typo in the commit message. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/932#note_9503
On Wed Sep 28 07:51:41 2022 +0000, Zhiyi Zhang wrote:
Please add a space before *. And could you add some comments documenting what invalid configuration are you testing? It's not obvious to me, to be honest. The tests are passing PROPSHEETPAGE[AW] instead of HPROPSHEETPAGE. I'll add a comment.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/932#note_9506
On Wed Sep 28 07:53:08 2022 +0000, Zhiyi Zhang wrote:
Typo in the commit message. I don't see anything wrong with the commit message. What do you have in mind? I didn't use PROPSHEETPAGEW intentionally because the structure contains broken data. It's used as a hybrid of PROPSHEETPAGEA and PROPSHEETPAGEW (I'm planning to change that in next MRs).
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/932#note_9507
On Wed Sep 28 08:14:05 2022 +0000, Piotr Caban wrote:
I don't see anything wrong with the commit message. What do you have in mind? I didn't use PROPSHEETPAGEW intentionally because the structure contains broken data. It's used as a hybrid of PROPSHEETPAGEA and PROPSHEETPAGEW (I'm planning to change that in next MRs). comtl32 -> comctl32
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/932#note_9509
participants (3)
-
Piotr Caban -
Piotr Caban (＠piotr)
-
Zhiyi Zhang (＠zhiyi)