Skip to content

Ignore bare closing tags in html-like input #1926

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Aug 3, 2017
Merged

Ignore bare closing tags in html-like input #1926

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6bcf092
Ignore bare closing tags in html-like input
rreusser Aug 2, 2017
5cc607c
Add log message on unexpected html end tags
rreusser Aug 2, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions src/lib/svg_text_utils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -400,7 +400,12 @@ function buildSVGText(containerNode, str) {
}

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 •
edited
Loading

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 </...>"


var innerNode = nodeStack.pop();

if(type !== innerNode.type) {
Lib.log('Start tag <' + innerNode.type + '> doesnt match end tag <' +
type + '>. Pretending it did match.', str);
Expand Down
14 changes: 14 additions & 0 deletions test/jasmine/tests/svg_text_utils_test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -371,5 +371,19 @@ describe('svg+text utils', function() {
opener(2.6) + 'modified' + closer, textCase);
});
});

it('ignores bare closing tags', function() {
var node = mockTextSVGElement('</sub>');

// sub shows up as a zero-width space (u200B) on either side of the 5:
expect(node.text()).toEqual('');
});

it('ignores extra closing tags', function() {
var node = mockTextSVGElement('test<sub>5</sub></sub>more');

// sub shows up as a zero-width space (u200B) on either side of the 5:
expect(node.text()).toEqual('test\u200b5\u200bmore');
});
});
});