From 388cf157fc5d7c1849bb479e4307f593509413cd Mon Sep 17 00:00:00 2001
From: Jeroen De Dauw <jeroendedauw@proton.me>
Date: Mon, 6 Apr 2026 23:01:41 +0200
Subject: [PATCH] Fix LeafletEditor.remove() crash when no edits were made

self.saveButton is only created after the user makes a change
(via _showSaveButton). Calling remove() before any edits crashes
with TypeError because self.saveButton is undefined, which also
prevents geoJsonLayer cleanup from running.

Add null guard and QUnit tests that verify actual layer cleanup
(layerCount decreases), which also serves as mutation verification:
without the fix, the TypeError prevents geoJsonLayer.remove() from
executing, so layer count stays the same and the test fails.

Closes #888

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
---
 extension.json                        |  1 +
 resources/leaflet/LeafletEditor.js    |  4 +-
 tests/js/leaflet/LeafletEditorTest.js | 58 +++++++++++++++++++++++++++
 3 files changed, 62 insertions(+), 1 deletion(-)
 create mode 100644 tests/js/leaflet/LeafletEditorTest.js

diff --git a/extension.json b/extension.json
index 834b0c4f..e2480e1a 100644
--- a/extension.json
+++ b/extension.json
@@ -348,6 +348,7 @@
 			"leaflet/GeoJsonTest.js",
 			"leaflet/FeatureBuilderTest.js",
 			"leaflet/JQueryLeafletTest.js",
+			"leaflet/LeafletEditorTest.js",
 			"MapSaverTest.js"
 		],
 		"dependencies": [
diff --git a/resources/leaflet/LeafletEditor.js b/resources/leaflet/LeafletEditor.js
index 68315f81..3d118b4c 100644
--- a/resources/leaflet/LeafletEditor.js
+++ b/resources/leaflet/LeafletEditor.js
@@ -231,7 +231,9 @@
 
 		self.remove = function() {
 			self.drawControl.remove();
-			self.saveButton.remove();
+			if (self.saveButton) {
+				self.saveButton.remove();
+			}
 			self.geoJsonLayer.remove();
 		};
 
diff --git a/tests/js/leaflet/LeafletEditorTest.js b/tests/js/leaflet/LeafletEditorTest.js
new file mode 100644
index 00000000..3516b7ee
--- /dev/null
+++ b/tests/js/leaflet/LeafletEditorTest.js
@@ -0,0 +1,58 @@
+( function () {
+	'use strict';
+
+	QUnit.module( 'Maps.LeafletEditor', {
+		beforeEach: function () {
+			this.$container = $( '<div>' ).css( { width: '400px', height: '300px' } ).appendTo( '#qunit-fixture' );
+			this.map = L.map( this.$container[ 0 ], { center: [ 52, 5 ], zoom: 10 } );
+			this.mapSaver = new window.maps.MapSaver( 'TestPage' );
+		},
+		afterEach: function () {
+			this.map.remove();
+		}
+	} );
+
+	QUnit.test( 'remove() does not crash when no edits were made', function ( assert ) {
+		var editor = window.maps.leaflet.LeafletEditor(
+			this.map,
+			this.mapSaver
+		);
+
+		editor.initialize( { type: 'FeatureCollection', features: [] } );
+
+		var layerCountBefore = 0;
+		this.map.eachLayer( function () { layerCountBefore++; } );
+
+		editor.remove();
+
+		var layerCountAfter = 0;
+		this.map.eachLayer( function () { layerCountAfter++; } );
+
+		assert.true( layerCountAfter < layerCountBefore, 'GeoJSON layer was removed from the map' );
+	} );
+
+	QUnit.test( 'remove() cleans up after edits were made', function ( assert ) {
+		var editor = window.maps.leaflet.LeafletEditor(
+			this.map,
+			this.mapSaver
+		);
+
+		editor.initialize( { type: 'FeatureCollection', features: [] } );
+
+		// Simulate a created feature to trigger _showSaveButton
+		this.map.fire( L.Draw.Event.CREATED, {
+			layer: L.marker( [ 52, 5 ] )
+		} );
+
+		var layerCountBefore = 0;
+		this.map.eachLayer( function () { layerCountBefore++; } );
+
+		editor.remove();
+
+		var layerCountAfter = 0;
+		this.map.eachLayer( function () { layerCountAfter++; } );
+
+		assert.true( layerCountAfter < layerCountBefore, 'Layers were removed from the map after edits' );
+	} );
+
+}() );