From fd5411d74b7eb9821ad6581ac17fd6096d5f5ee7 Mon Sep 17 00:00:00 2001
From: Yohan Boniface <yb@enix.org>
Date: Sat, 14 Nov 2015 20:41:53 +0100
Subject: [PATCH] Fix popup toggle on marker click (fix #3992)

Issue was:
- popup toggling is made on 'click' event
- map listen to 'preclick' to close any open popup
- at second user click, 'preclick' has been fired, then popup
  closed, so the popup toggling was reopening it
- this was not an issue before 0d3448d494b4c6191f20282c679ecefa27bb3c50
  because 'preclick' on the marker was not bubbled to the map
- also the unittest covering this use case was too much coupled with
  the use case with calling marker.fire instead of simulating user
  click on the marker icon
---
 spec/suites/layer/PopupSpec.js | 17 +++++++++--------
 src/layer/Popup.js             |  2 ++
 src/map/Map.js                 |  2 +-
 3 files changed, 12 insertions(+), 9 deletions(-)

diff --git a/spec/suites/layer/PopupSpec.js b/spec/suites/layer/PopupSpec.js
index b3688c61..119550c5 100644
--- a/spec/suites/layer/PopupSpec.js
+++ b/spec/suites/layer/PopupSpec.js
@@ -6,10 +6,15 @@ describe('Popup', function () {
 		c = document.createElement('div');
 		c.style.width = '400px';
 		c.style.height = '400px';
+		document.body.appendChild(c);
 		map = new L.Map(c);
 		map.setView(new L.LatLng(55.8, 37.6), 6);
 	});
 
+	afterEach(function () {
+		document.body.removeChild(c);
+	});
+
 	it("closes on map click when map has closePopupOnClick option", function () {
 		map.options.closePopupOnClick = true;
 
@@ -51,22 +56,18 @@ describe('Popup', function () {
 		map.addLayer(marker);
 
 		marker.bindPopup('Popup1');
-		map.options.closePopupOnClick = true;
+		expect(map.hasLayer(marker._popup)).to.be(false);
 
 		// toggle open popup
-		marker.fire('click', {
-			latlng: new L.LatLng(55.8, 37.6)
-		});
+		happen.click(marker._icon);
 		expect(map.hasLayer(marker._popup)).to.be(true);
 
 		// toggle close popup
-		marker.fire('click', {
-			latlng: new L.LatLng(55.8, 37.6)
-		});
+		happen.click(marker._icon);
 		expect(map.hasLayer(marker._popup)).to.be(false);
 	});
 
-	it("it should use a popup with a fuction as content with a FeatureGroup", function () {
+	it("it should use a popup with a function as content with a FeatureGroup", function () {
 		var marker1 = new L.Marker(new L.LatLng(55.8, 37.6));
 		var marker2 = new L.Marker(new L.LatLng(54.6, 38.2));
 		var group = new L.FeatureGroup([marker1, marker2]).addTo(map);
diff --git a/src/layer/Popup.js b/src/layer/Popup.js
index bfcbfe2e..e52d8d95 100644
--- a/src/layer/Popup.js
+++ b/src/layer/Popup.js
@@ -57,6 +57,7 @@ L.Popup = L.Layer.extend({
 
 		if (this._source) {
 			this._source.fire('popupopen', {popup: this}, true);
+			this._source.on('preclick', L.DomEvent.stopPropagation);
 		}
 	},
 
@@ -77,6 +78,7 @@ L.Popup = L.Layer.extend({
 
 		if (this._source) {
 			this._source.fire('popupclose', {popup: this}, true);
+			this._source.off('preclick', L.DomEvent.stopPropagation);
 		}
 	},
 
diff --git a/src/map/Map.js b/src/map/Map.js
index bccefdee..0a4cd55b 100644
--- a/src/map/Map.js
+++ b/src/map/Map.js
@@ -668,7 +668,6 @@ L.Map = L.Evented.extend({
 	_handleDOMEvent: function (e) {
 		if (!this._loaded || L.DomEvent._skipped(e)) { return; }
 
-		// find the layer the event is propagating from and its parents
 		var type = e.type === 'keypress' && e.keyCode === 13 ? 'click' : e.type;
 
 		if (e.type === 'click') {
@@ -690,6 +689,7 @@ L.Map = L.Evented.extend({
 
 		if (e._stopped) { return; }
 
+		// Find the layer the event is propagating from and its parents.
 		targets = (targets || []).concat(this._findEventTargets(e, type));
 
 		if (!targets.length) { return; }