Skip to content

Commit fe48aec

Browse files
nyalldawsonnyalldawson
committedJan 17, 2018
Fix crash with undo/redo and layout multiframes, restore test
·
ltr-3_40final-3_0_0
1 parent 479358e commit fe48aec

File tree

7 files changed

+37
-56
lines changed

7 files changed

+37
-56
lines changed
 

‎python/core/layout/qgslayoutmultiframe.sip.in

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -176,26 +176,26 @@ Returns the resize mode for the multiframe.
176176
.. seealso:: :py:func:`setResizeMode`
177177
%End
178178

179-
bool writeXml( QDomElement &parentElement, QDomDocument &document, const QgsReadWriteContext &context, bool ignoreFrames = false ) const;
179+
bool writeXml( QDomElement &parentElement, QDomDocument &document, const QgsReadWriteContext &context, bool includeFrames = false ) const;
180180
%Docstring
181181
Stores the multiframe state in a DOM element.
182182

183183
:param parentElement: parent DOM element (e.g. 'Layout' element)
184184
:param document: DOM document
185185
:param context: read write context
186-
:param ignoreFrames: set to false to avoid writing state information about child frames into DOM
186+
:param includeFrames: set to true to write state information about child frames into DOM
187187

188188
.. seealso:: :py:func:`readXml`
189189
%End
190190

191-
bool readXml( const QDomElement &itemElement, const QDomDocument &document, const QgsReadWriteContext &context, bool ignoreFrames = false );
191+
bool readXml( const QDomElement &itemElement, const QDomDocument &document, const QgsReadWriteContext &context, bool includeFrames = false );
192192
%Docstring
193193
Sets the item state from a DOM element.
194194

195195
:param itemElement: is the DOM node corresponding to item (e.g. 'LayoutItem' element)
196196
:param document: DOM document
197197
:param context: read write context
198-
:param ignoreFrames: set to false to avoid read state information about child frames from DOM
198+
:param includeFrames: set to true to read state information about child frames from DOM
199199

200200
.. seealso:: :py:func:`writeXml`
201201
%End

‎src/app/layout/qgslayoutattributetablewidget.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -394,7 +394,7 @@ void QgsLayoutAttributeTableWidget::mBackgroundColorButton_colorChanged( const Q
394394

395395
void QgsLayoutAttributeTableWidget::updateGuiElements()
396396
{
397-
if ( !mTable )
397+
if ( !mTable || !mFrame )
398398
{
399399
return;
400400
}

‎src/app/layout/qgslayouthtmlwidget.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -419,7 +419,7 @@ void QgsLayoutHtmlWidget::mAddFramePushButton_clicked()
419419

420420
void QgsLayoutHtmlWidget::setGuiElementValues()
421421
{
422-
if ( !mHtml )
422+
if ( !mHtml || !mFrame )
423423
{
424424
return;
425425
}

‎src/core/layout/qgslayoutmultiframe.cpp

Lines changed: 23 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -453,34 +453,26 @@ int QgsLayoutMultiFrame::frameIndex( QgsLayoutFrame *frame ) const
453453
return mFrameItems.indexOf( frame );
454454
}
455455

456-
bool QgsLayoutMultiFrame::writeXml( QDomElement &parentElement, QDomDocument &doc, const QgsReadWriteContext &context, bool ignoreFrames ) const
456+
bool QgsLayoutMultiFrame::writeXml( QDomElement &parentElement, QDomDocument &doc, const QgsReadWriteContext &context, bool includeFrames ) const
457457
{
458458
QDomElement element = doc.createElement( QStringLiteral( "LayoutMultiFrame" ) );
459459
element.setAttribute( QStringLiteral( "resizeMode" ), mResizeMode );
460460
element.setAttribute( QStringLiteral( "uuid" ), mUuid );
461461
element.setAttribute( QStringLiteral( "type" ), type() );
462462

463-
#if 0 //TODO
464-
465-
if ( !ignoreFrames )
466-
{
467-
QList<QgsComposerFrame *>::const_iterator frameIt = mFrameItems.constBegin();
468-
for ( ; frameIt != mFrameItems.constEnd(); ++frameIt )
469-
{
470-
( *frameIt )->writeXml( elem, doc );
471-
}
472-
}
473-
#else
474-
Q_UNUSED( ignoreFrames );
475-
#endif
476-
477463
for ( QgsLayoutFrame *frame : mFrameItems )
478464
{
479465
if ( !frame )
480466
continue;
481467

482468
QDomElement childItem = doc.createElement( QStringLiteral( "childFrame" ) );
483469
childItem.setAttribute( QStringLiteral( "uuid" ), frame->uuid() );
470+
471+
if ( includeFrames )
472+
{
473+
frame->writeXml( childItem, doc, context );
474+
}
475+
484476
element.appendChild( childItem );
485477
}
486478

@@ -490,7 +482,7 @@ bool QgsLayoutMultiFrame::writeXml( QDomElement &parentElement, QDomDocument &do
490482
return true;
491483
}
492484

493-
bool QgsLayoutMultiFrame::readXml( const QDomElement &element, const QDomDocument &doc, const QgsReadWriteContext &context, bool ignoreFrames )
485+
bool QgsLayoutMultiFrame::readXml( const QDomElement &element, const QDomDocument &doc, const QgsReadWriteContext &context, bool includeFrames )
494486
{
495487
if ( element.nodeName() != QStringLiteral( "LayoutMultiFrame" ) )
496488
{
@@ -504,29 +496,6 @@ bool QgsLayoutMultiFrame::readXml( const QDomElement &element, const QDomDocumen
504496

505497
mUuid = element.attribute( QStringLiteral( "uuid" ), QUuid::createUuid().toString() );
506498
mResizeMode = static_cast< ResizeMode >( element.attribute( QStringLiteral( "resizeMode" ), QStringLiteral( "0" ) ).toInt() );
507-
#if 0 //TODO
508-
if ( !ignoreFrames )
509-
{
510-
deleteFrames();
511-
}
512-
513-
514-
if ( !ignoreFrames )
515-
{
516-
QDomNodeList frameList = itemElem.elementsByTagName( QStringLiteral( "ComposerFrame" ) );
517-
for ( int i = 0; i < frameList.size(); ++i )
518-
{
519-
QDomElement frameElem = frameList.at( i ).toElement();
520-
QgsComposerFrame *newFrame = new QgsComposerFrame( mComposition, this, 0, 0, 0, 0 );
521-
newFrame->readXml( frameElem, doc );
522-
addFrame( newFrame, false );
523-
}
524-
525-
//TODO - think there should be a recalculateFrameSizes() call here
526-
}
527-
#else
528-
Q_UNUSED( ignoreFrames );
529-
#endif
530499

531500
deleteFrames();
532501
mFrameUuids.clear();
@@ -537,8 +506,22 @@ bool QgsLayoutMultiFrame::readXml( const QDomElement &element, const QDomDocumen
537506
if ( !elementNode.isElement() )
538507
continue;
539508

540-
QString uuid = elementNode.toElement().attribute( QStringLiteral( "uuid" ) );
509+
QDomElement frameElement = elementNode.toElement();
510+
511+
QString uuid = frameElement.attribute( QStringLiteral( "uuid" ) );
541512
mFrameUuids << uuid;
513+
514+
if ( includeFrames )
515+
{
516+
QDomNodeList frameNodes = frameElement.elementsByTagName( QStringLiteral( "LayoutItem" ) );
517+
if ( !frameNodes.isEmpty() )
518+
{
519+
QDomElement frameItemElement = frameNodes.at( 0 ).toElement();
520+
std::unique_ptr< QgsLayoutFrame > newFrame = qgis::make_unique< QgsLayoutFrame >( mLayout, this );
521+
newFrame->readXml( frameItemElement, doc, context );
522+
addFrame( newFrame.release(), false );
523+
}
524+
}
542525
}
543526

544527
bool result = readPropertiesFromElement( element, doc, context );

‎src/core/layout/qgslayoutmultiframe.h

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -194,20 +194,20 @@ class CORE_EXPORT QgsLayoutMultiFrame: public QgsLayoutObject, public QgsLayoutU
194194
* \param parentElement parent DOM element (e.g. 'Layout' element)
195195
* \param document DOM document
196196
* \param context read write context
197-
* \param ignoreFrames set to false to avoid writing state information about child frames into DOM
197+
* \param includeFrames set to true to write state information about child frames into DOM
198198
* \see readXml()
199199
*/
200-
bool writeXml( QDomElement &parentElement, QDomDocument &document, const QgsReadWriteContext &context, bool ignoreFrames = false ) const;
200+
bool writeXml( QDomElement &parentElement, QDomDocument &document, const QgsReadWriteContext &context, bool includeFrames = false ) const;
201201

202202
/**
203203
* Sets the item state from a DOM element.
204204
* \param itemElement is the DOM node corresponding to item (e.g. 'LayoutItem' element)
205205
* \param document DOM document
206206
* \param context read write context
207-
* \param ignoreFrames set to false to avoid read state information about child frames from DOM
207+
* \param includeFrames set to true to read state information about child frames from DOM
208208
* \see writeXml()
209209
*/
210-
bool readXml( const QDomElement &itemElement, const QDomDocument &document, const QgsReadWriteContext &context, bool ignoreFrames = false );
210+
bool readXml( const QDomElement &itemElement, const QDomDocument &document, const QgsReadWriteContext &context, bool includeFrames = false );
211211

212212
/**
213213
* Returns a list of all child frames for this multiframe.

‎src/core/layout/qgslayoutmultiframeundocommand.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ void QgsLayoutMultiFrameUndoCommand::saveState( QDomDocument &stateDoc ) const
5656
QgsLayoutMultiFrame *item = mLayout->multiFrameByUuid( mFrameUuid );
5757
Q_ASSERT_X( item, "QgsLayoutMultiFrameUndoCommand::saveState", "could not retrieve item for saving state" );
5858

59-
item->writeXml( documentElement, stateDoc, QgsReadWriteContext() );
59+
item->writeXml( documentElement, stateDoc, QgsReadWriteContext(), true );
6060
stateDoc.appendChild( documentElement );
6161
}
6262

@@ -70,7 +70,7 @@ void QgsLayoutMultiFrameUndoCommand::restoreState( QDomDocument &stateDoc )
7070
item = recreateItem( mItemType, mLayout );
7171
}
7272

73-
item->readXml( stateDoc.documentElement().firstChild().toElement(), stateDoc, QgsReadWriteContext() );
73+
item->readXml( stateDoc.documentElement().firstChild().toElement(), stateDoc, QgsReadWriteContext(), true );
7474
mLayout->project()->setDirty( true );
7575
}
7676

‎tests/src/core/testqgslayoutmultiframe.cpp

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -44,11 +44,9 @@ class TestQgsLayoutMultiFrame : public QObject
4444
void displayName();
4545
void frameIsEmpty(); //test if frame is empty works
4646
void addRemovePage(); //test if page is added and removed for RepeatUntilFinished mode
47-
#if 0 //TODO
4847
void undoRedo(); //test that combinations of frame/multiframe undo/redo don't crash
4948
void undoRedoRemovedFrame(); //test that undo doesn't crash with removed frames
5049
void undoRedoRemovedFrame2();
51-
#endif
5250
void registry();
5351
void deleteFrame();
5452
void writeReadXml();
@@ -344,7 +342,7 @@ void TestQgsLayoutMultiFrame::addRemovePage()
344342
mLayout->removeMultiFrame( htmlItem );
345343
delete htmlItem;
346344
}
347-
#if 0//TODO
345+
348346
void TestQgsLayoutMultiFrame::undoRedo()
349347
{
350348
QgsLayoutItemHtml *htmlItem = new QgsLayoutItemHtml( mLayout );
@@ -513,7 +511,7 @@ void TestQgsLayoutMultiFrame::undoRedoRemovedFrame2()
513511
htmlItem->addFrame( frame1 );
514512

515513
}
516-
#endif
514+
517515
void TestQgsLayoutMultiFrame::registry()
518516
{
519517
// test QgsLayoutItemRegistry

0 commit comments

Comments
 (0)
Please sign in to comment.