From: Robin Dunn Date: Thu, 30 Jan 2003 01:53:28 +0000 (+0000) Subject: 1. Added m_count and maintain its value in InsertItem, RemoveItem X-Git-Url: https://git.saurik.com/wxWidgets.git/commitdiff_plain/68c124a1685a951ff9375d23fcc9ba5a6befc31f 1. Added m_count and maintain its value in InsertItem, RemoveItem etc. and return it from GetItemCount to save so many calls to ListView_GetItemCount. 2. If EVT_LIST_DELETE_ALL_ITEMS was not handled then the post-processing code to prevent LVN_DELETEITEM messages was not getting executed. Changed it so the post processing always happens. 3. It's possible with some versions of comctl32 that garbage values can be passed in nmLV->lParam so don't try to get the internal item data pointer from it. This should close bug# 659939 4. Added FreeAllInternalData from DeleteAllItems, (part of patch #672065) and also guard against processing LVN_CHANG* messages while freeing the internal data. git-svn-id: https://svn.wxwidgets.org/svn/wx/wxWidgets/trunk@19021 c3d73ce0-8a6f-49c7-b76d-6d57e0e08775 --- diff --git a/include/wx/msw/listctrl.h b/include/wx/msw/listctrl.h index 5de83175f9..eaf82c0cd6 100644 --- a/include/wx/msw/listctrl.h +++ b/include/wx/msw/listctrl.h @@ -370,6 +370,9 @@ protected: long m_baseStyle; // Basic Windows style flags, for recreation purposes int m_colCount; // Windows doesn't have GetColumnCount so must // keep track of inserted/deleted columns + long m_count; // Keep track of item count to save calls to + // ListView_GetItemCount + bool m_ignoreChangeMessages; // TRUE if we have any internal data (user data & attributes) bool m_AnyInternalData; diff --git a/src/msw/listctrl.cpp b/src/msw/listctrl.cpp index 943a87ae8d..a2771749ed 100644 --- a/src/msw/listctrl.cpp +++ b/src/msw/listctrl.cpp @@ -244,6 +244,8 @@ void wxListCtrl::Init() m_ownsImageListNormal = m_ownsImageListSmall = m_ownsImageListState = FALSE; m_baseStyle = 0; m_colCount = 0; + m_count = 0; + m_ignoreChangeMessages = FALSE; m_textCtrl = NULL; m_AnyInternalData = FALSE; m_hasAnyAttr = FALSE; @@ -378,8 +380,10 @@ void wxListCtrl::FreeAllInternalData() int n = GetItemCount(); int i = 0; + m_ignoreChangeMessages = TRUE; for (i = 0; i < n; i++) wxDeleteInternalData(this, i); + m_ignoreChangeMessages = TRUE; m_AnyInternalData = FALSE; } @@ -1014,7 +1018,7 @@ bool wxListCtrl::SetItemPosition(long item, const wxPoint& pos) // Gets the number of items in the list control int wxListCtrl::GetItemCount() const { - return ListView_GetItemCount(GetHwnd()); + return m_count; } // Retrieves the spacing between icons in pixels. @@ -1204,6 +1208,10 @@ bool wxListCtrl::DeleteItem(long item) return FALSE; } + m_count -= 1; + wxASSERT_MSG( m_count == ListView_GetItemCount(GetHwnd()), + wxT("m_count should match ListView_GetItemCount")); + // the virtual list control doesn't refresh itself correctly, help it if ( IsVirtual() ) { @@ -1232,6 +1240,7 @@ bool wxListCtrl::DeleteItem(long item) // Deletes all items bool wxListCtrl::DeleteAllItems() { + FreeAllInternalData(); return ListView_DeleteAllItems(GetHwnd()) != 0; } @@ -1448,8 +1457,12 @@ long wxListCtrl::InsertItem(wxListItem& info) } }; + long rv = ListView_InsertItem(GetHwnd(), & item); + m_count += 1; + wxASSERT_MSG( m_count == ListView_GetItemCount(GetHwnd()), + wxT("m_count should match ListView_GetItemCount")); - return (long) ListView_InsertItem(GetHwnd(), & item); + return rv; } long wxListCtrl::InsertItem(long index, const wxString& label) @@ -1641,6 +1654,7 @@ bool wxListCtrl::MSWCommand(WXUINT cmd, WXWORD id) bool wxListCtrl::MSWOnNotify(int idCtrl, WXLPARAM lParam, WXLPARAM *result) { + // prepare the event // ----------------- @@ -1770,53 +1784,29 @@ bool wxListCtrl::MSWOnNotify(int idCtrl, WXLPARAM lParam, WXLPARAM *result) const int iItem = nmLV->iItem; - // set the data event field for all messages for which the system gives - // us a valid NM_LISTVIEW::lParam - switch ( nmLV->hdr.code ) + + // FreeAllInternalData will cause LVN_ITEMCHANG* messages, which can be + // ignored for efficiency. It is done here because the internal data is in the + // process of being deleted so we don't want to try and access it below. + if ( m_ignoreChangeMessages && + ( (nmLV->hdr.code == LVN_ITEMCHANGED) || (nmLV->hdr.code == LVN_ITEMCHANGING))) { - case LVN_BEGINDRAG: - case LVN_BEGINRDRAG: - case LVN_COLUMNCLICK: - case LVN_ITEMCHANGED: - case LVN_ITEMCHANGING: - if ( iItem != -1 ) - { - if ( iItem >= GetItemCount() ) - { - // there is apparently a bug in comctl32.dll version - // 5.50.4704.1100 (note that the MS DLL database - // doesn't say what this version is, it's not the one - // shipped with W2K although the bug was reported under - // that system) and it sends us LVN_ITEMCHANGING - // notifications with the item out of range -- and as - // we access the items client data, we crash below - // - // see - // - // http://lists.wxwindows.org/cgi-bin/ezmlm-cgi?8:mss:29852:knlihdmadhaljafjajei - // - // and the thread continuation for more details - // (although note that the bug may be present in other - // versions of comctl32.dll as well as it has been - // reported quite a few times) - // - // to fix this, simply ignore these "bad" events (as - // above with HDN_GETDISPINFOW) - return TRUE; - } + return TRUE; + } - wxListItemInternalData *internaldata = - (wxListItemInternalData *) nmLV->lParam; - if ( internaldata ) - event.m_item.m_data = internaldata->lParam; - } + // If we have a valid item then check if there is a data value + // associated with it and put it in the event. + if ( iItem >= 0 && iItem < GetItemCount() ) + { + wxListItemInternalData *internaldata = + wxGetInternalData(GetHwnd(), iItem); - default: - // fall through - ; + if ( internaldata ) + event.m_item.m_data = internaldata->lParam; } + switch ( nmhdr->code ) { case LVN_BEGINRDRAG: @@ -1885,11 +1875,17 @@ bool wxListCtrl::MSWOnNotify(int idCtrl, WXLPARAM lParam, WXLPARAM *result) break; case LVN_DELETEALLITEMS: + m_count = 0; eventType = wxEVT_COMMAND_LIST_DELETE_ALL_ITEMS; event.m_itemIndex = -1; break; case LVN_DELETEITEM: + if (m_count == 0) + // this should be prevented by the post-processing code below, + // but "just in case" + return FALSE; + eventType = wxEVT_COMMAND_LIST_DELETE_ITEM; event.m_itemIndex = iItem; // delete the assoicated internal data @@ -2133,12 +2129,10 @@ bool wxListCtrl::MSWOnNotify(int idCtrl, WXLPARAM lParam, WXLPARAM *result) event.SetEventType(eventType); - if ( !GetEventHandler()->ProcessEvent(event) ) - return FALSE; + bool processed = GetEventHandler()->ProcessEvent(event); // post processing // --------------- - switch ( nmhdr->code ) { case LVN_DELETEALLITEMS: @@ -2146,7 +2140,6 @@ bool wxListCtrl::MSWOnNotify(int idCtrl, WXLPARAM lParam, WXLPARAM *result) // notifications - this makes deleting all items from a list ctrl // much faster *result = TRUE; - return TRUE; case LVN_ENDLABELEDITA: @@ -2168,9 +2161,10 @@ bool wxListCtrl::MSWOnNotify(int idCtrl, WXLPARAM lParam, WXLPARAM *result) return TRUE; } - *result = !event.IsAllowed(); + if ( processed ) + *result = !event.IsAllowed(); - return TRUE; + return processed; } #if defined(_WIN32_IE) && _WIN32_IE >= 0x300 @@ -2370,6 +2364,9 @@ void wxListCtrl::SetItemCount(long count) { wxLogLastError(_T("ListView_SetItemCount")); } + m_count = count; + wxASSERT_MSG( m_count == ListView_GetItemCount(GetHwnd()), + wxT("m_count should match ListView_GetItemCount")); } void wxListCtrl::RefreshItem(long item)