Am 16.01.2011 18:17, schrieb James McKenzie:
> On 1/16/11 9:55 AM, Nikolay Sivov wrote:
>> On 1/16/2011 19:39, James McKenzie wrote:
>>> All:
>>>
>>> I would like comments on this patch.
>> Ok.
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.
> We still do tests on Windows95 to prevent regressions, or am I incorrect?
> This code could be removed then.
see http://test.winehq.org/data/
>>> + 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?
> Should I go ahead and just 'return' then?
>>> + /* 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?
> I can fix this.
>>
>>> + 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.
> Ok. I missed this one.
>>
>>> + /*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?
> No. I will remove this code.
>>> + /* Per http://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.
> Can I modify this to state the page name?
remove the url, maybe better something like "msdn's documentation for
functionxyz..."
>>
>>> + 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.
>>
> Ok. I questioned this as well.
>>> + todo_wine {
>>> + SendMessageW(hwndRichEdit, EM_SETMARGINS, EC_USEFONTINFO,
>>> MAKELONG(EC_USEFONTINFO, EC_USEFONTINFO));
>>> + }
>> Hm.
> This is one of the test cases that was recommended and it appears to be the
> only one that 'works'.
i bet Nikolay sighed about the curly brackets
>>
>>> + 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.
> I will look at this as well. I'll have to figure out how to change the
> resolution on the test bot and or my local copy of WindowsXP SP2 (not
> upgradable, no Internet connection to that system.
to be clear: i guess it's about the dpi and not the resolution like 1024x768.
--
Best Regards, André Hentschel
