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

Highcharts v5.0.10: Unclipped symbols in meteogram sample #6550

Closed
TorsteinHonsi opened this Issue Apr 3, 2017 · 4 comments

Comments

Projects
None yet
3 participants
@TorsteinHonsi
Collaborator

TorsteinHonsi commented Apr 3, 2017

Expected behaviour

Clipped sprites

Actual behaviour

Sprites from number 10 and up are not clipped

Live demo with steps to reproduce

http://jsfiddle.net/gh/get/library/pure/highcharts/highcharts/tree/master/samples/highcharts/demo/combo-meteogram/

Regression since dcd3737

@zainrafique

This comment has been minimized.

Show comment
Hide comment
@zainrafique

zainrafique Apr 28, 2017

I am still getting this problem. Can you please tell me what is the expected time of next release?

zainrafique commented Apr 28, 2017

I am still getting this problem. Can you please tell me what is the expected time of next release?

@TorsteinHonsi

This comment has been minimized.

Show comment
Hide comment
@TorsteinHonsi

TorsteinHonsi May 2, 2017

Collaborator

It should be released within two to three weeks. In the meantime, here's a drop-in fix that you can use for v5.0.10. View it live on jsFiddle.

// Drop-in fix for Highcharts issue #6550, requires Highcharts v5.0.10.
(function (H) {
	var each = H.each;
	/**
	 * Destroy the element and element wrapper and clear up the DOM and event
	 * hooks.
	 *
	 * @returns {void}
	 */
	H.SVGElement.prototype.destroy = function () {
		var wrapper = this,
			element = wrapper.element || {},
			parentToClean =
				wrapper.renderer.isSVG &&
				element.nodeName === 'SPAN' &&
				wrapper.parentGroup,
			grandParent,
			ownerSVGElement = element.ownerSVGElement,
			i;

		// remove events
		element.onclick = element.onmouseout = element.onmouseover =
			element.onmousemove = element.point = null;
		stop(wrapper); // stop running animations

		if (wrapper.clipPath && ownerSVGElement) {
			// Look for existing references to this clipPath and remove them
			// before destroying the element (#6196).
			each(
				ownerSVGElement.querySelectorAll('[clip-path]'),
				function (el) {
					// Include the closing paranthesis in the test to rule out
					// id's from 10 and above (#6550)
					if (el.getAttribute('clip-path')
							.indexOf(wrapper.clipPath.element.id + ')') > -1) {
						el.removeAttribute('clip-path');
					}
				}
			);
			wrapper.clipPath = wrapper.clipPath.destroy();
		}

		// Destroy stops in case this is a gradient object
		if (wrapper.stops) {
			for (i = 0; i < wrapper.stops.length; i++) {
				wrapper.stops[i] = wrapper.stops[i].destroy();
			}
			wrapper.stops = null;
		}

		// remove element
		wrapper.safeRemoveChild(element);

		/*= if (build.classic) { =*/
		wrapper.destroyShadows();
		/*= } =*/

		// In case of useHTML, clean up empty containers emulating SVG groups (#1960, #2393, #2697).
		while (parentToClean && parentToClean.div && parentToClean.div.childNodes.length === 0) {
			grandParent = parentToClean.parentGroup;
			wrapper.safeRemoveChild(parentToClean.div);
			delete parentToClean.div;
			parentToClean = grandParent;
		}

		// remove from alignObjects
		if (wrapper.alignTo) {
			erase(wrapper.renderer.alignedObjects, wrapper);
		}

		for (var key in wrapper) {
        	if (wrapper.hasOwnProperty(key)) {
				delete wrapper[key];
			}
        }

		return null;
	};
}(Highcharts));
Collaborator

TorsteinHonsi commented May 2, 2017

It should be released within two to three weeks. In the meantime, here's a drop-in fix that you can use for v5.0.10. View it live on jsFiddle.

// Drop-in fix for Highcharts issue #6550, requires Highcharts v5.0.10.
(function (H) {
	var each = H.each;
	/**
	 * Destroy the element and element wrapper and clear up the DOM and event
	 * hooks.
	 *
	 * @returns {void}
	 */
	H.SVGElement.prototype.destroy = function () {
		var wrapper = this,
			element = wrapper.element || {},
			parentToClean =
				wrapper.renderer.isSVG &&
				element.nodeName === 'SPAN' &&
				wrapper.parentGroup,
			grandParent,
			ownerSVGElement = element.ownerSVGElement,
			i;

		// remove events
		element.onclick = element.onmouseout = element.onmouseover =
			element.onmousemove = element.point = null;
		stop(wrapper); // stop running animations

		if (wrapper.clipPath && ownerSVGElement) {
			// Look for existing references to this clipPath and remove them
			// before destroying the element (#6196).
			each(
				ownerSVGElement.querySelectorAll('[clip-path]'),
				function (el) {
					// Include the closing paranthesis in the test to rule out
					// id's from 10 and above (#6550)
					if (el.getAttribute('clip-path')
							.indexOf(wrapper.clipPath.element.id + ')') > -1) {
						el.removeAttribute('clip-path');
					}
				}
			);
			wrapper.clipPath = wrapper.clipPath.destroy();
		}

		// Destroy stops in case this is a gradient object
		if (wrapper.stops) {
			for (i = 0; i < wrapper.stops.length; i++) {
				wrapper.stops[i] = wrapper.stops[i].destroy();
			}
			wrapper.stops = null;
		}

		// remove element
		wrapper.safeRemoveChild(element);

		/*= if (build.classic) { =*/
		wrapper.destroyShadows();
		/*= } =*/

		// In case of useHTML, clean up empty containers emulating SVG groups (#1960, #2393, #2697).
		while (parentToClean && parentToClean.div && parentToClean.div.childNodes.length === 0) {
			grandParent = parentToClean.parentGroup;
			wrapper.safeRemoveChild(parentToClean.div);
			delete parentToClean.div;
			parentToClean = grandParent;
		}

		// remove from alignObjects
		if (wrapper.alignTo) {
			erase(wrapper.renderer.alignedObjects, wrapper);
		}

		for (var key in wrapper) {
        	if (wrapper.hasOwnProperty(key)) {
				delete wrapper[key];
			}
        }

		return null;
	};
}(Highcharts));
@zainrafique

This comment has been minimized.

Show comment
Hide comment
@zainrafique

zainrafique commented May 4, 2017

@TorsteinHonsi thank you man

@vesterij

This comment has been minimized.

Show comment
Hide comment
@vesterij

vesterij May 5, 2017

Hah, I have traced this bug past three days. I just found the reason but couldn't reproduce it in jsfiddle and then noticed that version was changed and it had been fixed literally yesterday. Thanks.

vesterij commented May 5, 2017

Hah, I have traced this bug past three days. I just found the reason but couldn't reproduce it in jsfiddle and then noticed that version was changed and it had been fixed literally yesterday. Thanks.

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