[PATCH v4 0/4] MR8220: Draft: comdlg32: Don't use a shadow devmode structure in PrintDlg.

[Jacob Czekalla (＠JacobCzekalla)]

If the "Properties" button is clicked in the printdlg it opens the print properties dlg. The problem is the values set in the properties dlg don't get passed through correctly to the final printing stage. This prevents properties like duplex and page size from working.

--
  v4: comdlg32: Don't use a shadow devmode structure in PrintDlgA.
      comdlg32/tests: Add tests for changing devmode properties in the hook procedure for PrintDlgA.
      comdlg32: Don't use a shadow devmode structure in PrintDlgW.
      comdlg32/tests: Add tests for changing devmode properties in the hook procedure for PrintDlgW.
 
https://gitlab.winehq.org/wine/wine/-/merge_requests/8220

[PATCH v4 1/4] comdlg32/tests: Add tests for changing devmode properties in the hook procedure for PrintDlgW.

[Jacob Czekalla]

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

---
 dlls/comdlg32/tests/printdlg.c | 65 ++++++++++++++++++++++++++++++++++
 1 file changed, 65 insertions(+)

diff --git a/dlls/comdlg32/tests/printdlg.c b/dlls/comdlg32/tests/printdlg.c
index 7fd4567ba9e..5c1f4ac9939 100644
--- a/dlls/comdlg32/tests/printdlg.c
+++ b/dlls/comdlg32/tests/printdlg.c
@@ -271,6 +271,70 @@ static void test_PrintDlgA(void)
     free(pDlg);
 }
 
+static UINT_PTR CALLBACK printer_properties_hook_procW(HWND hdlg, UINT msg, WPARAM wp, LPARAM lp)
+{
+    DEVMODEW* dm;
+    PRINTDLGW* dlg;
+
+    if (msg == WM_INITDIALOG)
+    {
+        dlg = (PRINTDLGW*)lp;
+        dm = GlobalLock(dlg->hDevMode);
+        todo_wine
+        ok(dm->dmDuplex != 123, "dmDuplex should not equal 123 in the hook.\n");
+        todo_wine
+        ok(dm->dmPaperSize != 321, "dmPaperSize should not equal 321 in the hook.\n");
+        GlobalUnlock(dlg->hDevMode);
+        PostMessageW(hdlg, WM_COMMAND, psh2, lp);
+    }
+    if (msg == WM_COMMAND && wp == psh2)
+    {
+        dlg = (PRINTDLGW*)lp;
+        dm = GlobalLock(dlg->hDevMode);
+        dm->dmDuplex = 999;
+        dm->dmPaperSize = 888;
+        GlobalUnlock(dlg->hDevMode);
+        PostMessageW(hdlg, WM_COMMAND, IDOK, FALSE);
+        return TRUE;
+    }
+    return 0;
+}
+
+void test_PrintDlgW(void)
+{
+    PRINTDLGW pd = { 0 };
+    DEVMODEW* dm;
+    DWORD name_size = 0;
+
+    GetDefaultPrinterW(NULL, &name_size);
+    if(name_size == 0)
+    {
+        skip("No printer configured.\n");
+        return;
+    }
+
+    pd.lStructSize = sizeof(pd);
+    pd.Flags = PD_ENABLEPRINTHOOK;
+    pd.lpfnPrintHook = printer_properties_hook_procW;
+
+    pd.hDevMode = GlobalAlloc(GMEM_MOVEABLE | GMEM_ZEROINIT, sizeof(DEVMODEW));
+    dm = GlobalLock(pd.hDevMode);
+    dm->dmSize = sizeof(*dm);
+    dm->dmFields |= DM_DUPLEX | DM_PAPERSIZE;
+    dm->dmDuplex = 123;
+    dm->dmPaperSize = 321;
+    GlobalUnlock(pd.hDevMode);
+
+    PrintDlgW(&pd);
+    dm = GlobalLock(pd.hDevMode);
+    todo_wine
+    ok(dm->dmDuplex == 999, "expected 999, but got %d.\n", dm->dmDuplex);
+    todo_wine
+    ok(dm->dmPaperSize == 888, "expected 888, but got %d.\n", dm->dmPaperSize);
+    GlobalUnlock(pd.hDevMode);
+    GlobalFree(pd.hDevMode);
+}
+
 /* ########################### */
 
 static HRESULT WINAPI callback_QueryInterface(IPrintDialogCallback *iface,
@@ -641,6 +705,7 @@ START_TEST(printdlg)
 
     test_PageSetupDlgA();
     test_PrintDlgA();
+    test_PrintDlgW();
     test_PrintDlgExW();
     test_abort_proc();
 }
-- 
GitLab


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

[PATCH v4 2/4] comdlg32: Don't use a shadow devmode structure in PrintDlgW.

[Jacob Czekalla]

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

Changes to hDevMode made in the hook procedure should persist.
---
 dlls/comdlg32/printdlg.c       | 71 +++++++++++++---------------------
 dlls/comdlg32/tests/printdlg.c |  4 --
 2 files changed, 27 insertions(+), 48 deletions(-)

diff --git a/dlls/comdlg32/printdlg.c b/dlls/comdlg32/printdlg.c
index b5c67c3abad..a56498be05c 100644
--- a/dlls/comdlg32/printdlg.c
+++ b/dlls/comdlg32/printdlg.c
@@ -70,7 +70,6 @@ typedef struct
 
 typedef struct
 {
-  LPDEVMODEW        lpDevMode;
   LPPRINTDLGW       lpPrintDlg;
   LPPRINTER_INFO_2W lpPrinterInfo;
   LPDRIVER_INFO_3W  lpDriverInfo;
@@ -660,7 +659,7 @@ static BOOL PRINTDLG_UpdatePrintDlgW(HWND hDlg,
 				    PRINT_PTRW* PrintStructures)
 {
     LPPRINTDLGW       lppd = PrintStructures->lpPrintDlg;
-    PDEVMODEW         lpdm = PrintStructures->lpDevMode;
+    PDEVMODEW         lpdm = GlobalLock(lppd->hDevMode);
     LPPRINTER_INFO_2W pi = PrintStructures->lpPrinterInfo;
 
 
@@ -699,6 +698,7 @@ static BOOL PRINTDLG_UpdatePrintDlgW(HWND hDlg,
 			    resourcestr, 255);
 		MessageBoxW(hDlg, resultstr, resourcestr,
 			    MB_OK | MB_ICONWARNING);
+                GlobalUnlock(lppd->hDevMode);
 		return FALSE;
 	    }
 	    lppd->nFromPage = nFromPage;
@@ -745,6 +745,7 @@ static BOOL PRINTDLG_UpdatePrintDlgW(HWND hDlg,
             lppd->nCopies = GetDlgItemInt(hDlg, edt3, NULL, FALSE);
 	}
     }
+    GlobalUnlock(lppd->hDevMode);
     return TRUE;
 }
 
@@ -1288,7 +1289,7 @@ static BOOL PRINTDLG_ChangePrinterW(HWND hDlg, WCHAR *name,
 				   PRINT_PTRW *PrintStructures)
 {
     LPPRINTDLGW lppd = PrintStructures->lpPrintDlg;
-    LPDEVMODEW lpdm = NULL;
+    LPDEVMODEW lpdm, dm_tmp;
     LONG dmSize;
     DWORD needed;
     HANDLE hprn;
@@ -1314,28 +1315,27 @@ static BOOL PRINTDLG_ChangePrinterW(HWND hDlg, WCHAR *name,
 
     PRINTDLG_UpdatePrinterInfoTextsW(hDlg, lppd->Flags, PrintStructures->lpPrinterInfo);
 
-    free(PrintStructures->lpDevMode);
-    PrintStructures->lpDevMode = NULL;
-
     dmSize = DocumentPropertiesW(0, 0, name, NULL, NULL, 0);
     if(dmSize == -1) {
         ERR("DocumentProperties fails on %s\n", debugstr_w(name));
 	return FALSE;
     }
-    PrintStructures->lpDevMode = malloc(dmSize);
-    dmSize = DocumentPropertiesW(0, 0, name, PrintStructures->lpDevMode, NULL,
+    dm_tmp = malloc(dmSize);
+    dmSize = DocumentPropertiesW(0, 0, name, dm_tmp, NULL,
 				 DM_OUT_BUFFER);
-    if(lppd->hDevMode && (lpdm = GlobalLock(lppd->hDevMode)) &&
-			  !lstrcmpW(lpdm->dmDeviceName,
-				  PrintStructures->lpDevMode->dmDeviceName)) {
-      /* Supplied devicemode matches current printer so try to use it */
-        DocumentPropertiesW(0, 0, name, PrintStructures->lpDevMode, lpdm,
-			    DM_OUT_BUFFER | DM_IN_BUFFER);
-    }
-    if(lpdm)
+    if(lppd->hDevMode)
+    {
+        lpdm = GlobalLock(lppd->hDevMode);
+        if(!lstrcmpW(lpdm->dmDeviceName, dm_tmp->dmDeviceName))
+            /* Supplied devicemode matches current printer so try to use it */
+            DocumentPropertiesW(0, 0, name, dm_tmp, lpdm, DM_OUT_BUFFER | DM_IN_BUFFER);
         GlobalUnlock(lppd->hDevMode);
-
-    lpdm = PrintStructures->lpDevMode;  /* use this as a shortcut */
+        lppd->hDevMode = GlobalReAlloc(lppd->hDevMode, dm_tmp->dmSize + dm_tmp->dmDriverExtra, GMEM_MOVEABLE);
+    }else
+        lppd->hDevMode = GlobalAlloc(GMEM_MOVEABLE, dm_tmp->dmSize + dm_tmp->dmDriverExtra);
+    lpdm = GlobalLock(lppd->hDevMode);
+    memcpy(lpdm, dm_tmp, dm_tmp->dmSize + dm_tmp->dmDriverExtra);
+    free(dm_tmp);
 
     if(!(lppd->Flags & PD_PRINTSETUP)) {
 	/* Print range (All/Range/Selection) */
@@ -1488,6 +1488,7 @@ static BOOL PRINTDLG_ChangePrinterW(HWND hDlg, WCHAR *name,
         /* hide if PD_SHOWHELP not specified */
         ShowWindow(GetDlgItem(hDlg, pshHelp), SW_HIDE);
     }
+    GlobalUnlock(lppd->hDevMode);
     return TRUE;
 }
 
@@ -1900,21 +1901,24 @@ static LRESULT PRINTDLG_WMCommandW(HWND hDlg, WPARAM wParam,
 {
     LPPRINTDLGW lppd = PrintStructures->lpPrintDlg;
     UINT PrinterComboID = (lppd->Flags & PD_PRINTSETUP) ? cmb1 : cmb4;
-    LPDEVMODEW lpdm = PrintStructures->lpDevMode;
+    LPDEVMODEW lpdm = GlobalLock(lppd->hDevMode);
 
     switch (LOWORD(wParam))  {
     case IDOK:
         TRACE(" OK button was hit\n");
         if (!PRINTDLG_UpdatePrintDlgW(hDlg, PrintStructures)) {
 	    FIXME("Update printdlg was not successful!\n");
-	    return(FALSE);
+	    GlobalUnlock(lppd->hDevMode);
+            return(FALSE);
 	}
 	EndDialog(hDlg, TRUE);
+        GlobalUnlock(lppd->hDevMode);
 	return(TRUE);
 
     case IDCANCEL:
         TRACE(" CANCEL button was hit\n");
         EndDialog(hDlg, FALSE);
+        GlobalUnlock(lppd->hDevMode);
 	return(FALSE);
 
      case pshHelp:
@@ -1965,8 +1969,8 @@ static LRESULT PRINTDLG_WMCommandW(HWND hDlg, WPARAM wParam,
 	     break;
 	 }
 	 DocumentPropertiesW(hDlg, hPrinter, PrinterName,
-			     PrintStructures->lpDevMode,
-			     PrintStructures->lpDevMode,
+			     lpdm,
+			     lpdm,
 			     DM_IN_BUFFER | DM_OUT_BUFFER | DM_IN_PROMPT);
 	 ClosePrinter(hPrinter);
          break;
@@ -2050,6 +2054,7 @@ static LRESULT PRINTDLG_WMCommandW(HWND hDlg, WPARAM wParam,
 	    break;
 	}
     }
+    GlobalUnlock(lppd->hDevMode);
     return FALSE;
 }
 
@@ -2548,30 +2553,9 @@ BOOL WINAPI PrintDlgW(LPPRINTDLGW lppd)
 					   (LPARAM)PrintStructures));
 
 	if(bRet) {
-	    DEVMODEW *lpdm = PrintStructures->lpDevMode, *lpdmReturn;
 	    PRINTER_INFO_2W *pi = PrintStructures->lpPrinterInfo;
 	    DRIVER_INFO_3W *di = PrintStructures->lpDriverInfo;
 
-	    if (lppd->hDevMode == 0) {
-	        TRACE(" No hDevMode yet... Need to create my own\n");
-		lppd->hDevMode = GlobalAlloc(GMEM_MOVEABLE,
-					lpdm->dmSize + lpdm->dmDriverExtra);
-	    } else {
-	        WORD locks;
-		if((locks = (GlobalFlags(lppd->hDevMode) & GMEM_LOCKCOUNT))) {
-		    WARN("hDevMode has %d locks on it. Unlocking it now\n", locks);
-		    while(locks--) {
-		        GlobalUnlock(lppd->hDevMode);
-			TRACE("Now got %d locks\n", locks);
-		    }
-		}
-		lppd->hDevMode = GlobalReAlloc(lppd->hDevMode,
-					       lpdm->dmSize + lpdm->dmDriverExtra,
-					       GMEM_MOVEABLE);
-	    }
-	    lpdmReturn = GlobalLock(lppd->hDevMode);
-	    memcpy(lpdmReturn, lpdm, lpdm->dmSize + lpdm->dmDriverExtra);
-
 	    if (lppd->hDevNames != 0) {
 	        WORD locks;
 		if((locks = (GlobalFlags(lppd->hDevNames) & GMEM_LOCKCOUNT))) {
@@ -2587,7 +2571,6 @@ BOOL WINAPI PrintDlgW(LPPRINTDLGW lppd)
 	    );
 	    GlobalUnlock(lppd->hDevMode);
 	}
-	free(PrintStructures->lpDevMode);
 	free(PrintStructures->lpPrinterInfo);
 	free(PrintStructures->lpDriverInfo);
 	free(PrintStructures);
diff --git a/dlls/comdlg32/tests/printdlg.c b/dlls/comdlg32/tests/printdlg.c
index 5c1f4ac9939..0e8e35e4bfc 100644
--- a/dlls/comdlg32/tests/printdlg.c
+++ b/dlls/comdlg32/tests/printdlg.c
@@ -280,9 +280,7 @@ static UINT_PTR CALLBACK printer_properties_hook_procW(HWND hdlg, UINT msg, WPAR
     {
         dlg = (PRINTDLGW*)lp;
         dm = GlobalLock(dlg->hDevMode);
-        todo_wine
         ok(dm->dmDuplex != 123, "dmDuplex should not equal 123 in the hook.\n");
-        todo_wine
         ok(dm->dmPaperSize != 321, "dmPaperSize should not equal 321 in the hook.\n");
         GlobalUnlock(dlg->hDevMode);
         PostMessageW(hdlg, WM_COMMAND, psh2, lp);
@@ -327,9 +325,7 @@ void test_PrintDlgW(void)
 
     PrintDlgW(&pd);
     dm = GlobalLock(pd.hDevMode);
-    todo_wine
     ok(dm->dmDuplex == 999, "expected 999, but got %d.\n", dm->dmDuplex);
-    todo_wine
     ok(dm->dmPaperSize == 888, "expected 888, but got %d.\n", dm->dmPaperSize);
     GlobalUnlock(pd.hDevMode);
     GlobalFree(pd.hDevMode);
-- 
GitLab


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

[PATCH v4 3/4] comdlg32/tests: Add tests for changing devmode properties in the hook procedure for PrintDlgA.

[Jacob Czekalla]

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

---
 dlls/comdlg32/tests/printdlg.c | 52 ++++++++++++++++++++++++++++++++++
 1 file changed, 52 insertions(+)

diff --git a/dlls/comdlg32/tests/printdlg.c b/dlls/comdlg32/tests/printdlg.c
index 0e8e35e4bfc..690028e1ca7 100644
--- a/dlls/comdlg32/tests/printdlg.c
+++ b/dlls/comdlg32/tests/printdlg.c
@@ -125,6 +125,35 @@ static UINT_PTR CALLBACK print_hook_proc(HWND hdlg, UINT msg, WPARAM wp, LPARAM
     return 0;
 }
 
+static UINT_PTR CALLBACK printer_properties_hook_procA(HWND hdlg, UINT msg, WPARAM wp, LPARAM lp)
+{
+    DEVMODEA* dm;
+    PRINTDLGA* dlg;
+
+    if (msg == WM_INITDIALOG)
+    {
+        dlg = (PRINTDLGA*)lp;
+        dm = GlobalLock(dlg->hDevMode);
+        todo_wine
+        ok(dm->dmDuplex != 123, "dmDuplex should not equal 123 in the hook.\n");
+        todo_wine
+        ok(dm->dmPaperSize != 321, "dmPaperSize should not equal 321 in the hook.\n");
+        GlobalUnlock(dlg->hDevMode);
+        PostMessageW(hdlg, WM_COMMAND, psh2, lp);
+    }
+    if (msg == WM_COMMAND && wp == psh2)
+    {
+        dlg = (PRINTDLGA*)lp;
+        dm = GlobalLock(dlg->hDevMode);
+        dm->dmDuplex = 999;
+        dm->dmPaperSize = 888;
+        GlobalUnlock(dlg->hDevMode);
+        PostMessageW(hdlg, WM_COMMAND, IDOK, FALSE);
+        return TRUE;
+    }
+    return 0;
+}
+
 static void test_PrintDlgA(void)
 {
     DWORD res, n_copies = 0;
@@ -268,6 +297,29 @@ static void test_PrintDlgA(void)
         GlobalFree(pDlg->hDevNames);
     }
 
+    ZeroMemory(pDlg, sizeof(*pDlg));
+
+    pDlg->lStructSize = sizeof(*pDlg);
+    pDlg->Flags = PD_ENABLEPRINTHOOK;
+    pDlg->lpfnPrintHook = printer_properties_hook_procA;
+
+    pDlg->hDevMode = GlobalAlloc(GMEM_MOVEABLE | GMEM_ZEROINIT, sizeof(DEVMODEA));
+    dm = GlobalLock(pDlg->hDevMode);
+    dm->dmSize = sizeof(*dm);
+    dm->dmFields |= DM_DUPLEX | DM_PAPERSIZE;
+    dm->dmDuplex = 123;
+    dm->dmPaperSize = 321;
+    GlobalUnlock(pDlg->hDevMode);
+
+    PrintDlgA(pDlg);
+    dm = GlobalLock(pDlg->hDevMode);
+    todo_wine
+    ok(dm->dmDuplex == 999, "expected 999, but got %d.\n", dm->dmDuplex);
+    todo_wine
+    ok(dm->dmPaperSize == 888, "expected 888, but got %d.\n", dm->dmPaperSize);
+    GlobalUnlock(pDlg->hDevMode);
+    GlobalFree(pDlg->hDevMode);
+
     free(pDlg);
 }
 
-- 
GitLab


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

[PATCH v4 4/4] comdlg32: Don't use a shadow devmode structure in PrintDlgA.

[Jacob Czekalla]

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

Changes to hDevMode made in the hook procedure should persist.
---
 dlls/comdlg32/printdlg.c       | 61 +++++++++++++++-------------------
 dlls/comdlg32/tests/printdlg.c |  4 ---
 2 files changed, 26 insertions(+), 39 deletions(-)

diff --git a/dlls/comdlg32/printdlg.c b/dlls/comdlg32/printdlg.c
index a56498be05c..84079c6a004 100644
--- a/dlls/comdlg32/printdlg.c
+++ b/dlls/comdlg32/printdlg.c
@@ -56,7 +56,6 @@ WINE_DEFAULT_DEBUG_CHANNEL(commdlg);
 
 typedef struct
 {
-  LPDEVMODEA        lpDevMode;
   LPPRINTDLGA       lpPrintDlg;
   LPPRINTER_INFO_2A lpPrinterInfo;
   LPDRIVER_INFO_3A  lpDriverInfo;
@@ -556,7 +555,7 @@ static BOOL PRINTDLG_UpdatePrintDlgA(HWND hDlg,
 				    PRINT_PTRA* PrintStructures)
 {
     LPPRINTDLGA       lppd = PrintStructures->lpPrintDlg;
-    PDEVMODEA         lpdm = PrintStructures->lpDevMode;
+    PDEVMODEA         lpdm = GlobalLock(lppd->hDevMode);
     LPPRINTER_INFO_2A pi = PrintStructures->lpPrinterInfo;
 
 
@@ -588,6 +587,7 @@ static BOOL PRINTDLG_UpdatePrintDlgA(HWND hDlg,
 		wsprintfW(resultstr,resourcestr, lppd->nMinPage, lppd->nMaxPage);
 		LoadStringW(COMDLG32_hInstance, PD32_PRINT_TITLE, resourcestr, 255);
 		MessageBoxW(hDlg, resultstr, resourcestr, MB_OK | MB_ICONWARNING);
+                GlobalUnlock(lppd->hDevMode);
 		return FALSE;
 	    }
 	    lppd->nFromPage = nFromPage;
@@ -652,6 +652,7 @@ static BOOL PRINTDLG_UpdatePrintDlgA(HWND hDlg,
 	    }
 	}
     }
+    GlobalUnlock(lppd->hDevMode);
     return TRUE;
 }
 
@@ -1082,7 +1083,7 @@ static void PRINTDLG_UpdatePrinterInfoTextsW(HWND hDlg, DWORD flags, const PRINT
 static BOOL PRINTDLG_ChangePrinterA(HWND hDlg, char *name, PRINT_PTRA *PrintStructures)
 {
     LPPRINTDLGA lppd = PrintStructures->lpPrintDlg;
-    LPDEVMODEA lpdm = NULL;
+    LPDEVMODEA lpdm, dm_tmp;
     LONG dmSize;
     DWORD needed;
     HANDLE hprn;
@@ -1108,28 +1109,27 @@ static BOOL PRINTDLG_ChangePrinterA(HWND hDlg, char *name, PRINT_PTRA *PrintStru
 
     PRINTDLG_UpdatePrinterInfoTextsA(hDlg, lppd->Flags, PrintStructures->lpPrinterInfo);
 
-    free(PrintStructures->lpDevMode);
-    PrintStructures->lpDevMode = NULL;
-
     dmSize = DocumentPropertiesA(0, 0, name, NULL, NULL, 0);
     if(dmSize == -1) {
         ERR("DocumentProperties fails on %s\n", debugstr_a(name));
 	return FALSE;
     }
-    PrintStructures->lpDevMode = malloc(dmSize);
-    dmSize = DocumentPropertiesA(0, 0, name, PrintStructures->lpDevMode, NULL,
+    dm_tmp = malloc(dmSize);
+    dmSize = DocumentPropertiesA(0, 0, name, dm_tmp, NULL,
 				 DM_OUT_BUFFER);
-    if(lppd->hDevMode && (lpdm = GlobalLock(lppd->hDevMode)) &&
-			  !lstrcmpA( (LPSTR) lpdm->dmDeviceName,
-				     (LPSTR) PrintStructures->lpDevMode->dmDeviceName)) {
-      /* Supplied devicemode matches current printer so try to use it */
-        DocumentPropertiesA(0, 0, name, PrintStructures->lpDevMode, lpdm,
-			    DM_OUT_BUFFER | DM_IN_BUFFER);
-    }
-    if(lpdm)
+    if(lppd->hDevMode)
+    {
+        lpdm = GlobalLock(lppd->hDevMode);
+        if(!lstrcmpA((LPSTR)lpdm->dmDeviceName, (LPSTR)dm_tmp->dmDeviceName))
+            /* Supplied devicemode matches current printer so try to use it */
+            DocumentPropertiesA(0, 0, name, dm_tmp, lpdm, DM_OUT_BUFFER | DM_IN_BUFFER);
         GlobalUnlock(lppd->hDevMode);
-
-    lpdm = PrintStructures->lpDevMode;  /* use this as a shortcut */
+        lppd->hDevMode = GlobalReAlloc(lppd->hDevMode, dm_tmp->dmSize + dm_tmp->dmDriverExtra, GMEM_MOVEABLE);
+    }else
+        lppd->hDevMode = GlobalAlloc(GMEM_MOVEABLE, dm_tmp->dmSize + dm_tmp->dmDriverExtra);
+    lpdm = GlobalLock(lppd->hDevMode);
+    memcpy(lpdm, dm_tmp, dm_tmp->dmSize + dm_tmp->dmDriverExtra);
+    free(dm_tmp);
 
     if(!(lppd->Flags & PD_PRINTSETUP)) {
 	/* Print range (All/Range/Selection) */
@@ -1282,6 +1282,7 @@ static BOOL PRINTDLG_ChangePrinterA(HWND hDlg, char *name, PRINT_PTRA *PrintStru
         /* hide if PD_SHOWHELP not specified */
         ShowWindow(GetDlgItem(hDlg, pshHelp), SW_HIDE);
     }
+    GlobalUnlock(lppd->hDevMode);
     return TRUE;
 }
 
@@ -1744,21 +1745,24 @@ static LRESULT PRINTDLG_WMCommandA(HWND hDlg, WPARAM wParam,
 {
     LPPRINTDLGA lppd = PrintStructures->lpPrintDlg;
     UINT PrinterComboID = (lppd->Flags & PD_PRINTSETUP) ? cmb1 : cmb4;
-    LPDEVMODEA lpdm = PrintStructures->lpDevMode;
+    LPDEVMODEA lpdm = GlobalLock(lppd->hDevMode);
 
     switch (LOWORD(wParam))  {
     case IDOK:
         TRACE(" OK button was hit\n");
         if (!PRINTDLG_UpdatePrintDlgA(hDlg, PrintStructures)) {
 	    FIXME("Update printdlg was not successful!\n");
+            GlobalUnlock(lppd->hDevMode);
 	    return(FALSE);
 	}
 	EndDialog(hDlg, TRUE);
+        GlobalUnlock(lppd->hDevMode);
 	return(TRUE);
 
     case IDCANCEL:
         TRACE(" CANCEL button was hit\n");
         EndDialog(hDlg, FALSE);
+        GlobalUnlock(lppd->hDevMode);
 	return(FALSE);
 
      case pshHelp:
@@ -1809,8 +1813,8 @@ static LRESULT PRINTDLG_WMCommandA(HWND hDlg, WPARAM wParam,
 	     break;
 	 }
 	 DocumentPropertiesA(hDlg, hPrinter, PrinterName,
-			     PrintStructures->lpDevMode,
-			     PrintStructures->lpDevMode,
+			     lpdm,
+			     lpdm,
 			     DM_IN_BUFFER | DM_OUT_BUFFER | DM_IN_PROMPT);
 	 ClosePrinter(hPrinter);
          break;
@@ -1893,6 +1897,7 @@ static LRESULT PRINTDLG_WMCommandA(HWND hDlg, WPARAM wParam,
 	    break;
 	}
     }
+    GlobalUnlock(lppd->hDevMode);
     return FALSE;
 }
 
@@ -2403,22 +2408,9 @@ BOOL WINAPI PrintDlgA(LPPRINTDLGA lppd)
 					   (LPARAM)PrintStructures));
 
 	if(bRet) {
-	    DEVMODEA *lpdm = PrintStructures->lpDevMode, *lpdmReturn;
 	    PRINTER_INFO_2A *pi = PrintStructures->lpPrinterInfo;
 	    DRIVER_INFO_3A *di = PrintStructures->lpDriverInfo;
 
-	    if (lppd->hDevMode == 0) {
-	        TRACE(" No hDevMode yet... Need to create my own\n");
-		lppd->hDevMode = GlobalAlloc(GMEM_MOVEABLE,
-					lpdm->dmSize + lpdm->dmDriverExtra);
-	    } else {
-		lppd->hDevMode = GlobalReAlloc(lppd->hDevMode,
-					       lpdm->dmSize + lpdm->dmDriverExtra,
-					       GMEM_MOVEABLE);
-	    }
-	    lpdmReturn = GlobalLock(lppd->hDevMode);
-	    memcpy(lpdmReturn, lpdm, lpdm->dmSize + lpdm->dmDriverExtra);
-
 	    PRINTDLG_CreateDevNames(&(lppd->hDevNames),
 		    di->pDriverPath,
 		    pi->pPrinterName,
@@ -2426,7 +2418,6 @@ BOOL WINAPI PrintDlgA(LPPRINTDLGA lppd)
 	    );
 	    GlobalUnlock(lppd->hDevMode);
 	}
-	free(PrintStructures->lpDevMode);
 	free(PrintStructures->lpPrinterInfo);
 	free(PrintStructures->lpDriverInfo);
 	free(PrintStructures);
diff --git a/dlls/comdlg32/tests/printdlg.c b/dlls/comdlg32/tests/printdlg.c
index 690028e1ca7..e30971aba75 100644
--- a/dlls/comdlg32/tests/printdlg.c
+++ b/dlls/comdlg32/tests/printdlg.c
@@ -134,9 +134,7 @@ static UINT_PTR CALLBACK printer_properties_hook_procA(HWND hdlg, UINT msg, WPAR
     {
         dlg = (PRINTDLGA*)lp;
         dm = GlobalLock(dlg->hDevMode);
-        todo_wine
         ok(dm->dmDuplex != 123, "dmDuplex should not equal 123 in the hook.\n");
-        todo_wine
         ok(dm->dmPaperSize != 321, "dmPaperSize should not equal 321 in the hook.\n");
         GlobalUnlock(dlg->hDevMode);
         PostMessageW(hdlg, WM_COMMAND, psh2, lp);
@@ -313,9 +311,7 @@ static void test_PrintDlgA(void)
 
     PrintDlgA(pDlg);
     dm = GlobalLock(pDlg->hDevMode);
-    todo_wine
     ok(dm->dmDuplex == 999, "expected 999, but got %d.\n", dm->dmDuplex);
-    todo_wine
     ok(dm->dmPaperSize == 888, "expected 888, but got %d.\n", dm->dmPaperSize);
     GlobalUnlock(pDlg->hDevMode);
     GlobalFree(pDlg->hDevMode);
-- 
GitLab

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

[PATCH v4 0/4] MR8220: comdlg32: Don't use a shadow devmode structure in PrintDlg. - approved

[Huw Davies (＠huw)]

This merge request was approved by Huw Davies.
-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/8220