[PATCH] Implement OleCreatePropertyFrame (try 2)

Piotr Caban piotr.caban at gmail.com
Thu Jan 14 00:38:39 CST 2010


Hi,

There're still some things that needs to be changed.

In prop_sheet_proc:
+    case WM_DESTROY:
+      {
+        OLEPropertyFrame *opf = (OLEPropertyFrame
*)GetWindowLongPtrW(hwnd, DWLP_USER);
+        if(opf) {
+            IPropertyPage_Show(opf->propPage, SW_HIDE);
+            IPropertyPage_Deactivate(opf->propPage);
+            SetWindowLongPtrW(hwnd, DWLP_USER, (LONG)NULL);
+        }
+      }
there's no break/return

In pixels_to_dialog_units:
+        *basex = ((float)size.cx/26+1)/2;

+    *x_pixels = MulDiv(*x_pixels, 4, *basex);
I proposed doing the computations on floats so the dialog size is more
accurate. If it was working for you correctly than it's probably better
to keep it the way I proposed. The cast in the patch you've sent doesn't
change anything.
If there are reasons to make the computations on integers it's better to
use GdiGetCharDimensions function.

I think that pixels_to_dialog_units function should be deleted and it's
code move to OleCreatePropertyFrameIndirect.

+    /* This was expected to work but didn't:
+     * LONG baseunits = GetDialogBaseUnits();
+     * basex = LOWORD(baseunits);
+     * basey = HIWORD(baseunits);
+    */
This comment is not correct. GetDialogBaseUnits is somehow strange and
should be avoided.

About PropertyPageSite interface:
You have implemented AddRef and Release functions but you don't really
care about reference counter. The object should be freed when the
counter reaches 0.

+typedef struct {
+    struct {
+         DLGTEMPLATE dlg;
+         WORD menu;
+         WORD class;
+         WORD caption;
+         WORD font;
+    } dialog;
+    OLEPropertyPageSite pps;
+    IPropertyPage  *propPage;
+} OLEPropertyFrame;
You don't need font field.

Cheers,
Piotr



More information about the wine-devel mailing list