On 1/16/2011 19:39, James McKenzie wrote:
All:

I would like comments on this patch.
Ok.

  static HWND new_window(LPCTSTR lpClassName, DWORD dwStyle, HWND parent) {
    HWND hwnd;
-  hwnd = CreateWindow(lpClassName, NULL, dwStyle|WS_POPUP|WS_HSCROLL|WS_VSCROLL
+/*  hwnd = CreateWindow(lpClassName, NULL, 
dwStyle|WS_POPUP|WS_HSCROLL|WS_VSCROLL
                        |WS_VISIBLE, 0, 0, 200, 60, parent, NULL,
-                      hmoduleRichEdit, NULL);
+                      hmoduleRichEdit, NULL); */
+  /* Set screen to VGA proportions for testing purposes, will be removed from 
final patch release */
+    hwnd = CreateWindow(lpClassName, NULL, 
dwStyle|WS_POPUP|WS_HSCROLL|WS_VSCROLL
+                        |WS_VISIBLE, 0, 0, 640, 480, parent, NULL,
+                        hmoduleRichEdit, NULL);
    ok(hwnd != NULL, "class: %s, error: %d\n", lpClassName, (int) 
GetLastError());
    return hwnd;
Commented code?

+static inline int is_win9xA()
+{
+    static WCHAR arialW[]={'A','r','i','a','l',0};
+    SetLastError(0xdeadbeef);
+    lstrcmpW(arialW, arialW);
+    return (GetLastError() == ERROR_CALL_NOT_IMPLEMENTED);
+}
I think we don't care much about 9x now.
+    hDC = GetDC(hwndRichEdit);
+    ok (hDC, "Creation of hdc failed \n");
+    if (!hDC)
+    {
+        DeleteObject(testFont1A);
+        DeleteObject(testFont2A);
+        DeleteObject(testFont1W);
+        DeleteObject(testFont2W);
+        DeleteObject(testFont3W);
+        DeleteObject(testFont4W);
+        DeleteDC(hDC);
+        DestroyWindow(hwndRichEdit);
+        return;
+    }
DeleteObject() with uninitialized handles?
+    /* Calculate the twips per pixel */
+    ry = GetDeviceCaps(hDC, LOGPIXELSY);
+
+    static const WCHAR arialW[] = {'A','r','i','a','l',0};
+    static const WCHAR courier[] = {'C','o','u','r','i','e','r',0};
+    static const WCHAR msSansSerif [] = {'M','S',' ','S','a','n','s',' 
','S','e','r','i','f',0};
+
Variable declaration in code?

+    todo_wine {
+    SendMessageA(hwndRichEdit, EM_SETMARGINS, EC_LEFTMARGIN, MAKELONG (5,0));
+    SendMessageA(hwndRichEdit, EM_GETRECT, 0, (LPARAM)&new_rect);
+    ok(new_rect.left == old_rect.left + 5, "The left border of the rectangle is 
wrong.  The value of it is %d.\n", \
+    new_rect.left);
+    }
todo_wine usually contains only test calls.

+    /*Set test font to be Courier font */
+    SendMessageW(hwndRichEdit, WM_SETFONT, (WPARAM)testFont1W, 
MAKELPARAM(TRUE, 0));
+    /*Verify font is set */
+    SendMessageW(hwndRichEdit, EM_GETCHARFORMAT, SCF_DEFAULT, 
(LPARAM)&CharFont1Unicode);
+    GetObjectW(testFont1W, sizeof(LOGFONTW),&sentLogFont);
Do you really need to verify that?
+    /* Perhttp://msdn.microsoft.com/en-us/library/bb761649%(VS.85).asp  if the 
lparam is
+       EC_USEFONTINFO the Left and Right Margins should be set to a narrow 
width if the
+       font has been set */
They don't use permanent links I believe.

+    ret = GetTextMetricsW (hDC,&tmw);
+    ok (ret, "GetTextMetricsW failed\n");
+    ok (7 == tmw.tmAveCharWidth, "Average Character Width for MS Sans Serif is 
%d\n", tmw.tmAveCharWidth);
+    ok (14 == tmw.tmMaxCharWidth, "Maximum Character Width for MS Sans Serif is 
%d\n", tmw.tmMaxCharWidth);
+    ok (150== CharFont1Unicode.yHeight /*Windows*/ || -195 == 
CharFont1Unicode.yHeight /*Wine*/, \
+       "Character height for MS Sans Serif is not 195 but is %d\n", 
abs(CharFont1Unicode.yHeight));
+    ok (10 == (abs(CharFont1Unicode.yHeight) * ry) / 1440 /*Windows*/ || 13 == 
(abs(CharFont1Unicode.yHeight) * ry) / 1440 /*Wine*/, \
+       "Character Height of MS San Serif character set is not 12 but %d\n", 
abs(CharFont1Unicode.yHeight) * ry / 1440);
+    ok (0 == CharFont1Unicode.wWeight /*Windows*/ || FW_NORMAL == 
CharFont1Unicode.wWeight /*Wine*/, "Average Character Weight for MS"\
+        " Sans Serif is %d\n", CharFont1Unicode.wWeight);
I don't like that. Looks like it's necessary to figure out why values differ comparing to native.

+    todo_wine {
+        SendMessageW(hwndRichEdit, EM_SETMARGINS, EC_USEFONTINFO, 
MAKELONG(EC_USEFONTINFO, EC_USEFONTINFO));
+    }
Hm.

+  if (winetest_debug>  1) {
    test_WM_CHAR();
    test_EM_FINDTEXT();
    test_EM_GETLINE();
@@ -7126,6 +7643,8 @@ START_TEST( editor )
    test_WM_GETDLGCODE();
    test_zoom();
    test_dialogmode();
+  }
+  test_em_setmargins();
Why?

Another question is how screen resolution affects that test.





Reply via email to