Don't trigger kill focus event twice

[wxWidgets.git] / src / msw / treectrl.cpp

diff --git a/src/msw/treectrl.cpp b/src/msw/treectrl.cpp
index 8b3b1a2f57b4c8dd5336c6ca18b720a6eceed2ac..3093583d52ad26a2a4852699c2211ab5fc47f431 100644 (file)
--- a/src/msw/treectrl.cpp
+++ b/src/msw/treectrl.cpp
@@ -52,6 +52,59 @@
 // get HTREEITEM from wxTreeItemId
 #define HITEM(item)     ((HTREEITEM)(((item).m_pItem)))
 
 // get HTREEITEM from wxTreeItemId
 #define HITEM(item)     ((HTREEITEM)(((item).m_pItem)))
 

+
+// older SDKs are missing these
+#ifndef TVN_ITEMCHANGINGA
+
+#define TVN_ITEMCHANGINGA (TVN_FIRST-16)
+#define TVN_ITEMCHANGINGW (TVN_FIRST-17)
+
+typedef struct tagNMTVITEMCHANGE
+{
+    NMHDR hdr;
+    UINT uChanged;
+    HTREEITEM hItem;
+    UINT uStateNew;
+    UINT uStateOld;
+    LPARAM lParam;
+} NMTVITEMCHANGE;
+
+#endif
+
+
+// this helper class is used on vista systems for preventing unwanted
+// item state changes in the vista tree control.  It is only effective in
+// multi-select mode on vista systems.
+
+// The vista tree control includes some new code that originally broke the
+// multi-selection tree, causing seemingly spurious item selection state changes
+// during Shift or Ctrl-click item selection. (To witness the original broken
+// behavior, simply make IsLocked() below always return false). This problem was
+// solved by using the following class to 'unlock' an item's selection state.
+
+class TreeItemUnlocker
+{
+public:
+    // unlock a single item
+    TreeItemUnlocker(HTREEITEM item) { ms_unlockedItem = item; }
+
+    // unlock all items, don't use unless absolutely necessary
+    TreeItemUnlocker() { ms_unlockedItem = (HTREEITEM)-1; }
+
+    // lock everything back
+    ~TreeItemUnlocker() { ms_unlockedItem = NULL; }
+
+
+    // check if the item state is currently locked
+    static bool IsLocked(HTREEITEM item)
+        { return ms_unlockedItem != (HTREEITEM)-1 && item != ms_unlockedItem; }
+
+private:
+    static HTREEITEM ms_unlockedItem;
+};
+
+HTREEITEM TreeItemUnlocker::ms_unlockedItem = NULL;
+

 // ----------------------------------------------------------------------------
 // private functions
 // ----------------------------------------------------------------------------
 // ----------------------------------------------------------------------------
 // private functions
 // ----------------------------------------------------------------------------


@@ -65,6 +118,8 @@ static bool IsItemSelected(HWND hwndTV, HTREEITEM hItem)
     tvi.stateMask = TVIS_SELECTED;
     tvi.hItem = hItem;
 
     tvi.stateMask = TVIS_SELECTED;
     tvi.hItem = hItem;
 

+    TreeItemUnlocker unlocker(hItem);
+

     if ( !TreeView_GetItem(hwndTV, &tvi) )
     {
         wxLogLastError(wxT("TreeView_GetItem"));
     if ( !TreeView_GetItem(hwndTV, &tvi) )
     {
         wxLogLastError(wxT("TreeView_GetItem"));


@@ -81,6 +136,8 @@ static bool SelectItem(HWND hwndTV, HTREEITEM hItem, bool select = true)
     tvi.state = select ? TVIS_SELECTED : 0;
     tvi.hItem = hItem;
 
     tvi.state = select ? TVIS_SELECTED : 0;
     tvi.hItem = hItem;
 

+    TreeItemUnlocker unlocker(hItem);
+

     if ( TreeView_SetItem(hwndTV, &tvi) == -1 )
     {
         wxLogLastError(wxT("TreeView_SetItem"));
     if ( TreeView_SetItem(hwndTV, &tvi) == -1 )
     {
         wxLogLastError(wxT("TreeView_SetItem"));


@@ -804,6 +861,8 @@ bool wxTreeCtrl::DoGetItem(wxTreeViewItem *tvItem) const
 
 void wxTreeCtrl::DoSetItem(wxTreeViewItem *tvItem)
 {
 
 void wxTreeCtrl::DoSetItem(wxTreeViewItem *tvItem)
 {

+    TreeItemUnlocker unlocker(tvItem->hItem);
+

     if ( TreeView_SetItem(GetHwnd(), tvItem) == -1 )
     {
         wxLogLastError(wxT("TreeView_SetItem"));
     if ( TreeView_SetItem(GetHwnd(), tvItem) == -1 )
     {
         wxLogLastError(wxT("TreeView_SetItem"));


@@ -934,7 +993,7 @@ void wxTreeCtrl::SetItemText(const wxTreeItemId& item, const wxString& text)
     {
         if ( item == m_idEdited )
         {
     {
         if ( item == m_idEdited )
         {

-            ::SetWindowText(hwndEdit, text);
+            ::SetWindowText(hwndEdit, text.wx_str());

         }
     }
 }
         }
     }
 }


@@ -1354,7 +1413,15 @@ wxTreeItemId wxTreeCtrl::GetNextVisible(const wxTreeItemId& item) const
     wxCHECK_MSG( item.IsOk(), wxTreeItemId(), wxT("invalid tree item") );
     wxASSERT_MSG( IsVisible(item), wxT("The item you call GetNextVisible() for must be visible itself!"));
 
     wxCHECK_MSG( item.IsOk(), wxTreeItemId(), wxT("invalid tree item") );
     wxASSERT_MSG( IsVisible(item), wxT("The item you call GetNextVisible() for must be visible itself!"));
 

-    return wxTreeItemId(TreeView_GetNextVisible(GetHwnd(), HITEM(item)));
+    wxTreeItemId next(TreeView_GetNextVisible(GetHwnd(), HITEM(item)));
+    if ( next.IsOk() && !IsVisible(next) )
+    {
+        // Win32 considers that any non-collapsed item is visible while we want
+        // to return only really visible items
+        next.Unset();
+    }
+
+    return next;

 }
 
 wxTreeItemId wxTreeCtrl::GetPrevVisible(const wxTreeItemId& item) const
 }
 
 wxTreeItemId wxTreeCtrl::GetPrevVisible(const wxTreeItemId& item) const


@@ -1362,7 +1429,15 @@ wxTreeItemId wxTreeCtrl::GetPrevVisible(const wxTreeItemId& item) const
     wxCHECK_MSG( item.IsOk(), wxTreeItemId(), wxT("invalid tree item") );
     wxASSERT_MSG( IsVisible(item), wxT("The item you call GetPrevVisible() for must be visible itself!"));
 
     wxCHECK_MSG( item.IsOk(), wxTreeItemId(), wxT("invalid tree item") );
     wxASSERT_MSG( IsVisible(item), wxT("The item you call GetPrevVisible() for must be visible itself!"));
 

-    return wxTreeItemId(TreeView_GetPrevVisible(GetHwnd(), HITEM(item)));
+    wxTreeItemId prev(TreeView_GetPrevVisible(GetHwnd(), HITEM(item)));
+    if ( prev.IsOk() && !IsVisible(prev) )
+    {
+        // just as above, Win32 function will happily return the previous item
+        // in the tree for the first visible item too
+        prev.Unset();
+    }
+
+    return prev;

 }
 
 // ----------------------------------------------------------------------------
 }
 
 // ----------------------------------------------------------------------------


@@ -1456,12 +1531,27 @@ wxTreeItemId wxTreeCtrl::DoInsertAfter(const wxTreeItemId& parent,
     tvIns.item.lParam = (LPARAM)param;
     tvIns.item.mask = mask;
 
     tvIns.item.lParam = (LPARAM)param;
     tvIns.item.mask = mask;
 

+    // don't use the hack below for the children of hidden root: this results
+    // in a crash inside comctl32.dll when we call TreeView_GetItemRect()
+    const bool firstChild = !IsHiddenRoot(parent) &&
+                                !TreeView_GetChild(GetHwnd(), HITEM(parent));
+

     HTREEITEM id = TreeView_InsertItem(GetHwnd(), &tvIns);
     if ( id == 0 )
     {
         wxLogLastError(wxT("TreeView_InsertItem"));
     }
 
     HTREEITEM id = TreeView_InsertItem(GetHwnd(), &tvIns);
     if ( id == 0 )
     {
         wxLogLastError(wxT("TreeView_InsertItem"));
     }
 

+    // apparently some Windows versions (2000 and XP are reported to do this)
+    // sometimes don't refresh the tree after adding the first child and so we
+    // need this to make the "[+]" appear
+    if ( firstChild )
+    {
+        RECT rect;
+        TreeView_GetItemRect(GetHwnd(), HITEM(parent), &rect, FALSE);
+        ::InvalidateRect(GetHwnd(), &rect, FALSE);
+    }
+

     // associate the application tree item with Win32 tree item handle
     param->SetItem(id);
 
     // associate the application tree item with Win32 tree item handle
     param->SetItem(id);
 


@@ -1530,6 +1620,10 @@ wxTreeItemId wxTreeCtrl::DoInsertItem(const wxTreeItemId& parent,
 
 void wxTreeCtrl::Delete(const wxTreeItemId& item)
 {
 
 void wxTreeCtrl::Delete(const wxTreeItemId& item)
 {

+    // unlock tree selections on vista, without this the
+    // tree ctrl will eventually crash after item deletion
+    TreeItemUnlocker unlock_all;
+

     if ( !TreeView_DeleteItem(GetHwnd(), HITEM(item)) )
     {
         wxLogLastError(wxT("TreeView_DeleteItem"));
     if ( !TreeView_DeleteItem(GetHwnd(), HITEM(item)) )
     {
         wxLogLastError(wxT("TreeView_DeleteItem"));


@@ -1539,6 +1633,9 @@ void wxTreeCtrl::Delete(const wxTreeItemId& item)
 // delete all children (but don't delete the item itself)
 void wxTreeCtrl::DeleteChildren(const wxTreeItemId& item)
 {
 // delete all children (but don't delete the item itself)
 void wxTreeCtrl::DeleteChildren(const wxTreeItemId& item)
 {

+    // unlock tree selections on vista for the duration of this call
+    TreeItemUnlocker unlock_all;
+

     wxTreeItemIdValue cookie;
 
     wxArrayTreeItemIds children;
     wxTreeItemIdValue cookie;
 
     wxArrayTreeItemIds children;


@@ -1562,6 +1659,9 @@ void wxTreeCtrl::DeleteChildren(const wxTreeItemId& item)
 
 void wxTreeCtrl::DeleteAllItems()
 {
 
 void wxTreeCtrl::DeleteAllItems()
 {

+    // unlock tree selections on vista for the duration of this call
+    TreeItemUnlocker unlock_all;
+

     // delete the "virtual" root item.
     if ( GET_VIRTUAL_ROOT() )
     {
     // delete the "virtual" root item.
     if ( GET_VIRTUAL_ROOT() )
     {


@@ -1609,7 +1709,7 @@ void wxTreeCtrl::DoExpand(const wxTreeItemId& item, int flag)
                                                            : IDX_COLLAPSE]
                                          [IDX_DONE],
                            this, item);
                                                            : IDX_COLLAPSE]
                                          [IDX_DONE],
                            this, item);

-        (void)GetEventHandler()->ProcessEvent(event);
+        (void)HandleWindowEvent(event);

     }
     //else: change didn't took place, so do nothing at all
 }
     }
     //else: change didn't took place, so do nothing at all
 }


@@ -1671,7 +1771,7 @@ void wxTreeCtrl::SelectItem(const wxTreeItemId& item, bool select)
                   _T("SelectItem(false) works only for multiselect") );
 
     wxTreeEvent event(wxEVT_COMMAND_TREE_SEL_CHANGING, this, item);
                   _T("SelectItem(false) works only for multiselect") );
 
     wxTreeEvent event(wxEVT_COMMAND_TREE_SEL_CHANGING, this, item);

-    if ( !GetEventHandler()->ProcessEvent(event) || event.IsAllowed() )
+    if ( !HandleWindowEvent(event) || event.IsAllowed() )

     {
         if ( HasFlag(wxTR_MULTIPLE) )
         {
     {
         if ( HasFlag(wxTR_MULTIPLE) )
         {


@@ -1692,7 +1792,7 @@ void wxTreeCtrl::SelectItem(const wxTreeItemId& item, bool select)
         }
 
         event.SetEventType(wxEVT_COMMAND_TREE_SEL_CHANGED);
         }
 
         event.SetEventType(wxEVT_COMMAND_TREE_SEL_CHANGED);

-        (void)GetEventHandler()->ProcessEvent(event);
+        (void)HandleWindowEvent(event);

     }
     //else: program vetoed the change
 }
     }
     //else: program vetoed the change
 }


@@ -1892,7 +1992,9 @@ bool wxTreeCtrl::MSWShouldPreProcessMessage(WXMSG* msg)
 {
     if ( msg->message == WM_KEYDOWN )
     {
 {
     if ( msg->message == WM_KEYDOWN )
     {

-        if ( msg->wParam == VK_RETURN )
+        // Only eat VK_RETURN if not being used by the application in
+        // conjunction with modifiers
+        if ( (msg->wParam == VK_RETURN) && !wxIsAnyModifierDown() )

         {
             // we need VK_RETURN to generate wxEVT_COMMAND_TREE_ITEM_ACTIVATED
             return false;
         {
             // we need VK_RETURN to generate wxEVT_COMMAND_TREE_ITEM_ACTIVATED
             return false;


@@ -1902,8 +2004,10 @@ bool wxTreeCtrl::MSWShouldPreProcessMessage(WXMSG* msg)
     return wxTreeCtrlBase::MSWShouldPreProcessMessage(msg);
 }
 
     return wxTreeCtrlBase::MSWShouldPreProcessMessage(msg);
 }
 

-bool wxTreeCtrl::MSWCommand(WXUINT cmd, WXWORD id)
+bool wxTreeCtrl::MSWCommand(WXUINT cmd, WXWORD id_)

 {
 {

+    const int id = (signed short)id_;
+

     if ( cmd == EN_UPDATE )
     {
         wxCommandEvent event(wxEVT_COMMAND_TEXT_UPDATED, id);
     if ( cmd == EN_UPDATE )
     {
         wxCommandEvent event(wxEVT_COMMAND_TEXT_UPDATED, id);


@@ -1981,7 +2085,7 @@ WXLRESULT wxTreeCtrl::MSWWindowProc(WXUINT nMsg, WXWPARAM wParam, WXLPARAM lPara
 
         event.m_pointDrag = pt;
 
 
         event.m_pointDrag = pt;
 

-        if ( GetEventHandler()->ProcessEvent(event) )
+        if ( HandleWindowEvent(event) )

             processed = true;
         //else: continue with generating wxEVT_CONTEXT_MENU in base class code
     }
             processed = true;
         //else: continue with generating wxEVT_CONTEXT_MENU in base class code
     }


@@ -2089,6 +2193,38 @@ WXLRESULT wxTreeCtrl::MSWWindowProc(WXUINT nMsg, WXWPARAM wParam, WXLPARAM lPara
                 }
                 break;
 
                 }
                 break;
 

+            case WM_RBUTTONDOWN:
+                // default handler removes the highlight from the currently
+                // focused item when right mouse button is pressed on another
+                // one but keeps the remaining items highlighted, which is
+                // confusing, so override this default behaviour for tree with
+                // multiple selections
+                if ( isMultiple )
+                {
+                    if ( !IsItemSelected(GetHwnd(), htItem) )
+                    {
+                        UnselectAll();
+                        SelectItem(htItem);
+                        ::SetFocus(GetHwnd(), htItem);
+                    }
+
+                    // fire EVT_RIGHT_DOWN
+                    HandleMouseEvent(nMsg, x, y, wParam);
+
+                    // send NM_RCLICK
+                    NMHDR nmhdr;
+                    nmhdr.hwndFrom = GetHwnd();
+                    nmhdr.idFrom = ::GetWindowLong(GetHwnd(), GWL_ID);
+                    nmhdr.code = NM_RCLICK;
+                    ::SendMessage(::GetParent(GetHwnd()), WM_NOTIFY,
+                                  nmhdr.idFrom, (LPARAM)&nmhdr);
+
+                    // prevent tree control default processing, as we've
+                    // already done everything
+                    processed = true;
+                }
+                break;
+

             case WM_MOUSEMOVE:
 #ifndef __WXWINCE__
                 if ( m_htClickedItem )
             case WM_MOUSEMOVE:
 #ifndef __WXWINCE__
                 if ( m_htClickedItem )


@@ -2188,7 +2324,7 @@ WXLRESULT wxTreeCtrl::MSWWindowProc(WXUINT nMsg, WXWPARAM wParam, WXLPARAM lPara
                     wxTreeEvent event(wxEVT_COMMAND_TREE_END_DRAG, this, htItem);
                     event.m_pointDrag = wxPoint(x, y);
 
                     wxTreeEvent event(wxEVT_COMMAND_TREE_END_DRAG, this, htItem);
                     event.m_pointDrag = wxPoint(x, y);
 

-                    (void)GetEventHandler()->ProcessEvent(event);
+                    (void)HandleWindowEvent(event);

 
                     // if we don't do it, the tree seems to think that 2 items
                     // are selected simultaneously which is quite weird
 
                     // if we don't do it, the tree seems to think that 2 items
                     // are selected simultaneously which is quite weird


@@ -2335,6 +2471,24 @@ wxTreeCtrl::MSWDefWindowProc(WXUINT nMsg, WXWPARAM wParam, WXLPARAM lParam)
         if ( wParam == VK_SPACE || wParam == VK_RETURN )
             return 0;
     }
         if ( wParam == VK_SPACE || wParam == VK_RETURN )
             return 0;
     }

+#if wxUSE_DRAGIMAGE
+    else if ( nMsg == WM_KEYDOWN )
+    {
+        if ( wParam == VK_ESCAPE )
+        {
+            if ( m_dragImage )
+            {
+                m_dragImage->EndDrag();
+                delete m_dragImage;
+                m_dragImage = NULL;
+
+                // if we don't do it, the tree seems to think that 2 items
+                // are selected simultaneously which is quite weird
+                TreeView_SelectDropTarget(GetHwnd(), 0);
+            }
+        }
+    }
+#endif // wxUSE_DRAGIMAGE

 
     return wxControl::MSWDefWindowProc(nMsg, wParam, lParam);
 }
 
     return wxControl::MSWDefWindowProc(nMsg, wParam, lParam);
 }


@@ -2495,8 +2649,7 @@ bool wxTreeCtrl::MSWOnNotify(int idCtrl, WXLPARAM lParam, WXLPARAM *result)
                 // fabricate the lParam and wParam parameters sufficiently
                 // similar to the ones from a "real" WM_KEYDOWN so that
                 // CreateKeyEvent() works correctly
                 // fabricate the lParam and wParam parameters sufficiently
                 // similar to the ones from a "real" WM_KEYDOWN so that
                 // CreateKeyEvent() works correctly

-                const bool isAltDown = ::GetKeyState(VK_MENU) < 0;
-                WXLPARAM lParam = (isAltDown ? KF_ALTDOWN : 0) << 16;
+                WXLPARAM lParam = (wxIsAltDown() ? KF_ALTDOWN : 0) << 16;

 
                 WXWPARAM wParam = info->wVKey;
 
 
                 WXWPARAM wParam = info->wVKey;
 


@@ -2514,7 +2667,7 @@ bool wxTreeCtrl::MSWOnNotify(int idCtrl, WXLPARAM lParam, WXLPARAM *result)
                                                 wParam);
 
                 // a separate event for Space/Return
                                                 wParam);
 
                 // a separate event for Space/Return

-                if ( !wxIsCtrlDown() && !wxIsShiftDown() && !isAltDown &&
+                if ( !wxIsAnyModifierDown() &&

                      ((info->wVKey == VK_SPACE) || (info->wVKey == VK_RETURN)) )
                 {
                    wxTreeItemId item;
                      ((info->wVKey == VK_SPACE) || (info->wVKey == VK_RETURN)) )
                 {
                    wxTreeItemId item;


@@ -2523,11 +2676,42 @@ bool wxTreeCtrl::MSWOnNotify(int idCtrl, WXLPARAM lParam, WXLPARAM *result)
 
                    wxTreeEvent event2(wxEVT_COMMAND_TREE_ITEM_ACTIVATED,
                                         this, item);
 
                    wxTreeEvent event2(wxEVT_COMMAND_TREE_ITEM_ACTIVATED,
                                         this, item);

-                   (void)GetEventHandler()->ProcessEvent(event2);
+                   (void)HandleWindowEvent(event2);

                 }
             }
             break;
 
                 }
             }
             break;
 

+
+        // Vista's tree control has introduced some problems with our
+        // multi-selection tree.  When TreeView_SelectItem() is called,
+        // the wrong items are deselected.
+
+        // Fortunately, Vista provides a new notification, TVN_ITEMCHANGING
+        // that can be used to regulate this incorrect behavior.  The
+        // following messages will allow only the unlocked item's selection
+        // state to change
+
+        case TVN_ITEMCHANGINGA:
+        case TVN_ITEMCHANGINGW:
+            {
+                // we only need to handles these in multi-select trees
+                if ( HasFlag(wxTR_MULTIPLE) )
+                {
+                    // get info about the item about to be changed
+                    NMTVITEMCHANGE* info = (NMTVITEMCHANGE*)lParam;
+                    if (TreeItemUnlocker::IsLocked(info->hItem))
+                    {
+                        // item's state is locked, don't allow the change
+                        // returning 1 will disallow the change
+                        *result = 1;
+                        return true;
+                    }
+                }
+
+                // allow the state change
+            }
+            return false;
+

         // NB: MSLU is broken and sends TVN_SELCHANGEDA instead of
         //     TVN_SELCHANGEDW in Unicode mode under Win98. Therefore
         //     we have to handle both messages:
         // NB: MSLU is broken and sends TVN_SELCHANGEDA instead of
         //     TVN_SELCHANGEDW in Unicode mode under Win98. Therefore
         //     we have to handle both messages:


@@ -2557,6 +2741,19 @@ bool wxTreeCtrl::MSWOnNotify(int idCtrl, WXLPARAM lParam, WXLPARAM *result)
                     event.m_itemOld = tv->itemOld.hItem;
                 }
             }
                     event.m_itemOld = tv->itemOld.hItem;
                 }
             }

+
+            // we receive this message from WM_LBUTTONDOWN handler inside
+            // comctl32.dll and so before the click is passed to
+            // DefWindowProc() which sets the focus to the window which was
+            // clicked and this can lead to unexpected event sequences: for
+            // example, we may get a "selection change" event from the tree
+            // before getting a "kill focus" event for the text control which
+            // had the focus previously, thus breaking user code doing input
+            // validation
+            //
+            // to avoid such surprises, we force the generation of focus events
+            // now, before we generate the selection change ones
+            SetFocus();

             break;
 
             // instead of explicitly checking for _WIN32_IE, check if the
             break;
 
             // instead of explicitly checking for _WIN32_IE, check if the


@@ -2697,7 +2894,7 @@ bool wxTreeCtrl::MSWOnNotify(int idCtrl, WXLPARAM lParam, WXLPARAM *result)
     if ( event.m_item.IsOk() )
         event.SetClientObject(GetItemData(event.m_item));
 
     if ( event.m_item.IsOk() )
         event.SetClientObject(GetItemData(event.m_item));
 

-    bool processed = GetEventHandler()->ProcessEvent(event);
+    bool processed = HandleWindowEvent(event);

 
     // post processing
     switch ( hdr->code )
 
     // post processing
     switch ( hdr->code )


@@ -2823,17 +3020,40 @@ bool wxTreeCtrl::MSWOnNotify(int idCtrl, WXLPARAM lParam, WXLPARAM *result)
             break;
 
         case TVN_ITEMEXPANDED:
             break;
 
         case TVN_ITEMEXPANDED:

-            // the item is not refreshed properly after expansion when it has
-            // an image depending on the expanded/collapsed state - bug in
-            // comctl32.dll or our code?

             {
                 NM_TREEVIEW *tv = (NM_TREEVIEW *)lParam;
             {
                 NM_TREEVIEW *tv = (NM_TREEVIEW *)lParam;

-                wxTreeItemId id(tv->itemNew.hItem);
+                const wxTreeItemId id(tv->itemNew.hItem);

 
 

-                int image = GetItemImage(id, wxTreeItemIcon_Expanded);
-                if ( image != -1 )
+                if ( tv->action == TVE_COLLAPSE )
+                {
+                    if ( wxApp::GetComCtl32Version() >= 600 )
+                    {
+                        // for some reason the item selection rectangle depends
+                        // on whether it is expanded or collapsed (at least
+                        // with comctl32.dll v6): it is wider (by 3 pixels) in
+                        // the expanded state, so when the item collapses and
+                        // then is deselected the rightmost 3 pixels of the
+                        // previously drawn selection are left on the screen
+                        //
+                        // it's not clear if it's a bug in comctl32.dll or in
+                        // our code (because it does not happen in Explorer but
+                        // OTOH we don't do anything which could result in this
+                        // AFAICS) but we do need to work around it to avoid
+                        // ugly artifacts
+                        RefreshItem(id);
+                    }
+                }
+                else // expand

                 {
                 {

-                    RefreshItem(id);
+                    // the item is also not refreshed properly after expansion when
+                    // it has an image depending on the expanded/collapsed state:
+                    // again, it's not clear if the bug is in comctl32.dll or our
+                    // code...
+                    int image = GetItemImage(id, wxTreeItemIcon_Expanded);
+                    if ( image != -1 )
+                    {
+                        RefreshItem(id);
+                    }

                 }
             }
             break;
                 }
             }
             break;