fix crash when triggering already destroyed map layer actions in iden…

…tify (alternative fix for PR#1319)
qgis · Apr 14, 2014 · 96809eb · 96809eb
1 parent 83d30b8
commit 96809eb
Show file tree

Hide file tree

Showing 3 changed files with 32 additions and 13 deletions.
diff --git a/src/app/qgsidentifyresultsdialog.cpp b/src/app/qgsidentifyresultsdialog.cpp
@@ -31,6 +31,7 @@
 #include "qgslogger.h"
 #include "qgsnetworkaccessmanager.h"
 #include "qgsproject.h"
+#include "qgsmaplayeractionregistry.h"
 
 #include <QCloseEvent>
 #include <QLabel>
@@ -242,6 +243,7 @@ void QgsIdentifyResultsWebViewItem::loadFinished( bool ok )
 //     actions (if any) [userrole: "actions"]
 //       edit [userrole: "edit"]
 //       action [userrole: "action", idx]
+//       action [userrole: "map_layer_action", QgsMapLayerAction]
 //     displayname [userroles: fieldIdx, original name] displayvalue [userrole: original value]
 //     displayname [userroles: fieldIdx, original name] displayvalue [userrole: original value]
 //     displayname [userroles: fieldIdx, original name] displayvalue [userrole: original value]
@@ -421,9 +423,9 @@ void QgsIdentifyResultsDialog::addFeature( QgsVectorLayer *vlayer, const QgsFeat
   }
 
   //get valid QgsMapLayerActions for this layer
-  mMapLayerActions = QgsMapLayerActionRegistry::instance()->mapLayerActions( vlayer );
+  QList< QgsMapLayerAction* > registeredActions = QgsMapLayerActionRegistry::instance()->mapLayerActions( vlayer );
 
-  if ( vlayer->pendingFields().size() > 0 || vlayer->actions()->size() || mMapLayerActions.size() )
+  if ( vlayer->pendingFields().size() > 0 || vlayer->actions()->size() || registeredActions.size() )
   {
     QTreeWidgetItem *actionItem = new QTreeWidgetItem( QStringList() << tr( "(Actions)" ) );
     actionItem->setData( 0, Qt::UserRole, "actions" );
@@ -452,20 +454,35 @@ void QgsIdentifyResultsDialog::addFeature( QgsVectorLayer *vlayer, const QgsFeat
     }
 
     //add actions from QgsMapLayerActionRegistry
-    for ( int i = 0; i < mMapLayerActions.size(); i++ )
+    for ( int i = 0; i < registeredActions.size(); i++ )
     {
-      QgsMapLayerAction* action = mMapLayerActions.at( i );
+      QgsMapLayerAction* action = registeredActions.at( i );
       QTreeWidgetItem *twi = new QTreeWidgetItem( QStringList() << "" << action->text() );
       twi->setIcon( 0, QgsApplication::getThemeIcon( "/mAction.svg" ) );
       twi->setData( 0, Qt::UserRole, "map_layer_action" );
-      twi->setData( 0, Qt::UserRole + 1, QVariant::fromValue( i ) );
+      twi->setData( 0, Qt::UserRole + 1, qVariantFromValue( qobject_cast<QObject *>( action ) ) );
       actionItem->addChild( twi );
+
+      connect( action, SIGNAL( destroyed() ), this, SLOT( mapLayerActionDestroyed() ) );
     }
   }
 
   highlightFeature( featItem );
 }
 
+void QgsIdentifyResultsDialog::mapLayerActionDestroyed()
+{
+  QTreeWidgetItemIterator it( lstResults );
+  while( *it )
+  {
+    if( (*it)->data( 0, Qt::UserRole ) == "map_layer_action" &&
+        (*it)->data( 0, Qt::UserRole + 1 ).value< QObject *>() == sender() )
+      delete *it;
+    else
+      ++it;
+  }
+}
+
 void QgsIdentifyResultsDialog::addFeature( QgsRasterLayer *layer,
     QString label,
     const QMap<QString, QString> &attributes,
@@ -692,11 +709,8 @@ void QgsIdentifyResultsDialog::itemClicked( QTreeWidgetItem *item, int column )
   }
   else if ( item->data( 0, Qt::UserRole ).toString() == "map_layer_action" )
   {
-    QgsMapLayerAction* action = mMapLayerActions.at( item->data( 0, Qt::UserRole + 1 ).toInt() );
-    if ( action )
-    {
-      doMapLayerAction( item, action );
-    }
+    QObject *action = item->data( 0, Qt::UserRole + 1 ).value<QObject *>();
+    doMapLayerAction( item, qobject_cast<QgsMapLayerAction *>( action ) );
   }
 }
 
@@ -939,6 +953,9 @@ void QgsIdentifyResultsDialog::doMapLayerAction( QTreeWidgetItem *item, QgsMapLa
   if ( !layer )
     return;
 
+  if ( !action )
+    return;
+
   int featIdx = featItem->data( 0, Qt::UserRole + 1 ).toInt();
   action->triggerForFeature( layer, &mFeatures[ featIdx ] );
 }

diff --git a/src/app/qgsidentifyresultsdialog.h b/src/app/qgsidentifyresultsdialog.h
@@ -191,6 +191,8 @@ class APP_EXPORT QgsIdentifyResultsDialog: public QDialog, private Ui::QgsIdenti
 
     void printCurrentItem();
 
+    void mapLayerActionDestroyed();
+
   private:
     enum ItemDataRole
     {
@@ -202,8 +204,6 @@ class APP_EXPORT QgsIdentifyResultsDialog: public QDialog, private Ui::QgsIdenti
     QgsMapCanvas *mCanvas;
     QList<QgsFeature> mFeatures;
 
-    QList< QgsMapLayerAction* > mMapLayerActions;
-
     QgsMapLayer *layer( QTreeWidgetItem *item );
     QgsVectorLayer *vectorLayer( QTreeWidgetItem *item );
     QgsRasterLayer *rasterLayer( QTreeWidgetItem *item );

diff --git a/src/gui/qgsmaptoolidentify.cpp b/src/gui/qgsmaptoolidentify.cpp
@@ -663,7 +663,9 @@ void QgsMapToolIdentify::handleMenuHover()
 
 void QgsMapToolIdentify::deleteRubberBands()
 {
-  qDeleteAll( mRubberBands );
+  QList<QgsHighlight*>::const_iterator it = mRubberBands.constBegin();
+  for ( ; it != mRubberBands.constEnd(); ++it )
+    delete *it;
 }
 
 void QgsMapToolIdentify::layerDestroyed()