Re: TreeView notifications fix
"Mike Hearn" <mike(a)theoretic.com> wrote:
ChangeLog: - Make the change notifications respect unicode vs ansi.
- TVN_SELCHANGINGW, + infoPtr->bNtfUnicode ? TVN_SELCHANGINGA : TVN_SELCHANGINGW,
Shouldn't this be reversed? -- Dmitry.
On Sun, 2003-03-16 at 23:09, Dmitry Timoshkov wrote:
"Mike Hearn" <mike(a)theoretic.com> wrote:
ChangeLog: - Make the change notifications respect unicode vs ansi.
- TVN_SELCHANGINGW, + infoPtr->bNtfUnicode ? TVN_SELCHANGINGA : TVN_SELCHANGINGW,
Shouldn't this be reversed?
I was wondering about that when I looked at the code again after coming back to it a few hours later, but the other way around doesn't work properly. I expect bNtfUnicode isn't being set right. I'll take a look today. But that way around is definately the one that makes things work. -- Mike Hearn <m.hearn(a)signal.qinetiq.com> QinetiQ - Malvern Technology Center
I think the problem is that under W2K WinAmp reports the dialog as being ANSI, and under wine it's reported as Unicode (because it's registered using CreateWindowExW with a unicode window proc). I know in November Dimi was having fun with this sort of thing, and that in Windows the listview at any rate reports this kind of thing incorrectly. Dimi, do you have any tips? Hopefully later I'll be able to trace it using the MS logger and find out why it thinks it's actually an ansi window. On Mon, 2003-03-17 at 09:46, Mike Hearn wrote:
On Sun, 2003-03-16 at 23:09, Dmitry Timoshkov wrote:
"Mike Hearn" <mike(a)theoretic.com> wrote:
ChangeLog: - Make the change notifications respect unicode vs ansi.
- TVN_SELCHANGINGW, + infoPtr->bNtfUnicode ? TVN_SELCHANGINGA : TVN_SELCHANGINGW,
Shouldn't this be reversed?
I was wondering about that when I looked at the code again after coming back to it a few hours later, but the other way around doesn't work properly. I expect bNtfUnicode isn't being set right. I'll take a look today. But that way around is definately the one that makes things work. -- Mike Hearn <m.hearn(a)signal.qinetiq.com> QinetiQ - Malvern Technology Center
On March 17, 2003 06:57 am, Mike Hearn wrote:
I know in November Dimi was having fun with this sort of thing, and that in Windows the listview at any rate reports this kind of thing incorrectly. Dimi, do you have any tips?
Yes, I don't think this patch is right. Here is the problem: Common controls send notifications back to the parent. These notifications can be either Unicode or ANSI. The documented way of choosing which format to use is through the WM_NOTIFYFORMAT message, which is send to the parent upon control creation. This sounds fairly simple, and it's exactly what our controls do. Until sometimes in Nov 2002, when Aric Stewart posted this: http://www.winehq.com/hypermail/wine-devel/2002/11/1167.html That is, it seems like the listview always sends notifications in ANSI, no matter what you do! This obviously doesn't make much sense, but Aric said he tested it, and that's how it works. As a result he posted this patch, which latter got into the tree: http://www.winehq.com/hypermail/wine-patches/2002/11/0276.html At the time I was trying to figure out a similar problem in the Treeview, whereas putty broke when we did the 'right' thing and started sending Unicode notifications. It seemed then obvious to me that the fix would be to translate all notifications back to ANSI, as in the listview case, but for some unknown reason that did not fix Putty on my box, so I never sent the patch. (BTW, if you want to play with this idea, the thing to do is to modify the get_notifycode() function, not the callers). In Fec 2003, Drew "DanteAliegri" Ogle <dantealiegri(a)umbc.edu> started looking into this problem, and I told him the story. He wasn't too impressed (and rightfully so!), and he maintained that the Unicode notifications are actually sent. I asked him if he can run some tests, and he did. Here is what he found: So, this rather simple testcase shows that at least in XP.. The executables are at http://open-socket.dyndns.org/~dante/execs.tar.bz2 atest recieves TVN_SELCHANGEDA and wtest recieves TVN_SELCHANGEDW Also , http://msdn.microsoft.com/library/default.asp?url=/library/en-us/shellcc/pla... may be useful. In the remarks section it addresses this problem. I've attached the test program to this message, in case anyone wants to play with it. It would be very interesing if people ran the two executables that Drew provided on different versions of Windows, and see what we get. Also, it would be more than interesting if we could modify the tests to create a listview instead, and see if the listview never sends a Unicode notification. -- Dimi.
This sounds fairly simple, and it's exactly what our controls do. Until sometimes in Nov 2002, when Aric Stewart posted this: http://www.winehq.com/hypermail/wine-devel/2002/11/1167.html That is, it seems like the listview always sends notifications in ANSI, no matter what you do! This obviously doesn't make much sense, but Aric said he tested it, and that's how it works. As a result he posted this patch, which latter got into the tree: http://www.winehq.com/hypermail/wine-patches/2002/11/0276.html
Right, I found that this morning.
At the time I was trying to figure out a similar problem in the Treeview, whereas putty broke when we did the 'right' thing and started sending Unicode notifications. It seemed then obvious to me that the fix would be to translate all notifications back to ANSI, as in the listview case, but for some unknown reason that did not fix Putty on my box, so I never sent the patch. (BTW, if you want to play with this idea, the thing to do is to modify the get_notifycode() function, not the callers).
And yes, I found that this morning as well :) At least in WinAmp, changing it to ansi seems to fix it....
In Feb 2003, Drew "DanteAliegri" Ogle <dantealiegri(a)umbc.edu> started looking into this problem, and I told him the story.
Which he repeated to me on IRC on sunday (so big thanks to dante)
He wasn't too impressed (and rightfully so!), and he maintained that the Unicode notifications are actually sent. I asked him if he can run some tests, and he did. Here is what he found:
This bit confuses me... what do you mean by actually sent? As in, actually sent on Windows? At least one issue seems to be that the NOTIFYFORMAT message returns different results under Wine and Win2K, on Windows the WinAmp prefs dialog wants ANSI, on Wine it wants Unicode. So perhaps the bug is actually in the way Wine determines whether a dialog is ansi/unicode, and the treeview (and winamp) are both correct? I couldn't figure out how to tell for sure what type a given window is under Windows (in wine you can watch the traces). Is there a way, using a spy tool or something?
So, this rather simple testcase shows that at least in XP.. The executables are at http://open-socket.dyndns.org/~dante/execs.tar.bz2 atest recieves TVN_SELCHANGEDA and wtest recieves TVN_SELCHANGEDW
Also , http://msdn.microsoft.com/library/default.asp?url=/library/en-us/shellcc/pla... may be useful.
In the remarks section it addresses this problem.
I've attached the test program to this message, in case anyone wants to play with it. It would be very interesing if people ran the two executables that Drew provided on different versions of Windows, and see what we get. Also, it would be more than interesting if we could modify the tests to create a listview instead, and see if the listview never sends a Unicode notification.
Well, I changed treeview.c to use get_notifycode (ie it's doing the right thing). That breaks WinAmp, probably trillian and putty too. The test cases now work OK, and give expected results. They give expected results on Win2K Terminal Server edition as well. So it looks like the treeview component isn't buggy on Windows, it does send Unicode when it's asked to. That just makes me even more suspicious of the strangely morphing WM_NOTIFYFORMAT result. I tried finding which API call created the prefs dialog in WinAmp in a logger trace on XP, but couldn't, which confused the hell out of me, in Wine it appears as a CreateWindowExW (strangely, the others are CreateWindowExA) - BUT it doesn't appear in a relay trace and it doesn't seem to be loaded via GetProcAddress :( So I think IsWindowUnicode is giving a "wrong" result on Wine. But I don't know where to look next. -- Mike Hearn <m.hearn(a)signal.qinetiq.com> QinetiQ - Malvern Technology Center
On 17 Mar 2003, Mike Hearn wrote:
And yes, I found that this morning as well :) At least in WinAmp, changing it to ansi seems to fix it....
You mean just making the if() be trivially true?
In Feb 2003, Drew "DanteAliegri" Ogle <dantealiegri(a)umbc.edu> started looking into this problem, and I told him the story.
Which he repeated to me on IRC on sunday (so big thanks to dante)
Indeed -- a big thanks indeed.
This bit confuses me... what do you mean by actually sent? As in, actually sent on Windows?
Yes, that's my understading. You can just download the executables and run them yourself on Windows, see what you get.
At least one issue seems to be that the NOTIFYFORMAT message returns different results under Wine and Win2K, on Windows the WinAmp prefs dialog wants ANSI, on Wine it wants Unicode. So perhaps the bug is actually in the way Wine determines whether a dialog is ansi/unicode, and the treeview (and winamp) are both correct?
That's my intuition as well, but we need to test it on Windows.
Well, I changed treeview.c to use get_notifycode (ie it's doing the right thing). That breaks WinAmp, probably trillian and putty too.
What do you mean? It was already using get_notifycode, no changes are needed, AFAIK. What exaclty did you change?
The test cases now work OK, and give expected results. They give expected results on Win2K Terminal Server edition as well.
Cool.
So it looks like the treeview component isn't buggy on Windows, it does send Unicode when it's asked to.
Can you also look to duplicate the tests for the listview as well? That stange behaviour for listview bothers me greatly.
That just makes me even more suspicious of the strangely morphing WM_NOTIFYFORMAT result. I tried finding which API call created the prefs dialog in WinAmp in a logger trace on XP, but couldn't, which confused the hell out of me, in Wine it appears as a CreateWindowExW (strangely, the others are CreateWindowExA) - BUT it doesn't appear in a relay trace and it doesn't seem to be loaded via GetProcAddress :(
So I think IsWindowUnicode is giving a "wrong" result on Wine. But I don't know where to look next.
Why "you think"? It seems you actually see it give the wrong result, no? -- Dimi.
On Mon, 2003-03-17 at 15:22, Dimitrie O. Paun wrote:
On 17 Mar 2003, Mike Hearn wrote:
And yes, I found that this morning as well :) At least in WinAmp, changing it to ansi seems to fix it....
You mean just making the if() be trivially true?
As in always using the A version (not called get_notifycode)
Well, I changed treeview.c to use get_notifycode (ie it's doing the right thing). That breaks WinAmp, probably trillian and putty too.
What do you mean? It was already using get_notifycode, no changes are needed, AFAIK. What exaclty did you change?
It wasn't using get_notifycode before (which is why I didn't see that function), it always returned the W version, regardless of what the query had returned. See the original patch: - TVN_SELCHANGINGW, + infoPtr->bNtfUnicode ? TVN_SELCHANGINGA : TVN_SELCHANGINGW,
So it looks like the treeview component isn't buggy on Windows, it does send Unicode when it's asked to.
Can you also look to duplicate the tests for the listview as well? That stange behaviour for listview bothers me greatly.
I don't have VC++, so I'll look into getting mingw setup. I thought Aric did test cases and showed that Windows ignored the requested format? So it might be a different bug.
That just makes me even more suspicious of the strangely morphing WM_NOTIFYFORMAT result. I tried finding which API call created the prefs dialog in WinAmp in a logger trace on XP, but couldn't, which confused the hell out of me, in Wine it appears as a CreateWindowExW (strangely, the others are CreateWindowExA) - BUT it doesn't appear in a relay trace and it doesn't seem to be loaded via GetProcAddress :(
So I think IsWindowUnicode is giving a "wrong" result on Wine. But I don't know where to look next.
Why "you think"? It seems you actually see it give the wrong result, no?
Well, the result of WM_NOTIFYFORMAT is determined by the default window procedure, which calls IsWindowUnicode. IsWindowUnicode checks the type of the window procedure, which for Winamp prefs dialog is ANSI on W2K but Unicode on Wine. So, IsWindowUnicode() could well be correct and the bug lies elsewhere. But WinAmp calls CreateWindowExW, which means a unicode winproc, so it should be correct. That means either the wrong CreateWindowEx is being called (is it possible?), or the logic for determining whether a window is unicode or not is more complex than just "what type is the wndproc?". That is the point where I go "huh?!?" and get stuck. -- Mike Hearn <m.hearn(a)signal.qinetiq.com> QinetiQ - Malvern Technology Center
On 17 Mar 2003, Mike Hearn wrote:
You mean just making the if() be trivially true?
As in always using the A version (not called get_notifycode)
Huh? What do you mean "not called get_notifycode"??? What I mean by makgin the if() trivially true is: - if (!infoPtr->bNtfUnicode) + if (TRUE) That is, always translating to the A version.
What do you mean? It was already using get_notifycode, no changes are needed, AFAIK. What exaclty did you change?
It wasn't using get_notifycode before (which is why I didn't see that function), it always returned the W version, regardless of what the query had returned. See the original patch:
- TVN_SELCHANGINGW, + infoPtr->bNtfUnicode ? TVN_SELCHANGINGA : TVN_SELCHANGINGW,
It was using it, just search for it in the code. For example, in the example you give above, we have: if (TREEVIEW_SendTreeviewNotify(infoPtr, TVN_SELCHANGINGW, cause, TVIF_HANDLE | TVIF_STATE | TVIF_PARAM, prevSelect, newSelect)) And if you check the definition of TREEVIEW_SendTreeviewNotify, you see: nmhdr.hdr.code = get_notifycode(infoPtr, code); So no modifications are necessary, right?
Can you also look to duplicate the tests for the listview as well? That stange behaviour for listview bothers me greatly.
I don't have VC++, so I'll look into getting mingw setup. I thought Aric did test cases and showed that Windows ignored the requested format? So it might be a different bug.
You don't need VC++, the Makefile that came with the test is for mingw.
Well, the result of WM_NOTIFYFORMAT is determined by the default window procedure, which calls IsWindowUnicode.
IsWindowUnicode checks the type of the window procedure, which for Winamp prefs dialog is ANSI on W2K but Unicode on Wine. So,
And this is a bug, no?
IsWindowUnicode() could well be correct and the bug lies elsewhere. But
How can that be, if we get different results than on Windows?
WinAmp calls CreateWindowExW, which means a unicode winproc, so it should be correct. That means either the wrong CreateWindowEx is being called (is it possible?), or the logic for determining whether a window is unicode or not is more complex than just "what type is the wndproc?".
That seems to be the case, yes. Maybe they look somehow at the type of executable? Is there anything about Unicode in the executable? -- Dimi.
And if you check the definition of TREEVIEW_SendTreeviewNotify, you see:
nmhdr.hdr.code = get_notifycode(infoPtr, code);
So no modifications are necessary, right?
D'oh. The way this windows unicode stuff works behind the scenes is pretty confusing, I assumed if you specified it with a W or A extension that's the one it'd use, clearly not. Sorry :(
IsWindowUnicode() could well be correct and the bug lies elsewhere. But
How can that be, if we get different results than on Windows?
Well the bug might be in window creation or class registration I suppose. As IsWindowUnicode on Wine just checks the window proc type, and WinAmp on wine uses a W wndproc but an A wndproc on Windows, I'd guess it's in class registration or window creation though.
WinAmp calls CreateWindowExW, which means a unicode winproc, so it should be correct. That means either the wrong CreateWindowEx is being called (is it possible?), or the logic for determining whether a window is unicode or not is more complex than just "what type is the wndproc?".
That seems to be the case, yes. Maybe they look somehow at the type of executable? Is there anything about Unicode in the executable?
This is the MSDN page for IsWindowUnicode(): http://msdn.microsoft.com/library/default.asp?url=/library/en-us/winui/winui... Unfortunately, parts of it appear to have been written by somebody who was either exhausted or stoned, for instance the part about 9x behaviour. The unicode page link is 404d also. It implies that the character set of a window is determined by * on 9x, which CreateWindowExA/W call was used. * or which RegisterClassA/W call was used. Well, WinAmp seems to use CreateWindowExW to create the window, but I can't find where the window class is registered, it doesn't seem to appear in a +class trace anywhere. The string name is "#32770", so maybe it's internal. So anyway, I'm not sure where the character set of the wndproc is determined, but perhaps the set of the RegisterClass call should take priority over CreateWindowEx and in Wine it doesn't. I'll see if I can find where the window class WinAmp uses is registered tomorrow if nobody knows. Could somebody explain whether Wine is supposed to be closer to NT or 9x? When there are two conflicting behaviours, do we go with the NT one or the 9x one (or both)? thanks -mike
Could somebody explain whether Wine is supposed to be closer to NT or 9x? When there are two conflicting behaviours, do we go with the NT one or the 9x one (or both)?
When in doubt please try to make it act like Windows NT. __________________________________________________ Do you Yahoo!? Yahoo! Web Hosting - establish your business online http://webhosting.yahoo.com
And this is a bug, no?
IsWindowUnicode() could well be correct and the bug lies elsewhere. But
How can that be, if we get different results than on Windows?
Turns out you were right, the bug seems to be in IsWindowUnicode. It only ever looks at the winproc, but dialogs (for some reason) seem to use their own, in the extra window bytes. Adding in a check for that, fixed WinAmp. I think this is definately a more correct fix than my last one. ChangeLog: - Make IsWindowUnicode correct for dialogs thanks to dante and dimi for this patch, -mike
On March 19, 2003 06:39 am, Mike Hearn wrote:
- + LONG *dlgWndProc; + if (!(wndPtr = WIN_FindWndPtr(hwnd))) return FALSE; - retvalue = (WINPROC_GetProcType( wndPtr->winproc ) == WIN_PROC_32W); + /* dialog boxes store their window proc in a different location, so check where we should look here */ + dlgWndProc = (LONG *)(((char *)wndPtr->wExtra) + DWL_DLGPROC); + + if (wndPtr->flags & WIN_ISDIALOG) + retvalue = (WINPROC_GetProcType( (WNDPROC)*dlgWndProc ) == WIN_PROC_32W); + else + retvalue = (WINPROC_GetProcType( wndPtr->winproc ) == WIN_PROC_32W); + TRACE("hwnd=%p, proc=%p, retvalue=%d\n", hwnd, wndPtr->winproc, retvalue);
Very nice! But why not code it a bit more "streamlined" (hand made diff): + WNDPROC wndproc = (wndPtr->flags & WIN_ISDIALOG) ? + *(WNDPROC *)(((char *)wndPtr->wExtra) + DWL_DLGPROC) : wndPtr->winproc; + - retvalue = (WINPROC_GetProcType( wndPtr->winproc ) == WIN_PROC_32W); + retvalue = (WINPROC_GetProcType( winproc ) == WIN_PROC_32W); -- Dimi.
I couldn't figure out how to tell for sure what type a given window is under Windows (in wine you can watch the traces). Is there a way, using a spy tool or something?
Oh, found out a way, it turns out Spy++ puts "(Unicode)" next to the window proc address if it's unicode, but doesn't put anything next to it if it's ANSI, hence the fact that I didn't realise it'd tell you... sorry :/ -- Mike Hearn <m.hearn(a)signal.qinetiq.com> QinetiQ - Malvern Technology Center
participants (5)
-
Dimitrie O. Paun -
Dimitrie O. Paun -
Dmitry Timoshkov -
Mike Hearn -
Steven Edwards