Hello, Testing showed that the current implementation of notify_dispinfoT doesnt work with W2K. (or at least not with SP4 (: ) The attached patch corrects that our Listview doesnt display anything on native.
Following assumption proved to be wrong for the Message LVN_GETDISPINFOW: /* With testing on Windows 2000 it looks like the notify format has nothing to do with this message. It ALWAYS seems to be in ansi format. */
This time, only LVN_*ETDISPINFOW is patched, anything else is untouched. -Anybody please tell me if you see problems with this Fix since i dont want to break things.-
Tho
Thorsten Kani wrote:
Hello, Testing showed that the current implementation of notify_dispinfoT doesnt work with W2K. (or at least not with SP4 (: ) The attached patch corrects that our Listview doesnt display anything on native.
Following assumption proved to be wrong for the Message LVN_GETDISPINFOW: /* With testing on Windows 2000 it looks like the notify format has nothing to do with this message. It ALWAYS seems to be in ansi format. */
This time, only LVN_*ETDISPINFOW is patched, anything else is untouched. -Anybody please tell me if you see problems with this Fix since i dont want to break things.-
I have seen exactly the same problem with the IShellView implementation in native shell32. I have attached the patch I did at the time. I didn't submit it, because I soon got into a tangled mess where the function name was wrong and the isW parameter completely unnecessary and a whole bunch of functions calling it were also wrong.
To summarise: *all* common control notifications should be sent in the same format (ANSI/Unicode) as their parent (except if overriden by the CCS_SETUNICODEFORMAT message). It should not be based on the message sent to the control.
Rob
Index: listview.c =================================================================== RCS file: /home/wine/wine/dlls/comctl32/listview.c,v retrieving revision 1.394 diff -u -p -r1.394 listview.c --- listview.c 2 Sep 2004 23:00:53 -0000 1.394 +++ listview.c 27 Oct 2004 20:16:51 -0000 @@ -818,10 +818,6 @@ static int get_ansi_notification(INT uni }
/* - With testing on Windows 2000 it looks like the notify format - has nothing to do with this message. It ALWAYS seems to be - in ansi format. - infoPtr : listview struct notificationCode : *Unicode* notification code pdi : dispinfo structure (can be unicode or ansi) @@ -830,14 +826,10 @@ static int get_ansi_notification(INT uni static BOOL notify_dispinfoT(LISTVIEW_INFO *infoPtr, INT notificationCode, LPNMLVDISPINFOW pdi, BOOL isW) { BOOL bResult = FALSE; - BOOL convertToAnsi = FALSE; INT cchTempBufMax = 0, savCchTextMax = 0; LPWSTR pszTempBuf = NULL, savPszText = NULL;
- if ((pdi->item.mask & LVIF_TEXT) && is_textT(pdi->item.pszText, isW)) - convertToAnsi = isW; - - if (convertToAnsi) + if ((pdi->item.mask & LVIF_TEXT) && (infoPtr->notifyFormat == NFR_ANSI)) { if (notificationCode != LVN_GETDISPINFOW) { @@ -861,15 +853,16 @@ static BOOL notify_dispinfoT(LISTVIEW_IN savPszText = pdi->item.pszText; pdi->item.pszText = pszTempBuf; pdi->item.cchTextMax = cchTempBufMax; + + notificationCode = get_ansi_notification(notificationCode); }
TRACE(" pdi->item=%s\n", debuglvitem_t(&pdi->item, infoPtr->notifyFormat != NFR_ANSI));
- bResult = notify_hdr(infoPtr, get_ansi_notification(notificationCode), - (LPNMHDR)pdi); + bResult = notify_hdr(infoPtr, notificationCode, &pdi->hdr);
- if (convertToAnsi) + if ((pdi->item.mask & LVIF_TEXT) && (infoPtr->notifyFormat == NFR_ANSI)) { MultiByteToWideChar(CP_ACP, 0, (LPSTR) pdi->item.pszText, -1, savPszText, savCchTextMax);
Testing out your more general solution introduced no Problems with the Listview on my Testsystem. If the Patch gets applied, i will modify the callers of notify_dispinfo to respect that "isW" isnt longer used.
Tho
Robert Shearman wrote:
Thorsten Kani wrote:
Hello, Testing showed that the current implementation of notify_dispinfoT doesnt work with W2K. (or at least not with SP4 (: ) The attached patch corrects that our Listview doesnt display anything on native.
Following assumption proved to be wrong for the Message LVN_GETDISPINFOW: /* With testing on Windows 2000 it looks like the notify format has nothing to do with this message. It ALWAYS seems to be in ansi format. */
This time, only LVN_*ETDISPINFOW is patched, anything else is untouched. -Anybody please tell me if you see problems with this Fix since i dont want to break things.-
I have seen exactly the same problem with the IShellView implementation in native shell32. I have attached the patch I did at the time. I didn't submit it, because I soon got into a tangled mess where the function name was wrong and the isW parameter completely unnecessary and a whole bunch of functions calling it were also wrong.
To summarise: *all* common control notifications should be sent in the same format (ANSI/Unicode) as their parent (except if overriden by the CCS_SETUNICODEFORMAT message). It should not be based on the message sent to the control.
Rob
Index: listview.c
RCS file: /home/wine/wine/dlls/comctl32/listview.c,v retrieving revision 1.394 diff -u -p -r1.394 listview.c --- listview.c 2 Sep 2004 23:00:53 -0000 1.394 +++ listview.c 27 Oct 2004 20:16:51 -0000 @@ -818,10 +818,6 @@ static int get_ansi_notification(INT uni }
/*
- With testing on Windows 2000 it looks like the notify format
- has nothing to do with this message. It ALWAYS seems to be
- in ansi format.
- infoPtr : listview struct notificationCode : *Unicode* notification code pdi : dispinfo structure (can be unicode or ansi)
@@ -830,14 +826,10 @@ static int get_ansi_notification(INT uni static BOOL notify_dispinfoT(LISTVIEW_INFO *infoPtr, INT notificationCode, LPNMLVDISPINFOW pdi, BOOL isW) { BOOL bResult = FALSE;
BOOL convertToAnsi = FALSE; INT cchTempBufMax = 0, savCchTextMax = 0; LPWSTR pszTempBuf = NULL, savPszText = NULL;
if ((pdi->item.mask & LVIF_TEXT) && is_textT(pdi->item.pszText, isW))
convertToAnsi = isW;if (convertToAnsi)
- if ((pdi->item.mask & LVIF_TEXT) && (infoPtr->notifyFormat == NFR_ANSI)) { if (notificationCode != LVN_GETDISPINFOW) {
@@ -861,15 +853,16 @@ static BOOL notify_dispinfoT(LISTVIEW_IN savPszText = pdi->item.pszText; pdi->item.pszText = pszTempBuf; pdi->item.cchTextMax = cchTempBufMax;
notificationCode = get_ansi_notification(notificationCode);}
TRACE(" pdi->item=%s\n", debuglvitem_t(&pdi->item, infoPtr->notifyFormat != NFR_ANSI));
- bResult = notify_hdr(infoPtr, get_ansi_notification(notificationCode),
(LPNMHDR)pdi);
- bResult = notify_hdr(infoPtr, notificationCode, &pdi->hdr);
- if (convertToAnsi)
- if ((pdi->item.mask & LVIF_TEXT) && (infoPtr->notifyFormat == NFR_ANSI)) { MultiByteToWideChar(CP_ACP, 0, (LPSTR) pdi->item.pszText, -1, savPszText, savCchTextMax);
On Wed, Oct 27, 2004 at 09:28:33PM +0100, Robert Shearman wrote:
To summarise: *all* common control notifications should be sent in the same format (ANSI/Unicode) as their parent (except if overriden by the CCS_SETUNICODEFORMAT message). It should not be based on the message sent to the control.
Just to be 100% clear: -- what do you mean by "in the same format as their parent"? From your patch, it seems that: 1. In WM_CREATE, we have to query the notify format as such:
infoPtr->hwndNotify = lpcs->hwndParent; infoPtr->notifyFormat = SendMessageW(infoPtr->hwndNotify, WM_NOTIFYFORMAT, (WPARAM)infoPtr->hwndSelf, (LPARAM)NF_QUERY);
2. Handle WM_NOTIFYFORMAT, by querying the parent again:
infoPtr->notifyFormat = SendMessageW(hwndFrom, WM_NOTIFYFORMAT, (WPARAM)infoPtr->hwndSelf, NF_QUERY);
3. When sending notifications, convert to ASCII iff infoPtr->notifyFormat == NFR_ANSI
If so, this is what we had before Aric did this change: http://cvs.winehq.org/cvsweb/wine/dlls/comctl32/listview.c.diff?r1=1.330&... What was wrong with the code before? It tried to send the notification in the format specified by infoPtr->notifyFormat. The dependance on the the type of message that was sent to the control is just to do the conversion when we have to. That is:
fmt-of-msg-sent-to-ctrl infoPtr->notifyFormat conversion-required !isW (ASCII) NFR_ANSI no isW (Unicode) NFR_ANSI yes !isW (ASCII) NFR_UNICODE yes isW (Unicode) NFR_UNICODE no
The old code looked as isW only to determine if any conversion was required (which I think you have to), but the format of the actual notification was strictly determined by infoPtr->notifyFormat:
if (infoPtr->notifyFormat == NFR_ANSI) realNotifCode = get_ansi_notification(notificationCode); else realNotifCode = notificationCode; bResult = notify_hdr(infoPtr, realNotifCode, (LPNMHDR)pdi);
So, what was wrong with the original code?
-- CCS_SETUNICODEFORMAT message: this is a flag, not a message, no? If so, what is its semantics? If set, we have to ignore infoPtr->notifyFormat, and always send notifications in Unicode format?
It's important to figure this one once and for all, so that we can fix all the controls properly.
Dimitrie O. Paun wrote:
On Wed, Oct 27, 2004 at 09:28:33PM +0100, Robert Shearman wrote:
To summarise: *all* common control notifications should be sent in the same format (ANSI/Unicode) as their parent (except if overriden by the CCS_SETUNICODEFORMAT message). It should not be based on the message sent to the control.
Just to be 100% clear: -- what do you mean by "in the same format as their parent"?
The format is retrieved using the WM_NOTIFYFORMAT message, but can be overridden by CCM_SETUNICODEFORMAT.
From your patch, it seems that:
- In WM_CREATE, we have to query the notify format as such:
infoPtr->hwndNotify = lpcs->hwndParent; infoPtr->notifyFormat = SendMessageW(infoPtr->hwndNotify, WM_NOTIFYFORMAT, (WPARAM)infoPtr->hwndSelf, (LPARAM)NF_QUERY);
Yes, that is correct. It would be a lot better to use a boolean flag rather than storing the actual format code so that: if (infoPtr->notifyFormat == NFR_UNICODE) becomes: if (infoPtr->bUnicode)
- Handle WM_NOTIFYFORMAT, by querying the parent again:
infoPtr->notifyFormat = SendMessageW(hwndFrom, WM_NOTIFYFORMAT, (WPARAM)infoPtr->hwndSelf, NF_QUERY);
Yes. It should also handle the NF_QUERY case in the WM_NOTIFYFORMAT handler so that child windows, like the header control, will get a sane value.
3. When sending notifications, convert to ASCII iff infoPtr->notifyFormat == NFR_ANSI If so, this is what we had before Aric did this change:http://cvs.winehq.org/cvsweb/wine/dlls/comctl32/listview.c.diff?r1=1.330&... What was wrong with the code before? It tried to send the notification in the format specified by infoPtr->notifyFormat. The dependance on the the type of message that was sent to the control is just to do the conversion when we have to. That is:
fmt-of-msg-sent-to-ctrl infoPtr->notifyFormat conversion-required !isW (ASCII) NFR_ANSI no isW (Unicode) NFR_ANSI yes !isW (ASCII) NFR_UNICODE yes isW (Unicode) NFR_UNICODE no The old code looked as isW only to determine if any conversion was required (which I think you have to), but the format of the actual notification was strictly determined by infoPtr->notifyFormat: if (infoPtr->notifyFormat == NFR_ANSI) realNotifCode = get_ansi_notification(notificationCode); else realNotifCode = notificationCode; bResult = notify_hdr(infoPtr, realNotifCode, (LPNMHDR)pdi); So, what was wrong with the original code?
The old code does indeed send the correct notifications. Maybe there was a bug in the function somewhere that prompted Aric to make his change. However, the code could be a lot simpler by doing conversions from A to W on incoming messages and then only doing W to A conversions for the notifications, rather than the matrix of 4 cases you described above.
-- CCS_SETUNICODEFORMAT message: this is a flag, not a message, no? If so, what is its semantics? If set, we have to ignore infoPtr->notifyFormat, and always send notifications in Unicode format?
That was a mistake. I meant CCM_SETUNICODEFORMAT. (http://msdn.microsoft.com/library/default.asp?url=/library/en-us/shellcc/pla...)
It's important to figure this one once and for all, so that we can fix all the controls properly.
Rob
On Sun, Oct 31, 2004 at 10:50:33AM +0000, Robert Shearman wrote:
Yes, that is correct. It would be a lot better to use a boolean flag rather than storing the actual format code so that: if (infoPtr->notifyFormat == NFR_UNICODE) becomes: if (infoPtr->bUnicode)
Maybe a bit simpler yes, but surely not a lot :) Anyway, this flag is tested for NFR_* only _once_ in a 10000 line file, so I actually think that sticking to the standard is a better option here.
The old code does indeed send the correct notifications. Maybe there was a bug in the function somewhere that prompted Aric to make his change. However, the code could be a lot simpler by doing conversions from A to W on incoming messages and then only doing W to A conversions for the notifications, rather than the matrix of 4 cases you described above.
Oh, come on, it was just a few more lines of code. In fact, I was happy with the code as it standed (that's no surprise, as I wrote it :)). I think we should just revert to it, and accept changes only with supporting tests. At the time that Aric submitted the patch, I was rather unhappy with it, but he claimed he performed extensive tests that proved that behaviour. He should have submitted them as part of the test suite.
Oh, come on, it was just a few more lines of code. In fact, I was happy with the code as it standed (that's no surprise, as I wrote it :)). I think we should just revert to it, and accept changes only with supporting tests. At the time that Aric submitted the patch, I was rather unhappy with it, but he claimed he performed extensive tests that proved that behaviour. He should have submitted them as part of the test suite.
As i understand it, the old Listview code was assuming that LVN_GETDISPINFOW is never used and tried to convince apps with the "A" version of the Message.. This leads to problems with Braindamaged_Apps(TM) wich handle only the Unicode Message. (Tested App was "Explorer")
On Mon, Nov 01, 2004 at 01:43:44AM +0100, Thorsten Kani wrote:
As i understand it, the old Listview code was assuming that LVN_GETDISPINFOW is never used and tried to convince apps with the "A" version of the Message..
Why are you saying that. The code treats LVN_GETDISPINFOW special because it's the only type of message that *gets* textual data rather then sending. SO for this reason, we just reset the buffer on the way out. What other difference is there?
This leads to problems with Braindamaged_Apps(TM) wich handle only the Unicode Message. (Tested App was "Explorer")
Heh. Right now we only send ASCII notifications, which would be even worse. We need proper test cases.
I asked the List about the Problem and how to solve it correctly. Please excuse my bad English (: - will try your patch asap. Tho
Why are you saying that. The code treats LVN_GETDISPINFOW special because it's the only type of message that *gets* textual data rather then sending. SO for this reason, we just reset the buffer on the way out. What other difference is there?
This leads to problems with Braindamaged_Apps(TM) wich handle only the Unicode Message. (Tested App was "Explorer")
Heh. Right now we only send ASCII notifications, which would be even worse. We need proper test cases.
-
Dimitrie O. Paun -
Robert Shearman -
Thorsten Kani