dashArray: "none" in canvas renderer leaks context state between layers

[glen-nu]

How to reproduce

• Leaflet version I'm using: 1.3.4
• Browser (with version) I'm using: Chrome 69
• OS/Platform (with version) I'm using: Ubuntu 14.04
• Using react-leaflet

What behaviour I'm expecting and which behaviour I'm seeing

I currently run into a situation with Canvas renderer where my polylines and polygons are not rendered with the correct dashArray. Sometimes lines with a specified dashArray are rendered without one and lines without one are rendered with one. I wish I could reproduce this with a small example but I can't seem to figure out how to reproduce it exactly (I'm using react-leaflet 1.1.4 for what it's worth).

What seems to be happening is that the settings in

Leaflet/src/layer/vector/Canvas.js

Lines 314 to 334 in da6f20d

	_fillStroke: function (ctx, layer) {
	var options = layer.options;
	if (options.fill) {
	ctx.globalAlpha = options.fillOpacity;
	ctx.fillStyle = options.fillColor || options.color;
	ctx.fill(options.fillRule || 'evenodd');
	}
	if (options.stroke && options.weight !== 0) {
	if (ctx.setLineDash) {
	ctx.setLineDash(layer.options && layer.options._dashArray || []);
	}
	ctx.globalAlpha = options.opacity;
	ctx.lineWidth = options.weight;
	ctx.strokeStyle = options.color;
	ctx.lineCap = options.lineCap;
	ctx.lineJoin = options.lineJoin;
	ctx.stroke();
	}
	},

are being leaked between successive calls. From what I can tell, Canvas renderer attempts to isolate the state between calls in _draw using ctx.save() and ctx.restore(). However in my application, multiple calls to fillStroke are made in a single _draw call (which is called exactly once per map update). If I use ctx.save() and ctx.restore() in _fillStroke, the problem goes away

[ghybs]

Hi,

Thank you for using Leaflet.

Please can you provide a Plunker that reproduces that issue, without any wrapper library?
(no react, etc.)

[glen-nu]

With a bit of digging, I found what the issue is!

https://next.plnkr.co/edit/hhRnU8AT3rQQMvBt

Somewhere in our code, we're passing 'none' as our options.dashArray. This is parsed as [NaN] in _updateDashArray. This actually makes it all the way to the canvas rendering context and when the context receives an invalid argument for setLineDash, it ends up keeping the old state.

This is easily fixable from our end by simply not passing invalid values to dashArray. However this could still be considered a bug because this canvas and SVG render inconsistently for the same value of dashArray.

PS: I found another issue (unrelated?), where the dashArray wouldn't work if I omitted the commas i.e. '5 5' instead of 5, 5. I haven't been able to replicate this yet however.

[ghybs]

Thank you for your investigation! 👍

As for the issue with space-separated values, see #6277 (fix should be in Leaflet 1.3.4)

[ghybs]

I confirm that exiting _updateDashArray in case dashArray option is "none" (or illegal values) avoids the bug:
https://next.plnkr.co/edit/QBAL7SQ4dkvpXqZd