Skip to content

Conversation

rreusser
Copy link
Contributor

@rreusser rreusser commented Aug 2, 2017

See #1923. This PR modifies svg text utils to ignore dangling closing tags when there's nothing on the stack.

@rreusser rreusser added status: reviewable bug something broken labels Aug 2, 2017
function exitNode(type) {
// A bare closing tag can't close the root node. If we encounter this it
// means there's an extra closing tag that can just be ignored:
if(nodeStack.length === 1) return;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do you want to log this like we do below for if(type !== innerNode.type)? Though, we don't do anything with this so maybe we should get rid of them both...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That seems reasonable. Do those log messages show up on-screen? Does that seem desirable?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just in the console, but I think they also go to sentry when they happen on plot.ly

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

or maybe not... not really sure, I haven't looked there. But anyway Lib.log points to console.trace if it exists, otherwise it falls back on console.log

Copy link
Contributor Author

@rreusser rreusser Aug 2, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added message ✅ "Ignoring unexpected end tag </...>"

@alexcjohnson
Copy link
Collaborator

Nice catch and nice fix! 💃

@rreusser rreusser merged commit df5face into master Aug 3, 2017
@rreusser rreusser deleted the fix-invalid-html-input branch August 3, 2017 22:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug something broken

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants