Unescaped attributes #664
Merged
Unescaped attributes #664
Conversation
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
| // If not, add `doubleParent` to close push and text. | ||
| "}));\n" | ||
| ); | ||
| buff.push(insert_cmd, "can.view.txt(\n" + |
There was a problem hiding this comment.
@sykopomp can you identify the change in this section?
There was a problem hiding this comment.
All attributes are escaped here. status() is a string when the value is inside an attribute.
added 2 commits
January 16, 2014 18:29
This takes care of a bug where attribute values would sometimes have html entities in them when the content was escaped. For example, & in an attribute value string would show up as & in the actual attribute value. The only case where we actually want to escape attributes in can.view is when they're immediately concatenated because they're simple strings.
|
The current code passes the modified tests in the pull request without any changes. The escaping really only applies to HTML tag escaping (it doesn't do ampersands). I'd disagree that attributes should always be ampersand escaped, I'd expect that your original content should be "x&y" in that case. |
Conflicts: view/mustache/mustache_test.js
|
This is ready to merge pending some external changes involving test running. |
ghost
assigned 
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This fixes an issue where, say:
would render things such that
element.title === "x&y"This makes it so that
{{{x}}}and{{x}}, when applied to attributes, always do the exact same thing. It does not make sense for us to allow unescaped attribute values incan.viewEDIT: I found out after looking closer that this was only affecting things inside
{{#foo}}blocks. This does fix that issue, and things continue to work as before. I've added a new test that specifically checks escaping inside blocks within attributes.