Fix regression in selection change in wxOSX wxDataViewCtrl

The change of 8aae7ad937 (Fatal exception fixed in DVC on macOS while
wxDataViewModel::Cleared call + editing item., 2022-01-17) broke change
of selection when deleting an item from wxDataViewCtrl and the unit test
checking for this but, somehow, only when it was merged into the latest
master and not when it was originally done.

It's not really clear how did it work before, but fix this now by
distinguishing between just deleting some items and clearing everything
and only returning nil from -[wxCocoaOutlineDataSource
outlineView🧒ofItem:] in the latter case, but not the former one.

Also replace wxDataViewCtrl::m_Deleting boolean with an (opaque)
pointer, so that we could improve this further in the future without
breaking the ABI.

See #22025.
This commit is contained in:
Vadim Zeitlin
2022-02-04 17:12:40 +01:00
parent dbc8506883
commit ff8e60caea
3 changed files with 46 additions and 19 deletions

View File

@@ -230,11 +230,9 @@ public:
return m_CustomRendererPtr;
}
// checks if currently a delete process is running
bool IsDeleting() const
{
return m_Deleting;
}
// checks if a single item or all items are being deleted
bool IsDeleting() const;
bool IsClearing() const;
// with CG, we need to get the context from an kEventControlDraw event
// unfortunately, the DataBrowser callbacks don't provide the context
@@ -296,9 +294,11 @@ private:
//
// variables
//
bool m_Deleting; // flag indicating if a delete process is running; this flag is necessary because the notifier indicating an item deletion in the model may be called
// after the actual deletion of the item; then, native callback functions/delegates may try to update data of variables that are already deleted;
// if this flag is set all native variable update requests will be ignored
// If non-null, describes the item(s) being deleted. This is necessary to
// allow avoiding referencing already deleted items from the native
// callbacks/delegates.
struct wxOSXDVCDeleting* m_Deleting;
// This class can set (and reset) m_Deleting.
friend class wxOSXDVCScopedDeleter;

View File

@@ -551,9 +551,12 @@ outlineView:(NSOutlineView*)outlineView
{
wxUnusedVar(outlineView);
// See the comment in outlineView:objectValueForTableColumn:byItem: below:
// this function can also be called in the same circumstances.
if ( implementation->GetDataViewCtrl()->IsDeleting() )
// This function can be called when we're just deleting some item, but in
// this case it's being called for a different item (the one to which the
// selection changes if the previously selected item was deleted), so we
// can still use it, however we still can't use the item if everything is
// being deleted.
if ( implementation->GetDataViewCtrl()->IsClearing() )
return nil;
if ((item == currentParentItem) &&

View File

@@ -50,28 +50,40 @@ wxString ConcatenateDataViewItemValues(wxDataViewCtrl const* dataViewCtrlPtr, wx
}
// ============================================================================
// wxOSXDVCScopedDeleter
// wxOSXDVCDeleting and wxOSXDVCScopedDeleter
// ============================================================================
struct wxOSXDVCDeleting
{
explicit wxOSXDVCDeleting(const wxDataViewItem& parent) : m_parent(parent)
{
}
const wxDataViewItem m_parent;
wxDECLARE_NO_COPY_CLASS(wxOSXDVCDeleting);
};
// Helper class which exists only to reset m_Deleting on scope exit.
class wxOSXDVCScopedDeleter
{
public:
explicit wxOSXDVCScopedDeleter(wxDataViewCtrl* dvc) :
wxOSXDVCScopedDeleter(wxDataViewCtrl* dvc, const wxDataViewItem& parent) :
m_dvc(dvc),
m_valueOrig(m_dvc->m_Deleting)
{
m_dvc->m_Deleting = true;
m_dvc->m_Deleting = new wxOSXDVCDeleting(parent);
}
~wxOSXDVCScopedDeleter()
{
delete m_dvc->m_Deleting;
m_dvc->m_Deleting = m_valueOrig;
}
private:
wxDataViewCtrl* const m_dvc;
const bool m_valueOrig;
wxOSXDVCDeleting* const m_valueOrig;
wxDECLARE_NO_COPY_CLASS(wxOSXDVCScopedDeleter);
};
@@ -224,7 +236,7 @@ bool wxOSXDataViewModelNotifier::ItemDeleted(wxDataViewItem const& parent, wxDat
// not to be identical because the being edited item might be below the passed item in the hierarchy);
// to prevent the control trying to ask the model to update an already deleted item the control is informed that currently a deleting process
// has been started and that variables can currently not be updated even when requested by the system:
wxOSXDVCScopedDeleter setDeleting(m_DataViewCtrlPtr);
wxOSXDVCScopedDeleter setDeleting(m_DataViewCtrlPtr, parent);
bool ok = m_DataViewCtrlPtr->GetDataViewPeer()->Remove(parent);
@@ -239,7 +251,7 @@ bool wxOSXDataViewModelNotifier::ItemsDeleted(wxDataViewItem const& parent, wxDa
// not to be identical because the being edited item might be below the passed item in the hierarchy);
// to prevent the control trying to ask the model to update an already deleted item the control is informed that currently a deleting process
// has been started and that variables can currently not be updated even when requested by the system:
wxOSXDVCScopedDeleter setDeleting(m_DataViewCtrlPtr);
wxOSXDVCScopedDeleter setDeleting(m_DataViewCtrlPtr, parent);
// delete all specified items:
bool ok = m_DataViewCtrlPtr->GetDataViewPeer()->Remove(parent);
@@ -270,7 +282,7 @@ bool wxOSXDataViewModelNotifier::ValueChanged(wxDataViewItem const& item, unsign
bool wxOSXDataViewModelNotifier::Cleared()
{
// NOTE: See comments in ItemDeleted method above
wxOSXDVCScopedDeleter setDeleting(m_DataViewCtrlPtr);
wxOSXDVCScopedDeleter setDeleting(m_DataViewCtrlPtr, wxDataViewItem());
return m_DataViewCtrlPtr->GetDataViewPeer()->Reload();
}
@@ -403,11 +415,23 @@ wxDataViewCtrl::~wxDataViewCtrl()
void wxDataViewCtrl::Init()
{
m_CustomRendererPtr = NULL;
m_Deleting = false;
m_Deleting = NULL;
m_cgContext = NULL;
m_ModelNotifier = NULL;
}
bool wxDataViewCtrl::IsDeleting() const
{
return m_Deleting != NULL;
}
bool wxDataViewCtrl::IsClearing() const
{
// We only set the item being deleted to an invalid item when we're
// clearing the entire model.
return m_Deleting != NULL && !m_Deleting->m_parent.IsOk();
}
bool wxDataViewCtrl::Create(wxWindow *parent,
wxWindowID id,
const wxPoint& pos,