Skip to content

Commit db738f6

Browse files
nyalldawsonnyalldawson
authored
Merge pull request #5234 from nyalldawson/action_lifetime
Fix browser action item lifetime
2 parents cf636dc + 387771f commit db738f6

35 files changed

+185
-173
lines changed

‎doc/api_break.dox

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -961,6 +961,8 @@ QgsDataItem {#qgis_api_break_3_0_QgsDataItem}
961961
- capabilities() has been removed. Use capabilities2() instead (TODO: rename back to capabilities()).
962962
- emitBeginInsertItems(), emitEndInsertItems(), emitBeginRemoveItems(), emitEndRemoveItems(), emitDataChanged(), emitStateChanged() have been removed.
963963
- Favourites was renamed to Favorites
964+
- actions() now requires a new QWidget parent argument. Subclasses should ensure that returned items have been
965+
correctly parented to this widget.
964966

965967
QgsDataItemProviderRegistry {#qgis_api_break_3_0_QgsDataItemProviderRegistry}
966968
---------------------------

‎python/core/qgsdataitem.sip

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -131,10 +131,13 @@ Create new data item.
131131
:rtype: QWidget
132132
%End
133133

134-
virtual QList<QAction *> actions();
134+
virtual QList<QAction *> actions( QWidget *parent );
135135
%Docstring
136136
Returns the list of actions available for this item. This is usually used for the popup menu on right-clicking
137137
the item. Subclasses should override this to provide actions.
138+
139+
Subclasses should ensure that ownership of created actions is correctly handled by parenting them
140+
to the specified parent widget.
138141
:rtype: list of QAction
139142
%End
140143

@@ -543,7 +546,7 @@ Check if the given path is hidden from the browser model
543546
:rtype: bool
544547
%End
545548

546-
virtual QList<QAction *> actions();
549+
virtual QList<QAction *> actions( QWidget *parent );
547550

548551

549552

‎src/core/qgsdataitem.cpp

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -511,6 +511,12 @@ bool QgsDataItem::equal( const QgsDataItem *other )
511511
mPath == other->path() );
512512
}
513513

514+
QList<QAction *> QgsDataItem::actions( QWidget *parent )
515+
{
516+
Q_UNUSED( parent );
517+
return QList<QAction *>();
518+
}
519+
514520
bool QgsDataItem::handleDoubleClick()
515521
{
516522
return false;
@@ -859,10 +865,10 @@ bool QgsDirectoryItem::hiddenPath( const QString &path )
859865
return ( idx > -1 );
860866
}
861867

862-
QList<QAction *> QgsDirectoryItem::actions()
868+
QList<QAction *> QgsDirectoryItem::actions( QWidget *parent )
863869
{
864870
QList<QAction *> result;
865-
QAction *openFolder = new QAction( tr( "Open Directory…" ), this );
871+
QAction *openFolder = new QAction( tr( "Open Directory…" ), parent );
866872
connect( openFolder, &QAction::triggered, this, [ = ]
867873
{
868874
QDesktopServices::openUrl( QUrl::fromLocalFile( mDirPath ) );

‎src/core/qgsdataitem.h

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -139,10 +139,14 @@ class CORE_EXPORT QgsDataItem : public QObject
139139

140140
virtual QWidget *paramWidget() SIP_FACTORY { return nullptr; }
141141

142-
/** Returns the list of actions available for this item. This is usually used for the popup menu on right-clicking
142+
/**
143+
* Returns the list of actions available for this item. This is usually used for the popup menu on right-clicking
143144
* the item. Subclasses should override this to provide actions.
145+
*
146+
* Subclasses should ensure that ownership of created actions is correctly handled by parenting them
147+
* to the specified parent widget.
144148
*/
145-
virtual QList<QAction *> actions() { return QList<QAction *>(); }
149+
virtual QList<QAction *> actions( QWidget *parent );
146150

147151
/** Returns whether the item accepts drag and dropped layers - e.g. for importing a dataset to a provider.
148152
* Subclasses should override this and handleDrop() to accept dropped layers.
@@ -485,7 +489,7 @@ class CORE_EXPORT QgsDirectoryItem : public QgsDataCollectionItem
485489
//! Check if the given path is hidden from the browser model
486490
static bool hiddenPath( const QString &path );
487491

488-
QList<QAction *> actions() override;
492+
QList<QAction *> actions( QWidget *parent ) override;
489493

490494

491495
public slots:

‎src/gui/qgsbrowserdockwidget.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -209,7 +209,7 @@ void QgsBrowserDockWidget::showContextMenu( QPoint pt )
209209
menu->addAction( tr( "Add a Directory..." ), this, SLOT( addFavoriteDirectory() ) );
210210
}
211211

212-
QList<QAction *> actions = item->actions();
212+
QList<QAction *> actions = item->actions( menu );
213213
if ( !actions.isEmpty() )
214214
{
215215
if ( !menu->actions().isEmpty() )

‎src/providers/arcgisrest/qgsafsdataitems.cpp

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -50,9 +50,9 @@ QVector<QgsDataItem *> QgsAfsRootItem::createChildren()
5050
}
5151

5252
#ifdef HAVE_GUI
53-
QList<QAction *> QgsAfsRootItem::actions()
53+
QList<QAction *> QgsAfsRootItem::actions( QWidget *parent )
5454
{
55-
QAction *actionNew = new QAction( tr( "New Connection..." ), this );
55+
QAction *actionNew = new QAction( tr( "New Connection..." ), parent );
5656
connect( actionNew, &QAction::triggered, this, &QgsAfsRootItem::newConnection );
5757
return QList<QAction *>() << actionNew;
5858
}
@@ -120,15 +120,15 @@ bool QgsAfsConnectionItem::equal( const QgsDataItem *other )
120120
}
121121

122122
#ifdef HAVE_GUI
123-
QList<QAction *> QgsAfsConnectionItem::actions()
123+
QList<QAction *> QgsAfsConnectionItem::actions( QWidget *parent )
124124
{
125125
QList<QAction *> lst;
126126

127-
QAction *actionEdit = new QAction( tr( "Edit..." ), this );
127+
QAction *actionEdit = new QAction( tr( "Edit..." ), parent );
128128
connect( actionEdit, &QAction::triggered, this, &QgsAfsConnectionItem::editConnection );
129129
lst.append( actionEdit );
130130

131-
QAction *actionDelete = new QAction( tr( "Delete" ), this );
131+
QAction *actionDelete = new QAction( tr( "Delete" ), parent );
132132
connect( actionDelete, &QAction::triggered, this, &QgsAfsConnectionItem::deleteConnection );
133133
lst.append( actionDelete );
134134

‎src/providers/arcgisrest/qgsafsdataitems.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ class QgsAfsRootItem : public QgsDataCollectionItem
2626
QgsAfsRootItem( QgsDataItem *parent, const QString &name, const QString &path );
2727
QVector<QgsDataItem *> createChildren() override;
2828
#ifdef HAVE_GUI
29-
QList<QAction *> actions() override;
29+
QList<QAction *> actions( QWidget *parent ) override;
3030
QWidget *paramWidget() override;
3131
#endif
3232

@@ -46,7 +46,7 @@ class QgsAfsConnectionItem : public QgsDataCollectionItem
4646
QVector<QgsDataItem *> createChildren() override;
4747
bool equal( const QgsDataItem *other ) override;
4848
#ifdef HAVE_GUI
49-
QList<QAction *> actions() override;
49+
QList<QAction *> actions( QWidget *parent ) override;
5050
#endif
5151

5252
public slots:

‎src/providers/arcgisrest/qgsamsdataitems.cpp

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -48,9 +48,9 @@ QVector<QgsDataItem *> QgsAmsRootItem::createChildren()
4848
}
4949

5050
#ifdef HAVE_GUI
51-
QList<QAction *> QgsAmsRootItem::actions()
51+
QList<QAction *> QgsAmsRootItem::actions( QWidget *parent )
5252
{
53-
QAction *actionNew = new QAction( tr( "New Connection..." ), this );
53+
QAction *actionNew = new QAction( tr( "New Connection..." ), parent );
5454
connect( actionNew, &QAction::triggered, this, &QgsAmsRootItem::newConnection );
5555
return QList<QAction *>() << actionNew;
5656
}
@@ -136,15 +136,15 @@ bool QgsAmsConnectionItem::equal( const QgsDataItem *other )
136136
}
137137

138138
#ifdef HAVE_GUI
139-
QList<QAction *> QgsAmsConnectionItem::actions()
139+
QList<QAction *> QgsAmsConnectionItem::actions( QWidget *parent )
140140
{
141141
QList<QAction *> lst;
142142

143-
QAction *actionEdit = new QAction( tr( "Edit..." ), this );
143+
QAction *actionEdit = new QAction( tr( "Edit..." ), parent );
144144
connect( actionEdit, &QAction::triggered, this, &QgsAmsConnectionItem::editConnection );
145145
lst.append( actionEdit );
146146

147-
QAction *actionDelete = new QAction( tr( "Delete" ), this );
147+
QAction *actionDelete = new QAction( tr( "Delete" ), parent );
148148
connect( actionDelete, &QAction::triggered, this, &QgsAmsConnectionItem::deleteConnection );
149149
lst.append( actionDelete );
150150

‎src/providers/arcgisrest/qgsamsdataitems.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ class QgsAmsRootItem : public QgsDataCollectionItem
2929

3030
QVector<QgsDataItem *> createChildren() override;
3131
#ifdef HAVE_GUI
32-
virtual QList<QAction *> actions() override;
32+
virtual QList<QAction *> actions( QWidget *parent ) override;
3333
virtual QWidget *paramWidget() override;
3434
#endif
3535

@@ -49,7 +49,7 @@ class QgsAmsConnectionItem : public QgsDataCollectionItem
4949
QVector<QgsDataItem *> createChildren() override;
5050
bool equal( const QgsDataItem *other ) override;
5151
#ifdef HAVE_GUI
52-
QList<QAction *> actions() override;
52+
QList<QAction *> actions( QWidget *parent ) override;
5353
#endif
5454

5555
public slots:

‎src/providers/db2/qgsdb2dataitems.cpp

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -250,19 +250,19 @@ bool QgsDb2ConnectionItem::equal( const QgsDataItem *other )
250250
}
251251

252252
#ifdef HAVE_GUI
253-
QList<QAction *> QgsDb2ConnectionItem::actions()
253+
QList<QAction *> QgsDb2ConnectionItem::actions( QWidget *parent )
254254
{
255255
QList<QAction *> lst;
256256

257-
QAction *actionRefresh = new QAction( tr( "Refresh connection" ), this );
257+
QAction *actionRefresh = new QAction( tr( "Refresh Connection" ), parent );
258258
connect( actionRefresh, &QAction::triggered, this, &QgsDb2ConnectionItem::refreshConnection );
259259
lst.append( actionRefresh );
260260

261-
QAction *actionEdit = new QAction( tr( "Edit connection..." ), this );
261+
QAction *actionEdit = new QAction( tr( "Edit Connection..." ), parent );
262262
connect( actionEdit, &QAction::triggered, this, &QgsDb2ConnectionItem::editConnection );
263263
lst.append( actionEdit );
264264

265-
QAction *actionDelete = new QAction( tr( "Delete connection" ), this );
265+
QAction *actionDelete = new QAction( tr( "Delete Connection" ), parent );
266266
connect( actionDelete, &QAction::triggered, this, &QgsDb2ConnectionItem::deleteConnection );
267267
lst.append( actionDelete );
268268

@@ -430,14 +430,13 @@ QVector<QgsDataItem *> QgsDb2RootItem::createChildren()
430430
}
431431

432432
#ifdef HAVE_GUI
433-
QList<QAction *> QgsDb2RootItem::actions()
433+
QList<QAction *> QgsDb2RootItem::actions( QWidget *parent )
434434
{
435435
QList<QAction *> actionList;
436436

437-
QAction *action = new QAction( tr( "New Connection..." ), this );
437+
QAction *action = new QAction( tr( "New Connection..." ), parent );
438438
connect( action, &QAction::triggered, this, &QgsDb2RootItem::newConnection );
439439
actionList.append( action );
440-
QgsDebugMsg( "DB2: Browser Panel; New Connection option added." );
441440

442441
return actionList;
443442
}

‎src/providers/db2/qgsdb2dataitems.h

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ class QgsDb2RootItem : public QgsDataCollectionItem
4545
#ifdef HAVE_GUI
4646
virtual QWidget *paramWidget() override;
4747

48-
virtual QList<QAction *> actions() override;
48+
QList<QAction *> actions( QWidget *parent ) override;
4949
#endif
5050

5151
public slots:
@@ -90,10 +90,7 @@ class QgsDb2ConnectionItem : public QgsDataCollectionItem
9090

9191
#ifdef HAVE_GUI
9292

93-
/**
94-
* Add Refresh, Edit, and Delete actions for every QgsDb2ConnectionItem.
95-
*/
96-
virtual QList<QAction *> actions() override;
93+
QList<QAction *> actions( QWidget *parent ) override;
9794
#endif
9895

9996
virtual bool acceptDrop() override { return true; }

‎src/providers/geonode/qgsgeonodedataitems.cpp

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -66,10 +66,10 @@ QVector<QgsDataItem *> QgsGeoNodeConnectionItem::createChildren()
6666
return services;
6767
}
6868

69-
QList<QAction *> QgsGeoNodeConnectionItem::actions()
69+
QList<QAction *> QgsGeoNodeConnectionItem::actions( QWidget *parent )
7070
{
71-
QAction *actionEdit = new QAction( tr( "Edit Connection..." ), this );
72-
QAction *actionDelete = new QAction( tr( "Delete Connection" ), this );
71+
QAction *actionEdit = new QAction( tr( "Edit Connection..." ), parent );
72+
QAction *actionDelete = new QAction( tr( "Delete Connection" ), parent );
7373
connect( actionEdit, &QAction::triggered, this, &QgsGeoNodeConnectionItem::editConnection );
7474
connect( actionDelete, &QAction::triggered, this, &QgsGeoNodeConnectionItem::deleteConnection );
7575
return QList<QAction *>() << actionEdit << actionDelete;
@@ -239,9 +239,9 @@ QVector<QgsDataItem *> QgsGeoNodeRootItem::createChildren()
239239
return connections;
240240
}
241241

242-
QList<QAction *> QgsGeoNodeRootItem::actions()
242+
QList<QAction *> QgsGeoNodeRootItem::actions( QWidget *parent )
243243
{
244-
QAction *actionNew = new QAction( tr( "New Connection..." ), this );
244+
QAction *actionNew = new QAction( tr( "New Connection..." ), parent );
245245
connect( actionNew, &QAction::triggered, this, &QgsGeoNodeRootItem::newConnection );
246246
return QList<QAction *>() << actionNew;
247247
}

‎src/providers/geonode/qgsgeonodedataitems.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ class QgsGeoNodeConnectionItem : public QgsDataCollectionItem
2828
public:
2929
QgsGeoNodeConnectionItem( QgsDataItem *parent, QString name, QString path, std::unique_ptr< QgsGeoNodeConnection > conn );
3030
QVector<QgsDataItem *> createChildren() override;
31-
virtual QList<QAction *> actions() override;
31+
QList<QAction *> actions( QWidget *parent ) override;
3232

3333
QString mGeoNodeName;
3434

@@ -67,7 +67,7 @@ class QgsGeoNodeRootItem : public QgsDataCollectionItem
6767

6868
QVector<QgsDataItem *> createChildren() override;
6969

70-
virtual QList<QAction *> actions() override;
70+
virtual QList<QAction *> actions( QWidget *parent ) override;
7171

7272
private slots:
7373
void newConnection();

‎src/providers/grass/qgsgrassprovidermodule.cpp

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -51,11 +51,11 @@ QgsGrassItemActions::QgsGrassItemActions( const QgsGrassObject &grassObject, boo
5151
{
5252
}
5353

54-
QList<QAction *> QgsGrassItemActions::actions()
54+
QList<QAction *> QgsGrassItemActions::actions( QWidget *parent )
5555
{
5656
QList<QAction *> list;
5757

58-
QAction *optionsAction = new QAction( tr( "GRASS Options" ), this );
58+
QAction *optionsAction = new QAction( tr( "GRASS Options" ), parent );
5959
connect( optionsAction, &QAction::triggered, QgsGrass::instance(), &QgsGrass::openOptions );
6060
list << optionsAction;
6161

@@ -65,14 +65,14 @@ QList<QAction *> QgsGrassItemActions::actions()
6565
// TODO: check ownership
6666
if ( mGrassObject.type() == QgsGrassObject::Location )
6767
{
68-
QAction *newMapsetAction = new QAction( QgsApplication::getThemeIcon( QStringLiteral( "grass_new_mapset.png" ) ), tr( "New mapset" ), this );
68+
QAction *newMapsetAction = new QAction( QgsApplication::getThemeIcon( QStringLiteral( "grass_new_mapset.png" ) ), tr( "New mapset" ), parent );
6969
connect( newMapsetAction, &QAction::triggered, this, &QgsGrassItemActions::newMapset );
7070
list << newMapsetAction;
7171
}
7272

7373
if ( mGrassObject.type() == QgsGrassObject::Mapset && isMapsetOwner )
7474
{
75-
QAction *openMapsetAction = new QAction( QgsApplication::getThemeIcon( QStringLiteral( "grass_open_mapset.png" ) ), tr( "Open mapset" ), this );
75+
QAction *openMapsetAction = new QAction( QgsApplication::getThemeIcon( QStringLiteral( "grass_open_mapset.png" ) ), tr( "Open mapset" ), parent );
7676
connect( openMapsetAction, &QAction::triggered, this, &QgsGrassItemActions::openMapset );
7777
list << openMapsetAction;
7878
}
@@ -82,13 +82,13 @@ QList<QAction *> QgsGrassItemActions::actions()
8282
{
8383
if ( !QgsGrass::instance()->isMapsetInSearchPath( mGrassObject.mapset() ) )
8484
{
85-
QAction *openMapsetAction = new QAction( tr( "Add mapset to search path" ), this );
85+
QAction *openMapsetAction = new QAction( tr( "Add mapset to search path" ), parent );
8686
connect( openMapsetAction, &QAction::triggered, this, &QgsGrassItemActions::addMapsetToSearchPath );
8787
list << openMapsetAction;
8888
}
8989
else
9090
{
91-
QAction *openMapsetAction = new QAction( tr( "Remove mapset from search path" ), this );
91+
QAction *openMapsetAction = new QAction( tr( "Remove mapset from search path" ), parent );
9292
connect( openMapsetAction, &QAction::triggered, this, &QgsGrassItemActions::removeMapsetFromSearchPath );
9393
list << openMapsetAction;
9494
}
@@ -97,11 +97,11 @@ QList<QAction *> QgsGrassItemActions::actions()
9797
if ( ( mGrassObject.type() == QgsGrassObject::Raster || mGrassObject.type() == QgsGrassObject::Vector
9898
|| mGrassObject.type() == QgsGrassObject::Group ) && isMapsetOwner )
9999
{
100-
QAction *renameAction = new QAction( tr( "Rename" ), this );
100+
QAction *renameAction = new QAction( tr( "Rename" ), parent );
101101
connect( renameAction, &QAction::triggered, this, &QgsGrassItemActions::renameGrassObject );
102102
list << renameAction;
103103

104-
QAction *deleteAction = new QAction( tr( "Delete" ), this );
104+
QAction *deleteAction = new QAction( tr( "Delete" ), parent );
105105
connect( deleteAction, &QAction::triggered, this, &QgsGrassItemActions::deleteGrassObject );
106106
list << deleteAction;
107107
}
@@ -110,15 +110,15 @@ QList<QAction *> QgsGrassItemActions::actions()
110110
&& mValid && isMapsetOwner )
111111
{
112112
// TODO: disable new layer actions on maps currently being edited
113-
QAction *newPointAction = new QAction( tr( "New Point Layer" ), this );
113+
QAction *newPointAction = new QAction( tr( "New Point Layer" ), parent );
114114
connect( newPointAction, &QAction::triggered, this, &QgsGrassItemActions::newPointLayer );
115115
list << newPointAction;
116116

117-
QAction *newLineAction = new QAction( tr( "New Line Layer" ), this );
117+
QAction *newLineAction = new QAction( tr( "New Line Layer" ), parent );
118118
connect( newLineAction, &QAction::triggered, this, &QgsGrassItemActions::newLineLayer );
119119
list << newLineAction;
120120

121-
QAction *newPolygonAction = new QAction( tr( "New Polygon Layer" ), this );
121+
QAction *newPolygonAction = new QAction( tr( "New Polygon Layer" ), parent );
122122
connect( newPolygonAction, &QAction::triggered, this, &QgsGrassItemActions::newPolygonLayer );
123123
list << newPolygonAction;
124124
}
@@ -1178,11 +1178,11 @@ QgsGrassImportItem::~QgsGrassImportItem()
11781178
}
11791179

11801180
#ifdef HAVE_GUI
1181-
QList<QAction *> QgsGrassImportItem::actions()
1181+
QList<QAction *> QgsGrassImportItem::actions( QWidget *parent )
11821182
{
11831183
QList<QAction *> lst;
11841184

1185-
QAction *actionRename = new QAction( tr( "Cancel" ), this );
1185+
QAction *actionRename = new QAction( tr( "Cancel" ), parent );
11861186
connect( actionRename, &QAction::triggered, this, &QgsGrassImportItem::cancel );
11871187
lst.append( actionRename );
11881188

0 commit comments

Comments
 (0)