Skip to content

Commit ba079d8

Browse files
nyalldawsonnyalldawson
committedMay 27, 2017
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.
·
ltr-3_40final-3_0_0
1 parent 86ce441 commit ba079d8

File tree

5 files changed

+144
-0
lines changed

5 files changed

+144
-0
lines changed
 

‎src/core/composer/qgscomposermodel.cpp

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -947,10 +947,15 @@ QgsComposerProxyModel::QgsComposerProxyModel( QgsComposition *composition, QObje
947947
if ( mComposition )
948948
setSourceModel( mComposition->itemsModel() );
949949

950+
// WARNING: the below code triggers a Qt bug (tested on Qt 4.8, can't reproduce on Qt5) whenever the QgsComposerModel source model
951+
// calls beginInsertRows - since it's non functional anyway it's now disabled
952+
// PLEASE verify that the Qt issue is fixed before reenabling
953+
#if 0
950954
// TODO doesn't seem to work correctly - not updated when item changes
951955
setDynamicSortFilter( true );
952956
setSortLocaleAware( true );
953957
sort( QgsComposerModel::ItemId );
958+
#endif
954959
}
955960

956961
bool QgsComposerProxyModel::lessThan( const QModelIndex &left, const QModelIndex &right ) const

‎src/core/composer/qgscomposermodel.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -289,6 +289,7 @@ class CORE_EXPORT QgsComposerModel: public QAbstractItemModel
289289
void rebuildSceneItemList();
290290

291291
friend class TestQgsComposerModel;
292+
friend class TestQgsComposerGui;
292293
};
293294

294295

‎tests/src/core/testqgscomposermodel.cpp

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,8 @@ class TestQgsComposerModel : public QObject
6363
void reorderToTopWithRemoved(); //test reordering to top with removed items
6464
void reorderToBottomWithRemoved(); //test reordering to bottom with removed items
6565

66+
void proxyCrash();
67+
6668
private:
6769
QgsComposition *mComposition = nullptr;
6870
QgsComposerLabel *mItem1 = nullptr;
@@ -608,5 +610,26 @@ void TestQgsComposerModel::reorderToBottomWithRemoved()
608610
QCOMPARE( mComposition->itemsModel()->mItemsInScene.at( 1 ), mItem2 );
609611
}
610612

613+
void TestQgsComposerModel::proxyCrash()
614+
{
615+
// test for a possible crash when using QgsComposerProxyModel and reordering items
616+
QgsComposition *composition = new QgsComposition( QgsProject::instance() );
617+
618+
// create a proxy - it's not used, but will be watching...
619+
QgsComposerProxyModel *proxy = new QgsComposerProxyModel( composition );
620+
Q_UNUSED( proxy );
621+
622+
// add some items to composition
623+
QgsComposerLabel *item1 = new QgsComposerLabel( composition );
624+
composition->addItem( item1 );
625+
QgsComposerLabel *item2 = new QgsComposerLabel( composition );
626+
composition->addItem( item2 );
627+
QgsComposerLabel *item3 = new QgsComposerLabel( composition );
628+
composition->addItem( item3 );
629+
630+
// reorder items - expect no crash!
631+
composition->itemsModel()->reorderItemUp( item1 );
632+
}
633+
611634
QGSTEST_MAIN( TestQgsComposerModel )
612635
#include "testqgscomposermodel.moc"

‎tests/src/gui/CMakeLists.txt

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ INCLUDE_DIRECTORIES(${CMAKE_CURRENT_SOURCE_DIR}
1515
${CMAKE_SOURCE_DIR}/src/core
1616
${CMAKE_SOURCE_DIR}/src/core/expression
1717
${CMAKE_SOURCE_DIR}/src/core/auth
18+
${CMAKE_SOURCE_DIR}/src/core/composer
1819
${CMAKE_SOURCE_DIR}/src/core/geometry
1920
${CMAKE_SOURCE_DIR}/src/core/metadata
2021
${CMAKE_SOURCE_DIR}/src/core/raster
@@ -129,3 +130,4 @@ ADD_QGIS_TEST(editorwidgetregistrytest testqgseditorwidgetregistry.cpp)
129130
ADD_QGIS_TEST(keyvaluewidgettest testqgskeyvaluewidget.cpp)
130131
ADD_QGIS_TEST(listwidgettest testqgslistwidget.cpp)
131132
ADD_QGIS_TEST(filedownloader testqgsfiledownloader.cpp)
133+
ADD_QGIS_TEST(composergui testqgscomposergui.cpp)

‎tests/src/gui/testqgscomposergui.cpp

Lines changed: 113 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,113 @@
1+
/***************************************************************************
2+
testqgscomposergui.cpp
3+
----------------------
4+
Date : May 2017
5+
Copyright : (C) 2017 Nyall Dawson
6+
Email : nyall dot dawson at gmail dot com
7+
***************************************************************************
8+
* *
9+
* This program is free software; you can redistribute it and/or modify *
10+
* it under the terms of the GNU General Public License as published by *
11+
* the Free Software Foundation; either version 2 of the License, or *
12+
* (at your option) any later version. *
13+
* *
14+
***************************************************************************/
15+
16+
17+
18+
19+
#include <QtTest/QtTest>
20+
21+
#include "qgsmapsettings.h"
22+
#include "qgscomposition.h"
23+
#include "qgscomposerlabel.h"
24+
#include "qgscomposermap.h"
25+
#include "qgscomposermodel.h"
26+
#include "qgscomposeritemcombobox.h"
27+
28+
#include <QApplication>
29+
#include <QMainWindow>
30+
31+
class TestQgsComposerGui: public QObject
32+
{
33+
Q_OBJECT
34+
private slots:
35+
void initTestCase(); // will be called before the first testfunction is executed.
36+
void cleanupTestCase(); // will be called after the last testfunction was executed.
37+
void init(); // will be called before each testfunction is executed.
38+
void cleanup(); // will be called after every testfunction.
39+
40+
void testProxyCrash();
41+
42+
private:
43+
44+
};
45+
46+
void TestQgsComposerGui::initTestCase()
47+
{
48+
}
49+
50+
void TestQgsComposerGui::cleanupTestCase()
51+
{
52+
}
53+
54+
void TestQgsComposerGui::init()
55+
{
56+
}
57+
58+
void TestQgsComposerGui::cleanup()
59+
{
60+
}
61+
62+
void TestQgsComposerGui::testProxyCrash()
63+
{
64+
// test for a possible crash when using QgsComposerProxyModel and reordering items
65+
QgsComposition *composition = new QgsComposition( QgsProject::instance() );
66+
67+
// create a composer item combobox
68+
QgsComposerItemComboBox *cb = new QgsComposerItemComboBox( nullptr, composition );
69+
QgsComposerItemComboBox *cb2 = new QgsComposerItemComboBox( nullptr, composition );
70+
QgsComposerItemComboBox *cb3 = new QgsComposerItemComboBox( nullptr, composition );
71+
QgsComposerItemComboBox *cb4 = new QgsComposerItemComboBox( nullptr, composition );
72+
73+
// add some items to composition
74+
QgsComposerMap *item1 = new QgsComposerMap( composition );
75+
composition->addItem( item1 );
76+
QgsComposerLabel *item2 = new QgsComposerLabel( composition );
77+
composition->addItem( item2 );
78+
QgsComposerMap *item3 = new QgsComposerMap( composition );
79+
composition->addItem( item3 );
80+
81+
QCOMPARE( cb->count(), 3 );
82+
cb->setItemType( QgsComposerItem::ComposerMap );
83+
QCOMPARE( cb->count(), 2 );
84+
85+
cb4->setItemType( QgsComposerItem::ComposerLabel );
86+
87+
cb->setItem( item1 );
88+
QCOMPARE( cb->currentItem(), item1 );
89+
cb2->setItem( item3 );
90+
QCOMPARE( cb2->currentItem(), item3 );
91+
cb3->setItem( item2 );
92+
QCOMPARE( cb3->currentItem(), item2 );
93+
94+
// reorder items - expect no crash!
95+
// we do this by calling the private members, in order to simulate what
96+
// happens when a drag and drop reorder occurs
97+
composition->itemsModel()->mItemZList.removeOne( item1 );
98+
composition->itemsModel()->mItemZList.insert( 1, item1 );
99+
composition->itemsModel()->rebuildSceneItemList();
100+
101+
QCOMPARE( cb->currentItem(), item1 );
102+
QCOMPARE( cb2->currentItem(), item3 );
103+
104+
composition->itemsModel()->mItemZList.removeOne( item1 );
105+
composition->itemsModel()->mItemZList.insert( 0, item1 );
106+
composition->itemsModel()->rebuildSceneItemList();
107+
108+
delete composition;
109+
}
110+
111+
112+
QTEST_MAIN( TestQgsComposerGui )
113+
#include "testqgscomposergui.moc"

0 commit comments

Comments
 (0)
Please sign in to comment.