<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN">
<html>
<head>
<meta content="text/html; charset=ISO-8859-1"
http-equiv="Content-Type">
</head>
<body bgcolor="#ffffff" text="#000000">
On 1/16/2011 19:39, James McKenzie wrote:
<blockquote cite="mid:4D331F47.10508@earthlink.net" type="cite">All:
<br>
<br>
I would like comments on this patch.
<br>
</blockquote>
Ok.<br>
<br>
<blockquote type="cite">
<pre wrap=""> 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;
</pre>
</blockquote>
Commented code?<br>
<br>
<blockquote type="cite">
<pre wrap="">+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);
+}
</pre>
</blockquote>
I think we don't care much about 9x now.<br>
<blockquote type="cite">
<pre wrap="">+ 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;
+ }
</pre>
</blockquote>
DeleteObject() with uninitialized handles?<br>
<blockquote type="cite">
<pre wrap="">+ /* 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};
+
</pre>
</blockquote>
Variable declaration in code?<br>
<br>
<blockquote type="cite">
<pre wrap="">+ 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);
+ }
</pre>
</blockquote>
todo_wine usually contains only test calls.<br>
<br>
<blockquote type="cite">
<pre wrap="">+ /*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);
</pre>
</blockquote>
Do you really need to verify that?<br>
<blockquote type="cite">
<pre wrap="">+ /* Per <a class="moz-txt-link-freetext" href="http://msdn.microsoft.com/en-us/library/bb761649%%28VS.85%29.asp">http://msdn.microsoft.com/en-us/library/bb761649%(VS.85).asp</a> if the lparam is
+ EC_USEFONTINFO the Left and Right Margins should be set to a narrow width if the
+ font has been set */
</pre>
</blockquote>
They don't use permanent links I believe.<br>
<br>
<blockquote type="cite">
<pre wrap="">+ 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 /<b class="moz-txt-star"><span class="moz-txt-tag">*</span>Windows<span class="moz-txt-tag">*</span></b>/ || -195 == CharFont1Unicode.yHeight /<b class="moz-txt-star"><span class="moz-txt-tag">*</span>Wine<span class="moz-txt-tag">*</span></b>/, \
+ "Character height for MS Sans Serif is not 195 but is %d\n", abs(CharFont1Unicode.yHeight));
+ ok (10 == (abs(CharFont1Unicode.yHeight) * ry) / 1440 /<b class="moz-txt-star"><span class="moz-txt-tag">*</span>Windows<span class="moz-txt-tag">*</span></b>/ || 13 == (abs(CharFont1Unicode.yHeight) * ry) / 1440 /<b class="moz-txt-star"><span class="moz-txt-tag">*</span>Wine<span class="moz-txt-tag">*</span></b>/, \
+ "Character Height of MS San Serif character set is not 12 but %d\n", abs(CharFont1Unicode.yHeight) * ry / 1440);
+ ok (0 == CharFont1Unicode.wWeight /<b class="moz-txt-star"><span class="moz-txt-tag">*</span>Windows<span class="moz-txt-tag">*</span></b>/ || FW_NORMAL == CharFont1Unicode.wWeight /<b class="moz-txt-star"><span class="moz-txt-tag">*</span>Wine<span class="moz-txt-tag">*</span></b>/, "Average Character Weight for MS"\
+ " Sans Serif is %d\n", CharFont1Unicode.wWeight);
</pre>
</blockquote>
I don't like that. Looks like it's necessary to figure out why
values differ comparing to native.<br>
<br>
<blockquote type="cite">
<pre wrap="">+ todo_wine {
+ SendMessageW(hwndRichEdit, EM_SETMARGINS, EC_USEFONTINFO, MAKELONG(EC_USEFONTINFO, EC_USEFONTINFO));
+ }
</pre>
</blockquote>
Hm.<br>
<br>
<blockquote type="cite">
<pre wrap="">+ 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();
</pre>
</blockquote>
Why?<br>
<br>
Another question is how screen resolution affects that test.<br>
<br>
<br>
<br>
</body>
</html>