BUGFIX - Cannot read property 'attributionControl' of null #5202

Merged
merged 2 commits into from Dec 21, 2016

Projects

None yet

4 participants

@hagai26
Contributor
hagai26 commented Dec 17, 2016 edited

I get this exception when tried to open & close a marker popup immediately:
TypeError: Cannot read property 'attributionControl' of null
at NewClass._layerAdd (http://localhost:8100/build/main.js:20926:39)
at NewClass.whenReady (http://localhost:8100/build/main.js:20566:13)
at NewClass.addLayer (http://localhost:8100/build/main.js:20982:8)
at NewClass.openPopup (http://localhost:8100/build/main.js:24186:15)
at NewClass.openPopup (http://localhost:8100/build/main.js:24292:14)
at NewClass._openPopup (http://localhost:8100/build/main.js:24367:9)
at NewClass.fire (http://localhost:8100/build/main.js:17552:11)
at NewClass._fireDOMEvent (http://localhost:8100/build/main.js:20541:15)
at NewClass._handleDOMEvent (http://localhost:8100/build/main.js:20500:8)
at HTMLDivElement.handler (http://localhost:8100/build/main.js:21164:14)

It runs the following code:

_layerAdd: function (e) {
 var map = e.target;

 // check in case layer gets added and then removed before the map is ready
 if (!map.hasLayer(this)) { return; }

 this._map = map;
 this._zoomAnimated = map._zoomAnimated;

 if (this.getEvents) {
  var events = this.getEvents();
  map.on(events, this);
  this.once('remove', function () {
   map.off(events, this);
  }, this);
 }

 this.onAdd(map);

 if (this.getAttribution && this._map.attributionControl) {
  this._map.attributionControl.addAttribution(this.getAttribution());
 }

 this.fire('add');
 map.fire('layeradd', {layer: this});
}

but the line: this.onAdd(map); can make this._map be undefind, like in:

...
layer._map = layer._mapToAdd = null;

so I replaced it with map instead which must be defined because it is used afterwards as well.

@hagai26 hagai26 'this._map' might be undefined -> using 'map' instead
I get this exception when tried to open & close a marker popup immediately:
TypeError: Cannot read property 'attributionControl' of null
    at NewClass._layerAdd (http://localhost:8100/build/main.js:20926:39)
    at NewClass.whenReady (http://localhost:8100/build/main.js:20566:13)
    at NewClass.addLayer (http://localhost:8100/build/main.js:20982:8)
    at NewClass.openPopup (http://localhost:8100/build/main.js:24186:15)
    at NewClass.openPopup (http://localhost:8100/build/main.js:24292:14)
    at NewClass._openPopup (http://localhost:8100/build/main.js:24367:9)
    at NewClass.fire (http://localhost:8100/build/main.js:17552:11)
    at NewClass._fireDOMEvent (http://localhost:8100/build/main.js:20541:15)
    at NewClass._handleDOMEvent (http://localhost:8100/build/main.js:20500:8)
    at HTMLDivElement.handler (http://localhost:8100/build/main.js:21164:14)

It runs the following code:
_layerAdd: function (e) {
 var map = e.target;

 // check in case layer gets added and then removed before the map is ready
 if (!map.hasLayer(this)) { return; }

 this._map = map;
 this._zoomAnimated = map._zoomAnimated;

 if (this.getEvents) {
  var events = this.getEvents();
  map.on(events, this);
  this.once('remove', function () {
   map.off(events, this);
  }, this);
 }

 this.onAdd(map);

 if (this.getAttribution && this._map.attributionControl) {
  this._map.attributionControl.addAttribution(this.getAttribution());
 }

 this.fire('add');
 map.fire('layeradd', {layer: this});
}

but the line:  this.onAdd(map); can make this._map be undefind, like in:
...
layer._map = layer._mapToAdd = null;

so I replaced it with map instead which must be defined because it is used afterwards as well.
5da7d64
@IvanSanchez
Member

@hagai26 What's the code that triggers this? (I mean "can you show us the way you are opening and closing a popup immediately?") It would be nice to use it to build a unit test for this.

@hagai26
Contributor
hagai26 commented Dec 18, 2016

Sure.
I needed the popup feature only for autoPan after click. I didn't need the popup itself, so I did this:

this.map.on('popupopen', (e: L.LeafletPopupEvent) => {
	const marker = e.popup._source;
	marker.closePopup();
});

thus, a popup is closed after it is opened.
If I commented out 'marker.closePopup();', there is no exception.
With 'marker.closePopup();' there is the exception above.

@perliedman

Looks good, although it's really an edge case we're fixing here.

I'm guessing the code for attribution uses this._map since it was moved from TileLayer. Using map is shorter, clearer and also fixes the issue at hand.

@IvanSanchez IvanSanchez merged commit 4e58391 into Leaflet:master Dec 21, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@IvanSanchez IvanSanchez added a commit that referenced this pull request Dec 21, 2016
@IvanSanchez IvanSanchez Added unit test for #5202 b8b7281
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment