Fix another crash with rule-based labeling + data-defined properties (f…

…ixes #13453) The writing of data-defined properties to XML was using invalid data. Also fixes a possible memory leak in assignment operator. Thanks Nyall for help tracking it down! This code has been funded by Tuscany Region (Italy) - SITA (CIG: 63526840AE) and commissioned to Gis3W s.a.s.
qgis · Sep 30, 2015 · 1c877f1 · 1c877f1
1 parent 8ed7850
commit 1c877f1
Show file tree

Hide file tree

Showing 4 changed files with 33 additions and 18 deletions.
diff --git a/python/core/qgspallabeling.sip b/python/core/qgspallabeling.sip
@@ -512,6 +512,11 @@ class QgsPalLayerSettings
     /** Set a property to static instead data defined */
     void removeDataDefinedProperty( QgsPalLayerSettings::DataDefinedProperties p );
 
+    /** Clear all data-defined properties
+     * @note added in QGIS 2.12
+     */
+    void removeAllDataDefinedProperties();
+
     /** Convert old property value to new one as delimited values
      * @note not available in python bindings; as temporary solution until refactoring of project settings
      */

diff --git a/src/core/qgspallabeling.cpp b/src/core/qgspallabeling.cpp
@@ -469,6 +469,7 @@ QgsPalLayerSettings& QgsPalLayerSettings::operator=( const QgsPalLayerSettings &
   shadowBlendMode = s.shadowBlendMode;
 
   // data defined
+  removeAllDataDefinedProperties();
   QMap< QgsPalLayerSettings::DataDefinedProperties, QgsDataDefined* >::const_iterator it = s.dataDefinedProperties.constBegin();
   for ( ; it != s.dataDefinedProperties.constEnd(); ++it )
   {
@@ -492,13 +493,7 @@ QgsPalLayerSettings::~QgsPalLayerSettings()
   delete extentGeom;
 
   // delete all QgsDataDefined objects (which also deletes their QgsExpression object)
-  QMap< QgsPalLayerSettings::DataDefinedProperties, QgsDataDefined* >::iterator it = dataDefinedProperties.begin();
-  for ( ; it != dataDefinedProperties.constEnd(); ++it )
-  {
-    delete( it.value() );
-    it.value() = 0;
-  }
-  dataDefinedProperties.clear();
+  removeAllDataDefinedProperties();
 }
 
 
@@ -628,6 +623,14 @@ void QgsPalLayerSettings::writeDataDefinedPropertyMap( QgsVectorLayer* layer, QD
             propertyValue = QVariant( values.join( "~~" ) );
           }
         }
+
+        if ( parentElem )
+        {
+          // writing to XML document (instead of writing to layer)
+          QDomDocument doc = parentElem->ownerDocument();
+          QDomElement e = dd->toXmlElement( doc, i.value().first );
+          parentElem->appendChild( e );
+        }
       }
     }
 
@@ -651,17 +654,6 @@ void QgsPalLayerSettings::writeDataDefinedPropertyMap( QgsVectorLayer* layer, QD
         layer->removeCustomProperty( QString( "labeling/dataDefinedProperty" ) + QString::number( i.value().second ) );
       }
     }
-    else if ( parentElem )
-    {
-      // writing to XML document
-      QgsDataDefined* dd = it.value();
-      if ( dd )
-      {
-        QDomDocument doc = parentElem->ownerDocument();
-        QDomElement e = dd->toXmlElement( doc, i.value().first );
-        parentElem->appendChild( e );
-      }
-    }
   }
 }
 
@@ -1541,6 +1533,17 @@ void QgsPalLayerSettings::removeDataDefinedProperty( DataDefinedProperties p )
   }
 }
 
+void QgsPalLayerSettings::removeAllDataDefinedProperties()
+{
+  QMap< QgsPalLayerSettings::DataDefinedProperties, QgsDataDefined* >::iterator it = dataDefinedProperties.begin();
+  for ( ; it != dataDefinedProperties.constEnd(); ++it )
+  {
+    delete( it.value() );
+    it.value() = 0;
+  }
+  dataDefinedProperties.clear();
+}
+
 QString QgsPalLayerSettings::updateDataDefinedString( const QString& value )
 {
   // TODO: update or remove this when project settings for labeling are migrated to better XML layout

diff --git a/src/core/qgspallabeling.h b/src/core/qgspallabeling.h
@@ -505,6 +505,11 @@ class CORE_EXPORT QgsPalLayerSettings
     /** Set a property to static instead data defined */
     void removeDataDefinedProperty( QgsPalLayerSettings::DataDefinedProperties p );
 
+    /** Clear all data-defined properties
+     * @note added in QGIS 2.12
+     */
+    void removeAllDataDefinedProperties();
+
     /** Convert old property value to new one as delimited values
      * @note not available in python bindings; as temporary solution until refactoring of project settings
      */

diff --git a/tests/src/core/testqgslabelingenginev2.cpp b/tests/src/core/testqgslabelingenginev2.cpp
@@ -168,6 +168,8 @@ void TestQgsLabelingEngineV2::testRuleBased()
   s2.obstacle = false;
   s2.dist = 2;
   s2.textColor = Qt::red;
+  s2.setDataDefinedProperty( QgsPalLayerSettings::Size, true, true, "18", QString() );
+
   root->appendChild( new QgsRuleBasedLabeling::Rule( new QgsPalLayerSettings( s2 ), 0, 0, "Class = 'Jet'" ) );
 
   vl->setLabeling( new QgsRuleBasedLabeling( root ) );