Skip to content

Fix app crash on selecting unavailable layer, Add info message #4718

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
xkello wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Fix app crash on selecting unavailable layer, Add info message #4718

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion app/activeproject.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -300,7 +300,10 @@ bool ActiveProject::validateProject()
if ( !layer->isValid() )
{
errorsFound = true;
CoreUtils::log( QStringLiteral( "Project load" ), QStringLiteral( "Invalid layer %1" ).arg( layer->name() ) );
QgsError layerError = layer->error();
const QString reason = !layerError.isEmpty() ? layerError.summary()
: ( layer->dataProvider() ? layer->dataProvider()->error().summary() : QString() );
CoreUtils::log( QStringLiteral( "Project load" ), QStringLiteral( "Invalid layer %1: %2" ).arg( layer->name(), reason ) );
Comment on lines +303 to +306

@tomasMizera tomasMizera Sep 29, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's not nest multiple ternary operators. They become quite hard to read.

Try comparing it with this

Suggested change
QgsError layerError = layer->error();
const QString reason = !layerError.isEmpty() ? layerError.summary()
: ( layer->dataProvider() ? layer->dataProvider()->error().summary() : QString() );
CoreUtils::log( QStringLiteral( "Project load" ), QStringLiteral( "Invalid layer %1: %2" ).arg( layer->name(), reason ) );
QgsError layerError = layer->error();
QString reason;
if ( !layerError.isEmpty() )
{
reason = layerError.summary();
}
else if ( layer->dataProvider() )
{
reason = layer->dataProvider()->error().summary();
}
CoreUtils::log( QStringLiteral( "Project load" ), QStringLiteral( "Invalid layer %1: %2" ).arg( layer->name(), reason ) );

@tomasMizera tomasMizera Sep 29, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe our code convention says that there can be only one ternary, not multiple ones nested.

emit reportIssue( tr( "Layer" ) + ": " + layer->name(), tr( "Unable to load source " ) + ": " + layer->publicSource() );
}
else
Expand Down
21 changes: 21 additions & 0 deletions app/layer/layerdetaildata.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
#include "qgslegendsettings.h"
#include "qgslayertreemodel.h"

#include "coreutils.h"
#include "mmstyle.h"

LayerDetailData::LayerDetailData( QObject *parent )
Expand All @@ -39,6 +40,7 @@ void LayerDetailData::setLayerTreeNode( QgsLayerTreeNode *newLayerTreeNode )
mName = QString();
mLayerId = QString();
mIsVisible = false;
mIsValid = false;
mIsSpatial = false;
mIsVectorLayer = false;

Expand All @@ -50,6 +52,7 @@ void LayerDetailData::setLayerTreeNode( QgsLayerTreeNode *newLayerTreeNode )
emit nameChanged( mName );
emit layerIdChanged( mLayerId );
emit isVisibleChanged( mIsVisible );
emit isValidChanged( mIsValid );
emit isSpatialChanged( mIsSpatial );
emit isVectorLayerChanged( mIsVectorLayer );
emit vectorLayerChanged( nullptr );
Expand All @@ -64,6 +67,7 @@ void LayerDetailData::setLayerTreeNode( QgsLayerTreeNode *newLayerTreeNode )
emit nameChanged( mName );
emit layerIdChanged( mLayerId );
emit isVisibleChanged( mIsVisible );
emit isValidChanged( mIsValid );
emit isSpatialChanged( mIsSpatial );
emit isVectorLayerChanged( mIsVectorLayer );
emit vectorLayerChanged( nullptr );
Expand All @@ -80,6 +84,18 @@ void LayerDetailData::setLayerTreeNode( QgsLayerTreeNode *newLayerTreeNode )
mLayerId = nodeLayer->layerId();
emit layerIdChanged( mLayerId );

mIsValid = nodeLayer->layer()->isValid();
emit isValidChanged( mIsValid );

// Log invalid layers
if ( !mIsValid )
{
QgsError layerError = nodeLayer->layer()->error();
const QString reason = !layerError.isEmpty() ? layerError.summary()
: ( nodeLayer->layer()->dataProvider() ? nodeLayer->layer()->dataProvider()->error().summary() : QString() );
CoreUtils::log( QStringLiteral( "Layer detail" ), QStringLiteral( "Opened invalid layer %1: %2" ).arg( mName, reason ) );
}

mIsSpatial = nodeLayer->layer()->isSpatial();
emit isSpatialChanged( mIsSpatial );

Expand Down Expand Up @@ -192,6 +208,11 @@ bool LayerDetailData::isVisible() const
return mIsVisible;
}

bool LayerDetailData::isValid() const
{
return mIsValid;
}

QgsLegendRenderer *LayerDetailData::legendRenderer() const
{
return mLegendRenderer.get();
Expand Down
4 changes: 4 additions & 0 deletions app/layer/layerdetaildata.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ class LayerDetailData : public QObject
Q_PROPERTY( QString name READ name NOTIFY nameChanged )
Q_PROPERTY( QString layerId READ layerId NOTIFY layerIdChanged )
Q_PROPERTY( bool isVisible READ isVisible NOTIFY isVisibleChanged )
Q_PROPERTY( bool isValid READ isValid NOTIFY isValidChanged )
Q_PROPERTY( bool isSpatial READ isSpatial NOTIFY isSpatialChanged )
Q_PROPERTY( bool isVectorLayer READ isVectorLayer NOTIFY isVectorLayerChanged )
Q_PROPERTY( QgsVectorLayer *vectorLayer READ vectorLayer NOTIFY vectorLayerChanged )
Expand All @@ -40,6 +41,7 @@ class LayerDetailData : public QObject
void setLayerTreeNode( QgsLayerTreeNode *newLayerTreeNode );

bool isVisible() const;
bool isValid() const;
bool isSpatial() const;
bool isVectorLayer() const;
const QString &name() const;
Expand All @@ -54,6 +56,7 @@ class LayerDetailData : public QObject
signals:
void isSpatialChanged( bool showLegned );
void isVisibleChanged( bool isVisible );
void isValidChanged( bool isValid );
void isVectorLayerChanged( bool isVectorLayer );
void nameChanged( const QString &name );
void layerIdChanged( const QString &layerId );
Expand All @@ -69,6 +72,7 @@ class LayerDetailData : public QObject
QString mLayerId;
bool mIsSpatial = false;
bool mIsVisible = false;
bool mIsValid = false;
bool mIsVectorLayer = false;
};

Expand Down
2 changes: 1 addition & 1 deletion app/layerfeaturesmodel.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ void LayerFeaturesModel::populate()
{
cancelPendingRequests();

if ( mLayer && mLayer->dataProvider() )
if ( mLayer && mLayer->isValid() && mLayer->dataProvider() )
{
mFetchingResults = true;
emit fetchingResultsChanged( mFetchingResults );
Expand Down
30 changes: 30 additions & 0 deletions app/qml/layers/MMLayerDetailPage.qml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,31 @@ Page {
interactive: layerDetailData.isVectorLayer
}

Component {
id: layerUnavailablePageComponent

MMComponents.MMPage {
id: layerUnavailablePage

pageHeader.title: layerDetailData.name
onBackClicked: root.closePage()

pageContent: Item {
width: parent.width
height: parent.height

MMComponents.MMMessage {
anchors.centerIn: parent
width: parent.width - 2 * __style.margin20

image: __style.negativeMMSymbolImage
title: qsTr( "Layer unavailable" )
description: qsTr( "This layer couldn't be loaded. Check your connection or contact your administrator." )
}
}
}
}

Component {
id: layerDetailPageComponent

Expand Down Expand Up @@ -283,6 +308,11 @@ Page {
// * there is an exception, we do not want to show features of the map sketches layer
//

if ( !layerDetailData.isValid ) {
content.addItem( layerUnavailablePageComponent.createObject( content ) )
return
}

const isSketchingLayer = layerDetailData.layerId === __activeProject.mapSketchesLayerId()

if ( layerDetailData.isVectorLayer && !isSketchingLayer ) {
Expand Down
Loading