Created
April 3, 2014 21:45
-
-
Save joshma/9963528 to your computer and use it in GitHub Desktop.
chartbeat.js bug
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Hi there, | |
We've been noticing errors coming from chartbeat.js: "Object #<SVGAnimatedString> has no method 'replace'" - I took a moment to diagnose the issue and found the error to arise when you click on an SVG node with an HREF attribute. It looks like there's a click handler for anchor tags that isn't aborting when the SVG node is clicked: | |
r.W = function(a) { | |
var b, c; | |
if (a = a || window.event) { | |
c = a.target || a.srcElement; | |
if (c.tagName !== "A") | |
if (c.parentNode) | |
if (c.parentNode.tagName === "A") | |
c = c.parentNode; | |
else { | |
if (c.parentNode.parentNode && c.parentNode.parentNode.tagName === "A") | |
c = c.parentNode.parentNode | |
} | |
else | |
return; | |
a = c.href || ""; | |
a = a.replace(/-{2,}/g, "-"); | |
The handler captures information about anchor tags being clicked, and tries to abort if it can't find an anchor tag "nearby." However, it's missing a return statement for the inner-most if statement - if (c.parentNode.parentNode && c.parentNode.parentNode.tagName === "A") is FALSE, we actually need to return: | |
else { | |
if (c.parentNode.parentNode && c.parentNode.parentNode.tagName === "A") | |
c = c.parentNode.parentNode | |
else // MINE | |
return; // MINE | |
} | |
Otherwise, the function does NOT abort (the other return statement is not hit), and it attempts to perform a = a.replace(...). The reason why we were able to find this bug was because SVG nodes can have an HREF attribute (used for text following paths), but that HREF attribute is not a normal JS string, but an SVGAnimatedString which lacks the replace function. | |
So in this one weird case, c.href is a truthy value but does NOT have the replace function, thus triggering the issue. | |
The proposed fix (above) is probably what the function intends to do anyways, and should resolve our SVG woes. A fix here would be greatly appreciated! | |
Regards, | |
Josh |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment