Skip to content
This repository has been archived by the owner on Jan 12, 2024. It is now read-only.

Commit

Permalink
Correctly deal with invalid content in non-selfclosing tags. Fixes #297
Browse files Browse the repository at this point in the history
  • Loading branch information
@tmcw
tmcw committed Sep 11, 2013
1 parent 2ead8da commit 5938ebb
Show file tree
Hide file tree
Showing 3 changed files with 6 additions and 1 deletion.
2 changes: 1 addition & 1 deletion lib/carto/tree/definition.js
View file
Expand Up @@ -139,7 +139,7 @@ tree.Definition.prototype.symbolizersToXML = function(env, symbolizers, zoom) {
}
if (selfclosing) {
xml += '/>\n';
} else {
} else if (tagcontent) {
if (tagcontent.indexOf('<') != -1) {
xml += '>' + tagcontent + '</' + name + '>\n';
} else {
Expand Down
4 changes: 4 additions & 0 deletions test/errorhandling/issue297.mss
View file
@@ -0,0 +1,4 @@
#t {
text-name: invalid;
text-face-name: "Dejagnu";
}
1 change: 1 addition & 0 deletions test/errorhandling/issue297.result
View file
@@ -0,0 +1 @@
issue297.mss:2:2 Invalid value for text-name, the type expression is expected. invalid (of type keyword) was given.

7 comments on commit 5938ebb

@strk
Copy link
Contributor

@strk strk commented on 5938ebb Sep 12, 2013

Choose a reason for hiding this comment

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

I think your implementation of this would leave the opening tag without a closing '>' character.

@tmcw
Copy link
Contributor Author

@tmcw tmcw commented on 5938ebb Sep 12, 2013

Choose a reason for hiding this comment

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

Please provide a test to demonstrate.

@strk
Copy link
Contributor

@strk strk commented on 5938ebb Sep 12, 2013 via email

Choose a reason for hiding this comment

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

@tmcw
Copy link
Contributor Author

@tmcw tmcw commented on 5938ebb Sep 12, 2013

Choose a reason for hiding this comment

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

The point is that Render.render() won't return the actual XML on error, and tagcontent is only undefined on error...

Yes, so if this matters to you, then make a unit test for it.

@yohanboniface
Copy link
Contributor

Choose a reason for hiding this comment

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

FYI, Carto generates an invalid XML with this commit (on the HOT style):

mapnik.xml:635569: parser error : Couldn't find end of Start Tag TextSymbolizer line 635569
bolizer placement="interior" fontset-name="fontset-2" fill="#525252" size="7"   
                                                                               ^

No time to dig more right now, but I will try to look at it this week-end.

@yohanboniface
Copy link
Contributor

Choose a reason for hiding this comment

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

Follow up for casual readers for this thread: #305

@strk
Copy link
Contributor

@strk strk commented on 5938ebb Sep 21, 2013

Choose a reason for hiding this comment

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

Thanks @yohanboniface. FWIW, I still think that tag should be closed even when tagcontent is undefined (to reduce damage on unexpected events)

Please sign in to comment.