Skip to content
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.

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

Leaflet canvas remove() bug #6030

Closed
louMoxy opened this issue Jan 26, 2018 · 1 comment
Closed

Leaflet canvas remove() bug #6030

louMoxy opened this issue Jan 26, 2018 · 1 comment

Comments

@louMoxy
Copy link
Contributor

louMoxy commented Jan 26, 2018

I am removing a canvas map using map.remove() and creating a new canvas map and I am getting the error:
Uncaught TypeError: Cannot read property 'clearRect' of undefined

I have created a codepen to show the issue, just uncomment the mymap.remove(); and comment out the setTimeout and you will see the error.
https://codepen.io/moxy/pen/QaeKdp

const mapId = document.getElementById("leaflet");
const removeButton = document.getElementById("remove");

var mymap = L.map(mapId, {
  renderer: L.canvas(),
  layers: [
		new L.TileLayer('//{s}.tile.openstreetmap.org/{z}/{x}/{y}.png', {
			'attribution': 'Map data © <a href="http://openstreetmap.org">OpenStreetMap</a> contributors'
		})
	]
}).setView([51.505, -0.09], 13);

var circle = L.circle([51.508, -0.11], {
  color: "red",
  fillColor: "#f03",
  fillOpacity: 0.5,
  radius: 500
}).addTo(mymap);

// setTimeout(() => mymap.remove(), 10); // Works as expected
mymap.remove();  // Throws an error
//removeButton.addEventListener("click", function(event) {
//  mymap.remove(); // Works as expected
// });
@IvanSanchez
Copy link
Member

IvanSanchez commented Jan 26, 2018

This is a race condition. What normally goes on here is that a canvas renderer triggers a requestAnimationFrame over at

this._redrawRequest = this._redrawRequest || Util.requestAnimFrame(this._redraw, this);
, so that everything is drawn during the next frame.

In this case, the map (or the canvas renderer) is removed during that request for the animation frame, so the delayed call to _redraw is trying to get the drawing context, which has already been destroyed.

There are several approaches to this, but I feel that the cleanest is to call Util.cancelAnimFrame (this._requestRedraw) in L.Canvas._destroyContainer, next to

delete this._ctx;

This bug seems that can be triggered programatically, so if anybody wants to fix this, I encourage to do Test-Driven Development for this one. Add a unit test over at https://github.com/Leaflet/Leaflet/blob/master/spec/suites/layer/vector/CanvasSpec.js , make sure it fails with the same error message, then fix the bug, then check if the tests pass.

louMoxy pushed a commit to CityScience/Leaflet that referenced this issue Jan 29, 2018
Cancel animation frame before removing the map.
This fixes the bug where the the delayed call to _redraw is trying to get the drawing context which has already been destroyed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants