Skip to content

Commit 1b96926

Browse files
committedJan 7, 2018
Fix crash when deleting multiframe item child frames
1 parent 3f414e2 commit 1b96926

File tree

7 files changed

+45
-19
lines changed

7 files changed

+45
-19
lines changed
 

‎python/core/layout/qgslayoutframe.sip

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,9 @@ Creates a new QgsLayoutFrame belonging to the specified ``layout``.
4343
virtual QString displayName() const;
4444

4545

46+
virtual void cleanup();
47+
48+
4649
void setContentSection( const QRectF &section );
4750
%Docstring
4851
Sets the visible part of the multiframe's content which is visible within

‎python/core/layout/qgslayoutmultiframe.sip

Lines changed: 0 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -383,20 +383,6 @@ in finalizeRestoreFromXml(), not readPropertiesFromElement().
383383

384384

385385

386-
protected slots:
387-
388-
void handlePageChange();
389-
%Docstring
390-
Adapts to changed number of layout pages if resize type is RepeatOnEveryPage.
391-
%End
392-
393-
void handleFrameRemoval();
394-
%Docstring
395-
Called when a frame is removed. Updates frame list and recalculates
396-
content of remaining frames.
397-
%End
398-
399-
400386
};
401387

402388

‎src/core/layout/qgslayoutframe.cpp

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -192,6 +192,14 @@ QString QgsLayoutFrame::displayName() const
192192
return tr( "<Frame>" );
193193
}
194194

195+
void QgsLayoutFrame::cleanup()
196+
{
197+
if ( mMultiFrame )
198+
mMultiFrame->handleFrameRemoval( this );
199+
200+
QgsLayoutItem::cleanup();
201+
}
202+
195203
void QgsLayoutFrame::draw( QgsRenderContext &context, const QStyleOptionGraphicsItem *itemStyle )
196204
{
197205
if ( mMultiFrame )

‎src/core/layout/qgslayoutframe.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,8 @@ class CORE_EXPORT QgsLayoutFrame: public QgsLayoutItem
5252
//Overridden to allow multiframe to set display name
5353
QString displayName() const override;
5454

55+
void cleanup() override;
56+
5557
/**
5658
* Sets the visible part of the multiframe's content which is visible within
5759
* this frame (relative to the total multiframe extent in layout units).

‎src/core/layout/qgslayoutmultiframe.cpp

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,10 @@ void QgsLayoutMultiFrame::addFrame( QgsLayoutFrame *frame, bool recalcFrameSizes
6060
mFrameItems.push_back( frame );
6161
frame->mMultiFrame = this;
6262
connect( frame, &QgsLayoutItem::sizePositionChanged, this, &QgsLayoutMultiFrame::recalculateFrameSizes );
63-
connect( frame, &QgsLayoutFrame::destroyed, this, &QgsLayoutMultiFrame::handleFrameRemoval );
63+
connect( frame, &QgsLayoutFrame::destroyed, this, [this, frame ]
64+
{
65+
handleFrameRemoval( frame );
66+
} );
6467
if ( mLayout )
6568
{
6669
mLayout->addLayoutItem( frame );
@@ -306,12 +309,11 @@ void QgsLayoutMultiFrame::refresh()
306309
refreshDataDefinedProperty();
307310
}
308311

309-
void QgsLayoutMultiFrame::handleFrameRemoval()
312+
void QgsLayoutMultiFrame::handleFrameRemoval( QgsLayoutFrame *frame )
310313
{
311314
if ( mBlockUpdates )
312315
return;
313316

314-
QgsLayoutFrame *frame = qobject_cast<QgsLayoutFrame *>( sender() );
315317
if ( !frame )
316318
{
317319
return;

‎src/core/layout/qgslayoutmultiframe.h

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -370,7 +370,7 @@ class CORE_EXPORT QgsLayoutMultiFrame: public QgsLayoutObject, public QgsLayoutU
370370

371371
ResizeMode mResizeMode = UseExistingFrames;
372372

373-
protected slots:
373+
private slots:
374374

375375
/**
376376
* Adapts to changed number of layout pages if resize type is RepeatOnEveryPage.
@@ -381,7 +381,7 @@ class CORE_EXPORT QgsLayoutMultiFrame: public QgsLayoutObject, public QgsLayoutU
381381
* Called when a frame is removed. Updates frame list and recalculates
382382
* content of remaining frames.
383383
*/
384-
void handleFrameRemoval();
384+
void handleFrameRemoval( QgsLayoutFrame *frame );
385385

386386

387387
private:
@@ -394,6 +394,7 @@ class CORE_EXPORT QgsLayoutMultiFrame: public QgsLayoutObject, public QgsLayoutU
394394

395395
//! Unique id
396396
QString mUuid;
397+
friend class QgsLayoutFrame;
397398
};
398399

399400

‎tests/src/core/testqgslayoutmultiframe.cpp

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ class TestQgsLayoutMultiFrame : public QObject
4747
void undoRedoRemovedFrame(); //test that undo doesn't crash with removed frames
4848
void undoRedoRemovedFrame2();
4949
void registry();
50+
void deleteFrame();
5051

5152
private:
5253
QgsLayout *mLayout = nullptr;
@@ -556,5 +557,28 @@ void TestQgsLayoutMultiFrame::registry()
556557
QVERIFY( props.isEmpty() );
557558
}
558559

560+
void TestQgsLayoutMultiFrame::deleteFrame()
561+
{
562+
QgsLayout l( QgsProject::instance() );
563+
l.initializeDefaults();
564+
565+
QgsLayoutItemHtml *htmlItem = new QgsLayoutItemHtml( &l );
566+
QgsLayoutFrame *frame1 = new QgsLayoutFrame( &l, htmlItem );
567+
frame1->attemptSetSceneRect( QRectF( 0, 0, 100, 200 ) );
568+
htmlItem->addFrame( frame1 );
569+
QgsLayoutFrame *frame2 = new QgsLayoutFrame( &l, htmlItem );
570+
frame2->attemptSetSceneRect( QRectF( 0, 0, 100, 200 ) );
571+
htmlItem->addFrame( frame2 );
572+
573+
QCOMPARE( htmlItem->frameCount(), 2 );
574+
QCOMPARE( htmlItem->frames(), QList< QgsLayoutFrame * >() << frame1 << frame2 );
575+
l.removeLayoutItem( frame1 );
576+
QCOMPARE( htmlItem->frameCount(), 1 );
577+
QCOMPARE( htmlItem->frames(), QList< QgsLayoutFrame * >() << frame2 );
578+
l.removeLayoutItem( frame2 );
579+
QCOMPARE( htmlItem->frameCount(), 0 );
580+
QVERIFY( htmlItem->frames().empty() );
581+
}
582+
559583
QGSTEST_MAIN( TestQgsLayoutMultiFrame )
560584
#include "testqgslayoutmultiframe.moc"

0 commit comments

Comments
 (0)