From a49567109ab36421dbd68a4c86ef5ecf3d364ab5 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Fri, 28 Aug 2015 23:58:10 +0200 Subject: [PATCH] Consistently check for type mismatch in all ports in wxDataViewCtrl. Move the checks for the type mismatch between the type of the value returned by wxDataViewModel and the type expected by wxDataViewRenderer into common code. This avoids duplicating the same code in wxGTK and wxOSX and, more importantly, means that this check is also performed in wxMSW when using the generic version, so that the problems such as the one fixed in 3ff8c3c ("add missing wxDataViewDateRenderer::GetDefaultType()") would be visible there too. --- include/wx/dvrenderers.h | 28 +++++++++++++++++++- src/common/datavcmn.cpp | 53 +++++++++++++++++++++++++++++++++---- src/gtk/dataview.cpp | 18 +------------ src/osx/carbon/dataview.cpp | 5 +--- src/osx/cocoa/dataview.mm | 15 +---------- 5 files changed, 78 insertions(+), 41 deletions(-) diff --git a/include/wx/dvrenderers.h b/include/wx/dvrenderers.h index ea102e3854..5493388a36 100644 --- a/include/wx/dvrenderers.h +++ b/include/wx/dvrenderers.h @@ -121,7 +121,27 @@ public: wxString GetVariantType() const { return m_variantType; } - // helper that calls SetValue and SetAttr: + // Prepare for rendering the value of the corresponding item in the given + // column taken from the provided non-null model. + // + // Notice that the column must be the same as GetOwner()->GetModelColumn(), + // it is only passed to this method because the existing code already has + // it and should probably be removed in the future. + // + // Returns false if the value is missing (or invalid, i.e. has a wrong type + // differing from GetVariantType() of this renderer, in which case a debug + // error is also logged as it indicates an error in the user code). + bool PrepareValue(const wxDataViewModel* model, + const wxDataViewItem& item, + unsigned column); + + // Prepare for rendering the given item by both calling PrepareValue() and + // setting up the attributes. + // + // This is currently only used in the generic version but should really be + // used everywhere, i.e. SetEnabled() and SetAttr() should be overridden in + // the native versions instead of adding platform-specific equivalents for + // them as it's done currently. void PrepareForItem(const wxDataViewModel *model, const wxDataViewItem& item, unsigned column); @@ -178,6 +198,12 @@ protected: // Called from {Cancel,Finish}Editing() to cleanup m_editorCtrl void DestroyEditControl(); + // Helper of PrepareValue() also used in StartEditing(): returns the value + // checking that its type matches our GetVariantType(). + wxVariant CheckedGetValue(const wxDataViewModel* model, + const wxDataViewItem& item, + unsigned column) const; + wxString m_variantType; wxDataViewColumn *m_owner; wxWeakRef m_editorCtrl; diff --git a/src/common/datavcmn.cpp b/src/common/datavcmn.cpp index 53252c08e8..b8babcdecf 100644 --- a/src/common/datavcmn.cpp +++ b/src/common/datavcmn.cpp @@ -683,8 +683,7 @@ bool wxDataViewRendererBase::StartEditing( const wxDataViewItem &item, wxRect la m_item = item; // remember for later unsigned int col = GetOwner()->GetModelColumn(); - wxVariant value; - dv_ctrl->GetModel()->GetValue( value, item, col ); + const wxVariant& value = CheckedGetValue(dv_ctrl->GetModel(), item, col); m_editorCtrl = CreateEditorCtrl( dv_ctrl->GetMainWindow(), labelRect, value ); @@ -778,13 +777,57 @@ bool wxDataViewRendererBase::FinishEditing() return false; } +wxVariant +wxDataViewRendererBase::CheckedGetValue(const wxDataViewModel* model, + const wxDataViewItem& item, + unsigned column) const +{ + wxVariant value; + model->GetValue(value, item, column); + + // We always allow the cell to be null, regardless of the renderer type. + if ( !value.IsNull() ) + { + if ( value.GetType() != GetVariantType() ) + { + // If you're seeing this message, this indicates that either your + // renderer is using the wrong type, or your model returns values + // of the wrong type. + wxLogDebug("Wrong type returned from the model for column %u: " + "%s required but actual type is %s", + column, + GetVariantType(), + value.GetType()); + + // Don't return data of mismatching type, this could be unexpected. + value.MakeNull(); + } + } + + return value; +} + +bool +wxDataViewRendererBase::PrepareValue(const wxDataViewModel* model, + const wxDataViewItem& item, + unsigned column) +{ + const wxVariant& value = CheckedGetValue(model, item, column); + + if ( value.IsNull() ) + return false; + + SetValue(value); + + return true; +} + void wxDataViewRendererBase::PrepareForItem(const wxDataViewModel *model, const wxDataViewItem& item, unsigned column) { - wxVariant value; - model->GetValue(value, item, column); - SetValue(value); + if ( !PrepareValue(model, item, column) ) + return; wxDataViewItemAttr attr; model->GetAttr(item, column, attr); diff --git a/src/gtk/dataview.cpp b/src/gtk/dataview.cpp index eb6f79dd5c..1e8082018e 100644 --- a/src/gtk/dataview.cpp +++ b/src/gtk/dataview.cpp @@ -2923,23 +2923,7 @@ static void wxGtkTreeCellDataFunc( GtkTreeViewColumn *WXUNUSED(column), return; } - wxVariant value; - wx_model->GetValue( value, item, column ); - - // It is always possible to leave a cell empty, don't warn about type - // mismatch in this case. - if (!value.IsNull()) - { - if (value.GetType() != cell->GetVariantType()) - { - wxLogDebug("Wrong type returned from the model: " - "%s required but actual type is %s", - cell->GetVariantType(), - value.GetType()); - } - } - - cell->SetValue( value ); + cell->PrepareValue(wx_model, item, column); // deal with disabled items bool enabled = wx_model->IsEnabled( item, column ); diff --git a/src/osx/carbon/dataview.cpp b/src/osx/carbon/dataview.cpp index d16984af4c..70bec29329 100644 --- a/src/osx/carbon/dataview.cpp +++ b/src/osx/carbon/dataview.cpp @@ -1410,7 +1410,6 @@ OSStatus wxMacDataViewDataBrowserListViewControl::DataBrowserGetSetItemDataProc( if (propertyID >= kMinPropertyID) // in case data columns set the data { // variable definitions: - wxVariant variant; wxDataViewColumn* dataViewColumnPtr; wxDataViewCtrl* dataViewCtrlPtr; @@ -1420,11 +1419,9 @@ OSStatus wxMacDataViewDataBrowserListViewControl::DataBrowserGetSetItemDataProc( dataViewColumnPtr = GetColumnPtr(propertyID); wxCHECK_MSG(dataViewColumnPtr != NULL,errDataBrowserNotConfigured,_("No column for the specified column position existing.")); wxCHECK_MSG(dataViewColumnPtr->GetRenderer() != NULL,errDataBrowserNotConfigured,_("No renderer specified for column.")); - dataViewCtrlPtr->GetModel()->GetValue(variant,wxDataViewItem(reinterpret_cast(itemID)),dataViewColumnPtr->GetModelColumn()); - if (!(variant.IsNull())) + if (dataViewColumnPtr->GetRenderer()->PrepareValue(dataViewCtrlPtr->GetModel(),wxDataViewItem(reinterpret_cast(itemID)),dataViewColumnPtr->GetModelColumn())) { dataViewColumnPtr->GetRenderer()->GetNativeData()->SetItemDataRef(itemData); - dataViewColumnPtr->GetRenderer()->SetValue(variant); wxCHECK_MSG(dataViewColumnPtr->GetRenderer()->MacRender(),errDataBrowserNotConfigured,_("Rendering failed.")); } return noErr; diff --git a/src/osx/cocoa/dataview.mm b/src/osx/cocoa/dataview.mm index 4bf947052b..9ff9592575 100644 --- a/src/osx/cocoa/dataview.mm +++ b/src/osx/cocoa/dataview.mm @@ -1814,26 +1814,13 @@ outlineView:(NSOutlineView*)outlineView renderer->OSXApplyEnabled(enabled); // check if we have anything to render - wxVariant value; - model->GetValue(value, dvItem, colIdx); - if ( value.IsNull() ) + if ( !renderer->PrepareValue(model, dvItem, colIdx) ) { // for consistency with the generic implementation, just handle missing // values as blank return; } - if ( value.GetType() != renderer->GetVariantType() ) - { - wxLogDebug("Wrong type returned from the model: " - "%s required but actual type is %s", - renderer->GetVariantType(), - value.GetType()); - - // we can't use the value of wrong type - return; - } - // use the attributes: notice that we need to do this whether we have them // or not as even if this cell doesn't have any attributes, the previous // one might have had some and then we need to reset them back to default