Fix crash when reordering composer items via drag and drop

Caused by internal Qt bug when multiple QSortFilterProxyModels
used by widgets are attached to a parent model which calls
beginMoveRows.

Adds some tests, but none reproduce the crash. Not reproducable
on Qt5 builds.

src/core/composer/qgscomposermodel.cpp

@@ -947,10 +947,15 @@ QgsComposerProxyModel::QgsComposerProxyModel( QgsComposition *composition, QObje
   if ( mComposition )
     setSourceModel( mComposition->itemsModel() );
 
+  // WARNING: the below code triggers a Qt bug (tested on Qt 4.8, can't reproduce on Qt5) whenever the QgsComposerModel source model
+  // calls beginInsertRows - since it's non functional anyway it's now disabled
+  // PLEASE verify that the Qt issue is fixed before reenabling
+#if 0
   // TODO doesn't seem to work correctly - not updated when item changes
   setDynamicSortFilter( true );
   setSortLocaleAware( true );
   sort( QgsComposerModel::ItemId );
+#endif
 }
 
 bool QgsComposerProxyModel::lessThan( const QModelIndex &left, const QModelIndex &right ) const

src/core/composer/qgscomposermodel.h

@@ -289,6 +289,7 @@ class CORE_EXPORT QgsComposerModel: public QAbstractItemModel
     void rebuildSceneItemList();
 
     friend class TestQgsComposerModel;
+    friend class TestQgsComposerGui;
 };
 
 

tests/src/core/testqgscomposermodel.cpp

@@ -63,6 +63,8 @@ class TestQgsComposerModel : public QObject
     void reorderToTopWithRemoved(); //test reordering to top with removed items
     void reorderToBottomWithRemoved(); //test reordering to bottom with removed items
 
+    void proxyCrash();
+
   private:
     QgsComposition *mComposition = nullptr;
     QgsComposerLabel *mItem1 = nullptr;
@@ -608,5 +610,26 @@ void TestQgsComposerModel::reorderToBottomWithRemoved()
   QCOMPARE( mComposition->itemsModel()->mItemsInScene.at( 1 ), mItem2 );
 }
 
+void TestQgsComposerModel::proxyCrash()
+{
+  // test for a possible crash when using QgsComposerProxyModel and reordering items
+  QgsComposition *composition = new QgsComposition( QgsProject::instance() );
+
+  // create a proxy - it's not used, but will be watching...
+  QgsComposerProxyModel *proxy = new QgsComposerProxyModel( composition );
+  Q_UNUSED( proxy );
+
+  // add some items to composition
+  QgsComposerLabel *item1 = new QgsComposerLabel( composition );
+  composition->addItem( item1 );
+  QgsComposerLabel *item2 = new QgsComposerLabel( composition );
+  composition->addItem( item2 );
+  QgsComposerLabel *item3 = new QgsComposerLabel( composition );
+  composition->addItem( item3 );
+
+  // reorder items - expect no crash!
+  composition->itemsModel()->reorderItemUp( item1 );
+}
+
 QGSTEST_MAIN( TestQgsComposerModel )
 #include "testqgscomposermodel.moc"

tests/src/gui/CMakeLists.txt

@@ -15,6 +15,7 @@ INCLUDE_DIRECTORIES(${CMAKE_CURRENT_SOURCE_DIR}
   ${CMAKE_SOURCE_DIR}/src/core
   ${CMAKE_SOURCE_DIR}/src/core/expression
   ${CMAKE_SOURCE_DIR}/src/core/auth
+  ${CMAKE_SOURCE_DIR}/src/core/composer
   ${CMAKE_SOURCE_DIR}/src/core/geometry
   ${CMAKE_SOURCE_DIR}/src/core/metadata
   ${CMAKE_SOURCE_DIR}/src/core/raster
@@ -129,3 +130,4 @@ ADD_QGIS_TEST(editorwidgetregistrytest testqgseditorwidgetregistry.cpp)
 ADD_QGIS_TEST(keyvaluewidgettest testqgskeyvaluewidget.cpp)
 ADD_QGIS_TEST(listwidgettest testqgslistwidget.cpp)
 ADD_QGIS_TEST(filedownloader testqgsfiledownloader.cpp)
+ADD_QGIS_TEST(composergui testqgscomposergui.cpp)

tests/src/gui/testqgscomposergui.cpp

@@ -0,0 +1,113 @@
+/***************************************************************************
+    testqgscomposergui.cpp
+     ----------------------
+    Date                 : May 2017
+    Copyright            : (C) 2017 Nyall Dawson
+    Email                : nyall dot dawson at gmail dot com
+ ***************************************************************************
+ *                                                                         *
+ *   This program is free software; you can redistribute it and/or modify  *
+ *   it under the terms of the GNU General Public License as published by  *
+ *   the Free Software Foundation; either version 2 of the License, or     *
+ *   (at your option) any later version.                                   *
+ *                                                                         *
+ ***************************************************************************/
+
+
+
+
+#include <QtTest/QtTest>
+
+#include "qgsmapsettings.h"
+#include "qgscomposition.h"
+#include "qgscomposerlabel.h"
+#include "qgscomposermap.h"
+#include "qgscomposermodel.h"
+#include "qgscomposeritemcombobox.h"
+
+#include <QApplication>
+#include <QMainWindow>
+
+class TestQgsComposerGui: public QObject
+{
+    Q_OBJECT
+  private slots:
+    void initTestCase(); // will be called before the first testfunction is executed.
+    void cleanupTestCase(); // will be called after the last testfunction was executed.
+    void init(); // will be called before each testfunction is executed.
+    void cleanup(); // will be called after every testfunction.
+
+    void testProxyCrash();
+
+  private:
+
+};
+
+void TestQgsComposerGui::initTestCase()
+{
+}
+
+void TestQgsComposerGui::cleanupTestCase()
+{
+}
+
+void TestQgsComposerGui::init()
+{
+}
+
+void TestQgsComposerGui::cleanup()
+{
+}
+
+void TestQgsComposerGui::testProxyCrash()
+{
+  // test for a possible crash when using QgsComposerProxyModel and reordering items
+  QgsComposition *composition = new QgsComposition( QgsProject::instance() );
+
+  // create a composer item combobox
+  QgsComposerItemComboBox *cb = new QgsComposerItemComboBox( nullptr, composition );
+  QgsComposerItemComboBox *cb2 = new QgsComposerItemComboBox( nullptr, composition );
+  QgsComposerItemComboBox *cb3 = new QgsComposerItemComboBox( nullptr, composition );
+  QgsComposerItemComboBox *cb4 = new QgsComposerItemComboBox( nullptr, composition );
+
+  // add some items to composition
+  QgsComposerMap *item1 = new QgsComposerMap( composition );
+  composition->addItem( item1 );
+  QgsComposerLabel *item2 = new QgsComposerLabel( composition );
+  composition->addItem( item2 );
+  QgsComposerMap *item3 = new QgsComposerMap( composition );
+  composition->addItem( item3 );
+
+  QCOMPARE( cb->count(), 3 );
+  cb->setItemType( QgsComposerItem::ComposerMap );
+  QCOMPARE( cb->count(), 2 );
+
+  cb4->setItemType( QgsComposerItem::ComposerLabel );
+
+  cb->setItem( item1 );
+  QCOMPARE( cb->currentItem(), item1 );
+  cb2->setItem( item3 );
+  QCOMPARE( cb2->currentItem(), item3 );
+  cb3->setItem( item2 );
+  QCOMPARE( cb3->currentItem(), item2 );
+
+  // reorder items - expect no crash!
+  // we do this by calling the private members, in order to simulate what
+  // happens when a drag and drop reorder occurs
+  composition->itemsModel()->mItemZList.removeOne( item1 );
+  composition->itemsModel()->mItemZList.insert( 1, item1 );
+  composition->itemsModel()->rebuildSceneItemList();
+
+  QCOMPARE( cb->currentItem(), item1 );
+  QCOMPARE( cb2->currentItem(), item3 );
+
+  composition->itemsModel()->mItemZList.removeOne( item1 );
+  composition->itemsModel()->mItemZList.insert( 0, item1 );
+  composition->itemsModel()->rebuildSceneItemList();
+
+  delete composition;
+}
+
+
+QTEST_MAIN( TestQgsComposerGui )
+#include "testqgscomposergui.moc"