"On map identification" option of relation reference widget not working properly

[qgib]

Author Name: zicke - (zicke -)
Original Redmine Issue: 14805
Affected QGIS version: master
Redmine category:relations
Assignee: Matthias Kuhn

---

The "On map identification" option does not work if "Add feature" dialog is open. The identification works if you first add the (geometryless) feature and close the dialog (saving feature is not needed) and then identify the geometry.

[qgib]

Author Name: Giovanni Manghi (@gioman)

---

• regression was configured as 0
• easy_fix was configured as 0

[qgib]

Author Name: Simon South (@simonsouth)

---

This problem still occurs when trying to add related features (through a many-to-many relation) via the attribute table. It happens because the "Add Feature" dialog is "shown by calling its @exec()@ method":

QGIS/src/app/qgsfeatureaction.cpp

Line 251 in 1192f94

dialog->exec();

, effectively "making the dialog application-modal":https://doc.qt.io/qt-5/qdialog.html#modal-dialogs and preventing the user from interacting with the main window while it is open (so the feature-identification tool cannot function). Issue #19596 addressed the same problem occurring outside this specific context.

For the courageous, a simple workaround is to apply a similar fix by changing "line 40 of qgsguivectorlayertools.cpp":

QGIS/src/app/qgsguivectorlayertools.cpp

Line 40 in 1192f94

bool added = a.addFeature( defaultValues );

from

bool added = a.addFeature( defaultValues );
</code>

to

bool added = a.addFeature( defaultValues, false );
</code>

which effectively makes the dialog non-modal and allows the feature-identification tool to function normally.

This isn't really a solution, though:

• There's no parent-child relationship between the attribute table and the "Add Feature" dialog (nor any obvious way of specifying one presently), so one window or the other tends to get lost behind the application.

• Along the same lines, there's no way to specify "window modality":https://doc.qt.io/qt-5/qt.html#WindowModality-enum for the dialog, which is probably what's really needed here.

• The user is no longer blocked from clicking on the attribute table's controls while the dialog is open, so it may be possible to get the application or the layer in an inconsistent state.

I think a "proper" fix would involve refactoring the "Add Feature" dialog's logic, currently spread across "QgsFeatureAction::addFeature()":

QGIS/src/app/qgsfeatureaction.cpp

Lines 168 to 256 in 1192f94

	bool QgsFeatureAction::addFeature( const QgsAttributeMap &defaultAttributes, bool showModal, QgsExpressionContextScope *scope SIP_TRANSFER )
	{
	if ( !mLayer || !mLayer->isEditable() )
	return false;
	QgsSettings settings;
	bool reuseLastValues = settings.value( QStringLiteral( "qgis/digitizing/reuseLastValues" ), false ).toBool();
	QgsDebugMsg( QString( "reuseLastValues: %1" ).arg( reuseLastValues ) );
	QgsFields fields = mLayer->fields();
	QgsAttributeMap initialAttributeValues;
	for ( int idx = 0; idx < fields.count(); ++idx )
	{
	if ( defaultAttributes.contains( idx ) )
	{
	initialAttributeValues.insert( idx, defaultAttributes.value( idx ) );
	}
	else if ( reuseLastValues && sLastUsedValues.contains( mLayer ) && sLastUsedValues[ mLayer ].contains( idx ) )
	{
	initialAttributeValues.insert( idx, sLastUsedValues[ mLayer ][idx] );
	}
	}
	// create new feature template - this will initialize the attributes to valid values, handling default
	// values and field constraints
	QgsExpressionContext context = mLayer->createExpressionContext();
	if ( scope )
	context.appendScope( scope );
	QgsFeature newFeature = QgsVectorLayerUtils::createFeature( mLayer, mFeature->geometry(), initialAttributeValues,
	&context );
	*mFeature = newFeature;
	//show the dialog to enter attribute values
	//only show if enabled in settings and layer has fields
	bool isDisabledAttributeValuesDlg = ( fields.count() == 0 ) || settings.value( QStringLiteral( "qgis/digitizing/disable_enter_attribute_values_dialog" ), false ).toBool();
	// override application-wide setting with any layer setting
	switch ( mLayer->editFormConfig().suppress() )
	{
	case QgsEditFormConfig::SuppressOn:
	isDisabledAttributeValuesDlg = true;
	break;
	case QgsEditFormConfig::SuppressOff:
	isDisabledAttributeValuesDlg = false;
	break;
	case QgsEditFormConfig::SuppressDefault:
	break;
	}
	if ( isDisabledAttributeValuesDlg )
	{
	mLayer->beginEditCommand( text() );
	mFeatureSaved = mLayer->addFeature( *mFeature );
	if ( mFeatureSaved )
	{
	mLayer->endEditCommand();
	mLayer->triggerRepaint();
	}
	else
	{
	mLayer->destroyEditCommand();
	}
	}
	else
	{
	QgsAttributeDialog *dialog = newDialog( false );
	// delete the dialog when it is closed
	dialog->setAttribute( Qt::WA_DeleteOnClose );
	dialog->setMode( QgsAttributeForm::AddFeatureMode );
	dialog->setEditCommandMessage( text() );
	connect( dialog->attributeForm(), &QgsAttributeForm::featureSaved, this, &QgsFeatureAction::onFeatureSaved );
	if ( !showModal )
	{
	setParent( dialog ); // keep dialog until the dialog is closed and destructed
	dialog->show();
	mFeature = nullptr;
	return true;
	}
	dialog->exec();
	}
	// Will be set in the onFeatureSaved SLOT
	return mFeatureSaved;
	}

and "QgsFeatureAction::newDialog()":

QGIS/src/app/qgsfeatureaction.cpp

Lines 52 to 101 in 1192f94

	QgsAttributeDialog *QgsFeatureAction::newDialog( bool cloneFeature )
	{
	QgsFeature *f = cloneFeature ? new QgsFeature( *mFeature ) : mFeature;
	QgsAttributeEditorContext context;
	QgsDistanceArea myDa;
	myDa.setSourceCrs( mLayer->crs(), QgsProject::instance()->transformContext() );
	myDa.setEllipsoid( QgsProject::instance()->ellipsoid() );
	context.setDistanceArea( myDa );
	context.setVectorLayerTools( QgisApp::instance()->vectorLayerTools() );
	context.setMapCanvas( QgisApp::instance()->mapCanvas() );
	context.setFormMode( QgsAttributeEditorContext::StandaloneDialog );
	QgsAttributeDialog *dialog = new QgsAttributeDialog( mLayer, f, cloneFeature, parentWidget(), true, context );
	dialog->setWindowFlags( dialog->windowFlags() | Qt::Tool );
	dialog->setObjectName( QStringLiteral( "featureactiondlg:%1:%2" ).arg( mLayer->id() ).arg( f->id() ) );
	QList<QgsAction> actions = mLayer->actions()->actions( QStringLiteral( "Feature" ) );
	if ( !actions.isEmpty() )
	{
	dialog->setContextMenuPolicy( Qt::ActionsContextMenu );
	QAction *a = new QAction( tr( "Run actions" ), dialog );
	a->setEnabled( false );
	dialog->addAction( a );
	Q_FOREACH ( const QgsAction &action, actions )
	{
	if ( !action.runable() )
	continue;
	if ( !mLayer->isEditable() && action.isEnabledOnlyWhenEditable() )
	continue;
	QgsFeature &feat = const_cast<QgsFeature &>( *dialog->feature() );
	QgsFeatureAction *a = new QgsFeatureAction( action.name(), feat, mLayer, action.id(), -1, dialog );
	dialog->addAction( a );
	connect( a, &QAction::triggered, a, &QgsFeatureAction::execute );
	QAbstractButton *pb = dialog->findChild<QAbstractButton *>( action.name() );
	if ( pb )
	connect( pb, &QAbstractButton::clicked, a, &QgsFeatureAction::execute );
	}
	}
	return dialog;
	}

, into a separate class that inherits from @QDialog@ and simply returns the user's description of a new feature (or perhaps the feature itself) to its caller. That would provide the flexibility to use the dialog in multiple contexts and a straightforward way of specifying things like the dialog's parent and modality when important.

[qgib]

Author Name: Giovanni Manghi (@gioman)

---

It seems you could provide a patch via a pull request on github?

Simon South wrote:

This problem still occurs when trying to add related features (through a many-to-many relation) via the attribute table. It happens because the "Add Feature" dialog is "shown by calling its @exec()@ method":

QGIS/src/app/qgsfeatureaction.cpp

Line 251 in 1192f94

dialog->exec();

, effectively "making the dialog application-modal":https://doc.qt.io/qt-5/qdialog.html#modal-dialogs and preventing the user from interacting with the main window while it is open (so the feature-identification tool cannot function). Issue #19596 addressed the same problem occurring outside this specific context.

For the courageous, a simple workaround is to apply a similar fix by changing "line 40 of qgsguivectorlayertools.cpp":

QGIS/src/app/qgsguivectorlayertools.cpp

Line 40 in 1192f94

bool added = a.addFeature( defaultValues );

from

[...]

to

[...]

which effectively makes the dialog non-modal and allows the feature-identification tool to function normally.

This isn't really a solution, though:

• There's no parent-child relationship between the attribute table and the "Add Feature" dialog (nor any obvious way of specifying one presently), so one window or the other tends to get lost behind the application.

• Along the same lines, there's no way to specify "window modality":https://doc.qt.io/qt-5/qt.html#WindowModality-enum for the dialog, which is probably what's really needed here.

• The user is no longer blocked from clicking on the attribute table's controls while the dialog is open, so it may be possible to get the application or the layer in an inconsistent state.

I think a "proper" fix would involve refactoring the "Add Feature" dialog's logic, currently spread across "QgsFeatureAction::addFeature()":

QGIS/src/app/qgsfeatureaction.cpp

Lines 168 to 256 in 1192f94

	bool QgsFeatureAction::addFeature( const QgsAttributeMap &defaultAttributes, bool showModal, QgsExpressionContextScope *scope SIP_TRANSFER )
	{
	if ( !mLayer || !mLayer->isEditable() )
	return false;
	QgsSettings settings;
	bool reuseLastValues = settings.value( QStringLiteral( "qgis/digitizing/reuseLastValues" ), false ).toBool();
	QgsDebugMsg( QString( "reuseLastValues: %1" ).arg( reuseLastValues ) );
	QgsFields fields = mLayer->fields();
	QgsAttributeMap initialAttributeValues;
	for ( int idx = 0; idx < fields.count(); ++idx )
	{
	if ( defaultAttributes.contains( idx ) )
	{
	initialAttributeValues.insert( idx, defaultAttributes.value( idx ) );
	}
	else if ( reuseLastValues && sLastUsedValues.contains( mLayer ) && sLastUsedValues[ mLayer ].contains( idx ) )
	{
	initialAttributeValues.insert( idx, sLastUsedValues[ mLayer ][idx] );
	}
	}
	// create new feature template - this will initialize the attributes to valid values, handling default
	// values and field constraints
	QgsExpressionContext context = mLayer->createExpressionContext();
	if ( scope )
	context.appendScope( scope );
	QgsFeature newFeature = QgsVectorLayerUtils::createFeature( mLayer, mFeature->geometry(), initialAttributeValues,
	&context );
	*mFeature = newFeature;
	//show the dialog to enter attribute values
	//only show if enabled in settings and layer has fields
	bool isDisabledAttributeValuesDlg = ( fields.count() == 0 ) || settings.value( QStringLiteral( "qgis/digitizing/disable_enter_attribute_values_dialog" ), false ).toBool();
	// override application-wide setting with any layer setting
	switch ( mLayer->editFormConfig().suppress() )
	{
	case QgsEditFormConfig::SuppressOn:
	isDisabledAttributeValuesDlg = true;
	break;
	case QgsEditFormConfig::SuppressOff:
	isDisabledAttributeValuesDlg = false;
	break;
	case QgsEditFormConfig::SuppressDefault:
	break;
	}
	if ( isDisabledAttributeValuesDlg )
	{
	mLayer->beginEditCommand( text() );
	mFeatureSaved = mLayer->addFeature( *mFeature );
	if ( mFeatureSaved )
	{
	mLayer->endEditCommand();
	mLayer->triggerRepaint();
	}
	else
	{
	mLayer->destroyEditCommand();
	}
	}
	else
	{
	QgsAttributeDialog *dialog = newDialog( false );
	// delete the dialog when it is closed
	dialog->setAttribute( Qt::WA_DeleteOnClose );
	dialog->setMode( QgsAttributeForm::AddFeatureMode );
	dialog->setEditCommandMessage( text() );
	connect( dialog->attributeForm(), &QgsAttributeForm::featureSaved, this, &QgsFeatureAction::onFeatureSaved );
	if ( !showModal )
	{
	setParent( dialog ); // keep dialog until the dialog is closed and destructed
	dialog->show();
	mFeature = nullptr;
	return true;
	}
	dialog->exec();
	}
	// Will be set in the onFeatureSaved SLOT
	return mFeatureSaved;
	}

and "QgsFeatureAction::newDialog()":

QGIS/src/app/qgsfeatureaction.cpp

Lines 52 to 101 in 1192f94

	QgsAttributeDialog *QgsFeatureAction::newDialog( bool cloneFeature )
	{
	QgsFeature *f = cloneFeature ? new QgsFeature( *mFeature ) : mFeature;
	QgsAttributeEditorContext context;
	QgsDistanceArea myDa;
	myDa.setSourceCrs( mLayer->crs(), QgsProject::instance()->transformContext() );
	myDa.setEllipsoid( QgsProject::instance()->ellipsoid() );
	context.setDistanceArea( myDa );
	context.setVectorLayerTools( QgisApp::instance()->vectorLayerTools() );
	context.setMapCanvas( QgisApp::instance()->mapCanvas() );
	context.setFormMode( QgsAttributeEditorContext::StandaloneDialog );
	QgsAttributeDialog *dialog = new QgsAttributeDialog( mLayer, f, cloneFeature, parentWidget(), true, context );
	dialog->setWindowFlags( dialog->windowFlags() | Qt::Tool );
	dialog->setObjectName( QStringLiteral( "featureactiondlg:%1:%2" ).arg( mLayer->id() ).arg( f->id() ) );
	QList<QgsAction> actions = mLayer->actions()->actions( QStringLiteral( "Feature" ) );
	if ( !actions.isEmpty() )
	{
	dialog->setContextMenuPolicy( Qt::ActionsContextMenu );
	QAction *a = new QAction( tr( "Run actions" ), dialog );
	a->setEnabled( false );
	dialog->addAction( a );
	Q_FOREACH ( const QgsAction &action, actions )
	{
	if ( !action.runable() )
	continue;
	if ( !mLayer->isEditable() && action.isEnabledOnlyWhenEditable() )
	continue;
	QgsFeature &feat = const_cast<QgsFeature &>( *dialog->feature() );
	QgsFeatureAction *a = new QgsFeatureAction( action.name(), feat, mLayer, action.id(), -1, dialog );
	dialog->addAction( a );
	connect( a, &QAction::triggered, a, &QgsFeatureAction::execute );
	QAbstractButton *pb = dialog->findChild<QAbstractButton *>( action.name() );
	if ( pb )
	connect( pb, &QAbstractButton::clicked, a, &QgsFeatureAction::execute );
	}
	}
	return dialog;
	}

, into a separate class that inherits from @QDialog@ and simply returns the user's description of a new feature (or perhaps the feature itself) to its caller. That would provide the flexibility to use the dialog in multiple contexts and a straightforward way of specifying things like the dialog's parent and modality when important.

[qgib]

Author Name: Simon South (@simonsouth)

---

Giovanni Manghi wrote:

It seems you could provide a patch via a pull request on github?

Sure, but I don't consider the workaround I posted to be a suitable fix. The code needs to be refactored and this could be a significant undertaking, considering the "Add Feature" dialog is used in multiple places.

Does the idea of pulling out the UI-related portions of @QgsFeatureAction@ into a separate @QDialog@ subclass sound reasonable to you? If I could demonstrate this working, is it something the QGIS team might accept?

[qgib]

Author Name: Giovanni Manghi (@gioman)

---

Simon South wrote:

Giovanni Manghi wrote:

It seems you could provide a patch via a pull request on github?

Sure, but I don't consider the workaround I posted to be a suitable fix. The code needs to be refactored and this could be a significant undertaking, considering the "Add Feature" dialog is used in multiple places.

Does the idea of pulling out the UI-related portions of @QgsFeatureAction@ into a separate @QDialog@ subclass sound reasonable to you? If I could demonstrate this working, is it something the QGIS team might accept?

your suggestion are valuable, but to make them gain traction you should make a pull request or eventually comment the existing code, but things better done on github. Thanks!

[qgib]

Author Name: Giovanni Manghi (@gioman)

---

End of life notice: QGIS 2.18 LTR

Source:
http://blog.qgis.org/2019/03/09/end-of-life-notice-qgis-2-18-ltr/

---

• resolution was changed from to end of life
• status_id was changed from Open to Closed