From a9b44af251b8e58cd598335cde85cdb90477a414 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Thu, 25 Jan 2018 13:02:14 +0100 Subject: [PATCH] Fix crash with wxDataViewVirtualListModel in generic wxDataViewCtrl Recent sorting-related changes resulted in calling FindNode(), which can only be used with the non-"virtual list" models, unconditionally, after the items values was changed by user, resulting in a crash. Add the missing IsVirtualList() check and also add a comment explicitly stating that all code involving wxDataViewTreeNode can only be used after checking that IsVirtualList() returns false. Closes #18057. --- src/generic/datavgen.cpp | 30 +++++++++++++++++++----------- 1 file changed, 19 insertions(+), 11 deletions(-) diff --git a/src/generic/datavgen.cpp b/src/generic/datavgen.cpp index c06e72fc11..9a4f08276e 100644 --- a/src/generic/datavgen.cpp +++ b/src/generic/datavgen.cpp @@ -458,6 +458,11 @@ class wxDataViewTreeNode; typedef wxVector wxDataViewTreeNodes; +// Note: this class is not used at all for virtual list models, so all code +// using it, i.e. any functions taking or returning objects of this type, +// including wxDataViewMainWindow::m_root, can only be called after checking +// that we're using a non-"virtual list" model. + class wxDataViewTreeNode { public: @@ -2942,17 +2947,20 @@ bool wxDataViewMainWindow::ItemDeleted(const wxDataViewItem& parent, bool wxDataViewMainWindow::DoItemChanged(const wxDataViewItem & item, int view_column) { - // Move this node to its new correct place after it was updated. - // - // In principle, we could skip the call to PutInSortOrder() if the modified - // column is not the sort column, but in real-world applications it's fully - // possible and likely that custom compare uses not only the selected model - // column but also falls back to other values for comparison. To ensure - // consistency it is better to treat a value change as if it was an item - // change. - wxDataViewTreeNode* const node = FindNode(item); - wxCHECK_MSG( node, false, "invalid item" ); - node->PutInSortOrder(this); + if ( !IsVirtualList() ) + { + // Move this node to its new correct place after it was updated. + // + // In principle, we could skip the call to PutInSortOrder() if the modified + // column is not the sort column, but in real-world applications it's fully + // possible and likely that custom compare uses not only the selected model + // column but also falls back to other values for comparison. To ensure + // consistency it is better to treat a value change as if it was an item + // change. + wxDataViewTreeNode* const node = FindNode(item); + wxCHECK_MSG( node, false, "invalid item" ); + node->PutInSortOrder(this); + } wxDataViewColumn* column; if ( view_column == wxNOT_FOUND )