Re: user32: Add additional tests for scroll state, make them pass under Wine
2008/6/30 James Hawkins [EMAIL PROTECTED]:
On Mon, Jun 30, 2008 at 3:34 PM, Alex Villacís Lasso
[EMAIL PROTECTED] wrote:
...
+ok(si.nMin == 0xdeadbeef, si.nMin == 0x%x, expected
0xdeadbeef\n, si.nMin);
+ok(si.nMax == 0xdeadbeef, si.nMax == 0x%x, expected
0xdeadbeef\n, si.nMax);
+ok(si.nPos == 0xdeadbeef, si.nPos == 0x%x, expected
0xdeadbeef\n, si.nPos);
+ok(si.nPage == 0xdeadbeef, si.nPage == 0x%x, expected
0xdeadbeef\n, si.nPage);
+
+if (r == ERROR_NO_SCROLLBARS) behavior_winxp = TRUE;
+if (r == 0xdeadbeef) behavior_win98 = TRUE;
+} else {
+if (r2) {
+ok(si.nMin == 0, si.nMin == %d, expected 0\n, si.nMin);
+ok(si.nMax == 100, si.nMax == %d, expected 100\n, si.nMax);
+ok(si.nPos == 0, si.nPos == %d, expected 0\n, si.nPos);
+ok(si.nPage == 0, si.nPage == %d, expected 0\n, si.nPage);
+
+behavior_winxp = TRUE;
+} else {
+ok(r == 0xdeadbeef, GetScrollInfo failed, error is %ld
(0x%08lx) expected 0xdeadbeef\n, r, r);
+
+behavior_win98 = TRUE;
+}
+}
This is seriously complex (in a bad way). I think you've tried to
factor too much into one function. You don't need to set and check
the version, just test for both cases that are expected, like all the
other tests:
ok(si.nMin == 0 ||
si.nMin == 0xdeadbeef, si.nMin == 0x%x, expected 0 or
0xdeadbeef\n, si.nMin);
Francois Gouget also implemented a broken() method so you can get Wine
to do the right thing, something like:
ok(si.nMin == 0 /* XP */ ||
broken(si.nMin == 0xdeadbeef) /* 98 */ ,
si.nMin == 0x%x, expected 0 or 0xdeadbeef\n, si.nMin);
- Reece
user32: Add additional tests for scroll state, make them pass under Wine
This patch adds a few tests for scrollbar behavior in Windows. It shows
that if the window was created with at least WS_VSCROLL or WS_HSCROLL
styles, WinXP returns default information for scrollbar range, but Win98
returns an error, until first initialized. This error does not change if
styles are set or cleared before initializing ranges. On both versions,
setting either scrollbar range automatically initializes both if
required (Wine still returned error for the other scrollbar - fix
included). I have tried to test this on Win98 and WinXP-SP2, but maybe
other versions are different. Could you please test this patch? Also, is
it OK to tell apart Win9x vs. WinXP behavior in the way it is done in
this patch? Once either behavior is identified, it is consistent on all
future tests, as shown by the tests. Wine fixed to implement Win98 behavior.
Changelog:
* Document and test differences between WinXP and Win98 scrollbar behavior
* Make tests pass under Wine (Win98 behavior)
--
perl -e '$x=2.4;print sprintf(%.0f + %.0f = %.0f\n,$x,$x,$x+$x);'
From b76e90dbc6905daf76ec0df957bc93965f22464a Mon Sep 17 00:00:00 2001
From: =?utf-8?q?Alex=20Villac=C3=ADs=20Lasso?= [EMAIL PROTECTED]
Date: Sun, 29 Jun 2008 12:40:10 -0500
Subject: [PATCH] user32: Add additional tests for scroll state, make them pass under Wine.
---
dlls/user32/scroll.c | 36 +--
dlls/user32/tests/scroll.c | 273
2 files changed, 300 insertions(+), 9 deletions(-)
diff --git a/dlls/user32/scroll.c b/dlls/user32/scroll.c
index ddfacd6..e116357 100644
--- a/dlls/user32/scroll.c
+++ b/dlls/user32/scroll.c
@@ -140,6 +140,22 @@ static inline BOOL SCROLL_ScrollInfoValid( LPCSCROLLINFO info )
info-cbSize != sizeof(*info) - sizeof(info-nTrackPos)));
}
+static SCROLLBAR_INFO *SCROLL_AllocateInternalInfo(void)
+{
+SCROLLBAR_INFO *infoPtr;
+
+infoPtr = HeapAlloc( GetProcessHeap(), 0, sizeof(SCROLLBAR_INFO) );
+if (infoPtr)
+{
+/* Set default values */
+infoPtr-minVal = infoPtr-curVal = infoPtr-page = 0;
+/* From MSDN: max for a standard scroll bar is 100 by default. */
+infoPtr-maxVal = 100;
+/* Scroll bar is enabled by default after create */
+infoPtr-flags = ESB_ENABLE_BOTH;
+}
+return infoPtr;
+}
/***
* SCROLL_GetInternalInfo
@@ -166,16 +182,18 @@ static SCROLLBAR_INFO *SCROLL_GetInternalInfo( HWND hwnd, INT nBar, BOOL alloc )
{
if (nBar != SB_HORZ nBar != SB_VERT)
WARN(Cannot initialize nBar=%d\n,nBar);
-else if ((infoPtr = HeapAlloc( GetProcessHeap(), 0, sizeof(SCROLLBAR_INFO) )))
+else if ((infoPtr = SCROLL_AllocateInternalInfo()) != NULL)
{
-/* Set default values */
-infoPtr-minVal = infoPtr-curVal = infoPtr-page = 0;
-/* From MSDN: max for a standard scroll bar is 100 by default. */
-infoPtr-maxVal = 100;
-/* Scroll bar is enabled by default after create */
-infoPtr-flags = ESB_ENABLE_BOTH;
-if (nBar == SB_HORZ) wndPtr-pHScroll = infoPtr;
-else wndPtr-pVScroll = infoPtr;
+if (nBar == SB_HORZ)
+{
+wndPtr-pHScroll = infoPtr;
+if (!wndPtr-pVScroll) wndPtr-pVScroll = SCROLL_AllocateInternalInfo();
+}
+else
+{
+wndPtr-pVScroll = infoPtr;
+if (!wndPtr-pHScroll) wndPtr-pHScroll = SCROLL_AllocateInternalInfo();
+}
}
}
WIN_ReleasePtr( wndPtr );
diff --git a/dlls/user32/tests/scroll.c b/dlls/user32/tests/scroll.c
index 3bc407a..7b388ca 100644
--- a/dlls/user32/tests/scroll.c
+++ b/dlls/user32/tests/scroll.c
@@ -127,6 +127,219 @@ static void scrollbar_test3(void)
}
+static void scrollbar_test4(int fnBar, BOOL hBarEnabled, BOOL vBarEnabled)
+{
+SCROLLINFO si;
+LRESULT r;
+BOOL r2;
+BOOL behavior_winxp = FALSE;
+BOOL behavior_win98 = FALSE;
+
+r = GetWindowLongA(hMainWnd, GWL_STYLE);
+ok(((r WS_HSCROLL) != 0) == hBarEnabled,
+Unexpected WS_HSCROLL style, found %d expected %d\n,
+((r WS_HSCROLL) != 0), hBarEnabled);
+ok(((r WS_VSCROLL) != 0) == vBarEnabled,
+Unexpected WS_VSCROLL style, found %d expected %d\n,
+((r WS_VSCROLL) != 0), vBarEnabled);
+
+trace(Testing bar type %d with initial style (%d,%d)\n, fnBar, hBarEnabled, vBarEnabled);
+
+si.cbSize = sizeof(si);
+si.fMask = SIF_RANGE | SIF_PAGE | SIF_POS;
+si.nMin = si.nMax = si.nPos = si.nPage = 0xdeadbeef;
+SetLastError(0xdeadbeef);
+r2 = GetScrollInfo(hMainWnd, fnBar, si);
+r = GetLastError();
+if (!(hBarEnabled || vBarEnabled)) {
+ok(!r2, GetScrollInfo incorrectly succeeded\n);
+ok(( r == ERROR_NO_SCROLLBARS /* WinXP */
+|| r
Re: user32: Add additional tests for scroll state, make them pass under Wine
On Mon, Jun 30, 2008 at 3:34 PM, Alex Villacís Lasso
[EMAIL PROTECTED] wrote:
This patch adds a few tests for scrollbar behavior in Windows. It shows that
if the window was created with at least WS_VSCROLL or WS_HSCROLL styles,
WinXP returns default information for scrollbar range, but Win98 returns an
error, until first initialized. This error does not change if styles are set
or cleared before initializing ranges. On both versions, setting either
scrollbar range automatically initializes both if required (Wine still
returned error for the other scrollbar - fix included). I have tried to test
this on Win98 and WinXP-SP2, but maybe other versions are different. Could
you please test this patch? Also, is it OK to tell apart Win9x vs. WinXP
behavior in the way it is done in this patch? Once either behavior is
identified, it is consistent on all future tests, as shown by the tests.
Wine fixed to implement Win98 behavior.
Why in the world would you implement Win98 behavior over WinXP,
especially since the WinXP behavior is more logical and (probably,
haven't looked) stated on msdn?
+if (!(hBarEnabled || vBarEnabled)) {
+ok(!r2, GetScrollInfo incorrectly succeeded\n);
+ok(( r == ERROR_NO_SCROLLBARS /* WinXP */
+|| r == 0xdeadbeef /* Win98 */ ),
+GetLastError returned 0x%08lx expected 0x%08x
(ERROR_NO_SCROLLBARS or 0xdeadbeef)\n,
+r, ERROR_NO_SCROLLBARS);
That's hideous spacing just for the sake of lining up some error codes.
+ok(si.nMin == 0xdeadbeef, si.nMin == 0x%x, expected
0xdeadbeef\n, si.nMin);
+ok(si.nMax == 0xdeadbeef, si.nMax == 0x%x, expected
0xdeadbeef\n, si.nMax);
+ok(si.nPos == 0xdeadbeef, si.nPos == 0x%x, expected
0xdeadbeef\n, si.nPos);
+ok(si.nPage == 0xdeadbeef, si.nPage == 0x%x, expected
0xdeadbeef\n, si.nPage);
+
+if (r == ERROR_NO_SCROLLBARS) behavior_winxp = TRUE;
+if (r == 0xdeadbeef) behavior_win98 = TRUE;
+} else {
+if (r2) {
+ok(si.nMin == 0, si.nMin == %d, expected 0\n, si.nMin);
+ok(si.nMax == 100, si.nMax == %d, expected 100\n, si.nMax);
+ok(si.nPos == 0, si.nPos == %d, expected 0\n, si.nPos);
+ok(si.nPage == 0, si.nPage == %d, expected 0\n, si.nPage);
+
+behavior_winxp = TRUE;
+} else {
+ok(r == 0xdeadbeef, GetScrollInfo failed, error is %ld
(0x%08lx) expected 0xdeadbeef\n, r, r);
+
+behavior_win98 = TRUE;
+}
+}
This is seriously complex (in a bad way). I think you've tried to
factor too much into one function. You don't need to set and check
the version, just test for both cases that are expected, like all the
other tests:
ok(si.nMin == 0 ||
si.nMin == 0xdeadbeef, si.nMin == 0x%x, expected 0 or
0xdeadbeef\n, si.nMin);
--
James Hawkins
Re: user32: Add additional tests for scroll state, make them pass under Wine
Why in the world would you implement Win98 behavior over WinXP,
especially since the WinXP behavior is more logical and (probably,
haven't looked) stated on msdn?
I did not implement Win98 behavior - it was already implemented that way
in Wine, except for one difference that was found and changed to be
consistent. Having said that, I agree that it is better to implement the
WinXP behavior. I will do that in a future patch.
Reece Dunn escribió:
2008/6/30 James Hawkins [EMAIL PROTECTED]:
On Mon, Jun 30, 2008 at 3:34 PM, Alex Villacís Lasso
[EMAIL PROTECTED] wrote:
...
+ok(si.nMin == 0xdeadbeef, si.nMin == 0x%x, expected
0xdeadbeef\n, si.nMin);
+ok(si.nMax == 0xdeadbeef, si.nMax == 0x%x, expected
0xdeadbeef\n, si.nMax);
+ok(si.nPos == 0xdeadbeef, si.nPos == 0x%x, expected
0xdeadbeef\n, si.nPos);
+ok(si.nPage == 0xdeadbeef, si.nPage == 0x%x, expected
0xdeadbeef\n, si.nPage);
+
+if (r == ERROR_NO_SCROLLBARS) behavior_winxp = TRUE;
+if (r == 0xdeadbeef) behavior_win98 = TRUE;
+} else {
+if (r2) {
+ok(si.nMin == 0, si.nMin == %d, expected 0\n, si.nMin);
+ok(si.nMax == 100, si.nMax == %d, expected 100\n, si.nMax);
+ok(si.nPos == 0, si.nPos == %d, expected 0\n, si.nPos);
+ok(si.nPage == 0, si.nPage == %d, expected 0\n, si.nPage);
+
+behavior_winxp = TRUE;
+} else {
+ok(r == 0xdeadbeef, GetScrollInfo failed, error is %ld
(0x%08lx) expected 0xdeadbeef\n, r, r);
+
+behavior_win98 = TRUE;
+}
+}
This is seriously complex (in a bad way). I think you've tried to
factor too much into one function. You don't need to set and check
the version, just test for both cases that are expected, like all the
other tests:
ok(si.nMin == 0 ||
si.nMin == 0xdeadbeef, si.nMin == 0x%x, expected 0 or
0xdeadbeef\n, si.nMin);
Francois Gouget also implemented a broken() method so you can get Wine
to do the right thing, something like:
ok(si.nMin == 0 /* XP */ ||
broken(si.nMin == 0xdeadbeef) /* 98 */ ,
si.nMin == 0x%x, expected 0 or 0xdeadbeef\n, si.nMin);
- Reece
Thanks for the reminder. I will use it.
--
perl -e '$x=2.4;print sprintf(%.0f + %.0f = %.0f\n,$x,$x,$x+$x);'
