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

Arearange: zoom out to boosted mode caused error in js #7557

Closed
pawelfus opened this Issue Dec 19, 2017 · 2 comments

Comments

Projects
None yet
2 participants
@pawelfus
Contributor

pawelfus commented Dec 19, 2017

Error:

Version 6.0.4:

highcharts.src.js:4274 Uncaught TypeError: Cannot read property 'setAttribute' of undefined
    at H.SVGElement._defaultSetter (highcharts.src.js:4274)
    at H.SVGElement.eachAttribute (highcharts.src.js:2980)
    at H.objectEach (highcharts.src.js:1808)
    at H.SVGElement.attr (highcharts.src.js:2952)
    at H.SVGElement.removeClass (highcharts.src.js:3094)
    at object.setState (highcharts.src.js:33738)
    at object.setState (highcharts-more.src.js:1496)
    at highcharts.src.js:17619
    at Array.forEach (<anonymous>)
    at H.each (highcharts.src.js:1790)
_defaultSetter @ highcharts.src.js:4274
eachAttribute @ highcharts.src.js:2980
H.objectEach @ highcharts.src.js:1808
attr @ highcharts.src.js:2952
removeClass @ highcharts.src.js:3094
setState @ highcharts.src.js:33738
setState @ highcharts-more.src.js:1496
(anonymous) @ highcharts.src.js:17619
H.each @ highcharts.src.js:1790
runPointActions @ highcharts.src.js:17617
onContainerMouseMove @ highcharts.src.js:18059
container.onmousemove @ highcharts.src.js:18169

Live demo with steps to reproduce

Demo (and demo github vs current)

It looks like problem with not cleared point from chart.hoverPoints. This is what actually happens:

  • zoom in until we see markers
  • reset zoom and don't move mouse
  • you can still observe markers from previously zoomed range (shouldn't that be cleaned in reset zoom?)
  • original points (series.points) are correct, but in chart.hoverPoints we still have a point from the old range - but point.graphic was destroyed (nullified points). Interesting part: that marker is still visible..
@pawelfus

This comment has been minimized.

Show comment
Hide comment
@pawelfus

pawelfus Dec 19, 2017

Contributor

Found the problem, it's caused by top-graphic which is not cleared out:

if (points) {
for (i = 0; i < points.length; i = i + 1) {
point = points[i];
if (point && point.graphic) {
point.graphic = point.graphic.destroy();
}
}
}

Could we replace this with destroyElements method?

	if (points) {
		for (i = 0; i < points.length; i = i + 1) {
			point = points[i];
			if (point && point.destroyElements) {
				point.destroyElements();
			}
		}
	}

So it will be safe to remove all associated elements to a point? I'm not sure about the performance.. it's called only for SVG-based points, so shouldn't affect Canvas solution, right? On the other hand, we have there more checks than simple if (point && point.graphic) { ... }:

destroyElements: function () {
var point = this,
props = ['graphic', 'dataLabel', 'dataLabelUpper', 'connector', 'shadowGroup'],
prop,
i = 6;
while (i--) {
prop = props[i];
if (point[prop]) {
point[prop] = point[prop].destroy();
}
}
},

@cvasseng @TorsteinHonsi

Contributor

pawelfus commented Dec 19, 2017

Found the problem, it's caused by top-graphic which is not cleared out:

if (points) {
for (i = 0; i < points.length; i = i + 1) {
point = points[i];
if (point && point.graphic) {
point.graphic = point.graphic.destroy();
}
}
}

Could we replace this with destroyElements method?

	if (points) {
		for (i = 0; i < points.length; i = i + 1) {
			point = points[i];
			if (point && point.destroyElements) {
				point.destroyElements();
			}
		}
	}

So it will be safe to remove all associated elements to a point? I'm not sure about the performance.. it's called only for SVG-based points, so shouldn't affect Canvas solution, right? On the other hand, we have there more checks than simple if (point && point.graphic) { ... }:

destroyElements: function () {
var point = this,
props = ['graphic', 'dataLabel', 'dataLabelUpper', 'connector', 'shadowGroup'],
prop,
i = 6;
while (i--) {
prop = props[i];
if (point[prop]) {
point[prop] = point[prop].destroy();
}
}
},

@cvasseng @TorsteinHonsi

@TorsteinHonsi

This comment has been minimized.

Show comment
Hide comment
@TorsteinHonsi

TorsteinHonsi Dec 20, 2017

Collaborator

Could we replace this with destroyElements method?

Yes, let's do that! As for performance, this only happens when switching from non-boost to boost mode, so we're limited to the boostThreshold.

Will you do the fix?

Collaborator

TorsteinHonsi commented Dec 20, 2017

Could we replace this with destroyElements method?

Yes, let's do that! As for performance, this only happens when switching from non-boost to boost mode, so we're limited to the boostThreshold.

Will you do the fix?

pawelfus added a commit that referenced this issue Dec 20, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment