LISTVIEW_SetItemState problem in FlashFXP (and other applications)

[Rolf Kalbermatter]

I was just discussing the problem with FlashFXP on Wine and did some
Google search on this.
It turned out that back in 2003 this problem was already noticed by one
of the developers
of FlashFXP and also somehow documented as follows:
> http://forum.ru-board.com/topic.cgi?forum=5&topic=0132&start=240:
> - FlashFXP is no longer compatible with WINE, Switching to the

OwnerData
> listview has broken compatiblity, it's unlikely that WINE will be

supported.
> WINE needs to be made more compatible with FlashFXP.
> |- After reviewing the wine source code I believe the problem is

located here
> http://source.winehq.org/source/dlls/comctl32/listview.c line 2971

& 2972
> 2971     for (i = nFirst; i <= nLast; i++)
>    2972         LISTVIEW_SetItemState(infoPtr,i,&item);
>    The Problem: In OwnerData mode the OnData event is triggered when

the
> ItemState changes. However in the OnData event we set the item

state.
> I suspect this causes an infinite loop.

This probably is very close to the actual problem. Looking at current
listview.c
in CVS I see following at about this location:
> 2979     /* FIXME: this is not correct LVS_OWNERDATA
> 2980      * setting the item states individually will generate
> 2981      * a LVN_ITEMCHANGED notification for each one. Instead,
> 2982      * we have to send a LVN_ODSTATECHANGED notification.
> 2983      * See MSDN documentation for LVN_ITEMCHANGED.
> 2984      */
> 2985     for (i = nFirst; i <= nLast; i++)
> 2986         LISTVIEW_SetItemState(infoPtr,i,&item);

Of course there are people here much more familiar with Common Controls
than me, so
I will probably not be able to do a quick fix to this myself. Just
thought I would
point out my findings so that others can confirm or reject the probable
cause of the
endless loop in FlashFXP and other applications.

Rolf Kalbermatter

[Dimitrie O. Paun]

On Sun, Apr 10, 2005 at 09:06:37PM +0200, Rolf Kalbermatter wrote:
> Of course there are people here much more familiar with Common Controls
> than me, so I will probably not be able to do a quick fix to this myself.

Good find! Can you test this patch?

Index: dlls/comctl32/listview.c
===================================================================
RCS file: /var/cvs/wine/dlls/comctl32/listview.c,v
retrieving revision 1.406
diff -u -p -r1.406 listview.c
--- dlls/comctl32/listview.c	25 Mar 2005 20:49:00 -0000	1.406
+++ dlls/comctl32/listview.c	11 Apr 2005 02:37:26 -0000
@@ -2967,22 +2965,35 @@ static void LISTVIEW_AddGroupSelection(L
 {
     INT nFirst = min(infoPtr->nSelectionMark, nItem);
     INT nLast = max(infoPtr->nSelectionMark, nItem);
-    INT i;
+    NMLVODSTATECHANGE nmlv;
     LVITEMW item;
+    BOOL bOldChange;
+    INT i;
+
+    /* Temporarily disable change notification
+     * If the control is LVS_OWNERDATA, we need to send
+     * only one LVN_ODSTATECHANGED notification.
+     * See MSDN documentation for LVN_ITEMCHANGED.
+     */
+    bOldChange = infoPtr->bDoChangeNotify;
+    if (infoPtr->dwStyle & LVS_OWNERDATA) infoPtr->bDoChangeNotify = FALSE;

if (nFirst == -1) nFirst = nItem;

item.state = LVIS_SELECTED;
     item.stateMask = LVIS_SELECTED;

-    /* FIXME: this is not correct LVS_OWNERDATA
-     * setting the item states individually will generate
-     * a LVN_ITEMCHANGED notification for each one. Instead,
-     * we have to send a LVN_ODSTATECHANGED notification.
-     * See MSDN documentation for LVN_ITEMCHANGED.
-     */
     for (i = nFirst; i <= nLast; i++)
    LISTVIEW_SetItemState(infoPtr,i,&item);
+
+    ZeroMemory(&nmlv, sizeof(nmlv));
+    nmlv.iFrom = nFirst;
+    nmlv.iTo = nLast;
+    nmlv.uNewState = 0;
+    nmlv.uOldState = item.state;
+
+    notify_hdr(infoPtr, LVN_ODSTATECHANGED, (LPNMHDR)&nmlv);
+    infoPtr->bDoChangeNotify = bOldChange;
 }