Skip to content
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.

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

HTML entities in text nodes returned as real HTML #11

Closed
ssorallen opened this issue Jan 5, 2015 · 5 comments
Closed

HTML entities in text nodes returned as real HTML #11

ssorallen opened this issue Jan 5, 2015 · 5 comments

Comments

@ssorallen
Copy link
Contributor

When a TextNode is encountered, HTML entities inside it are returned as HTML rather than the HTML entity. This is problematic for the use of < and > in particular because the tags they wrap are then interpreted as real tags.

For example:

> converter.convert("<code>&lt;body&gt;</code>");
"<code><body></code>
"

The encoded symbols are returned as the symbols themselves, which eventually throws an error in Esprima because it encounters an unclosed tag.

This is expected and documented behavior of TextNodes and their data attribute, and it seems the way to get around it is to escape the characters differently before inserting the content into a real DOM node.

@ssorallen
Copy link
Contributor Author

Related StackOverflow answers that explains why this happens: text nodeValue containing HTML entity

@ssorallen
Copy link
Contributor Author

The simplest approach might be to escape ampersands in text passed to the converter:

> converter.convert("<code>&amp;lt;body&amp;gt;</code>")
"<code>&lt;body&gt;</code>
"

@Daniel15
Copy link
Member

I used one weird trick to escape required symbols:

var tempEl = document.createElement('div');
function escapeSpecialChars(value) {
  // Uses this One Weird Trick to escape text - Raw text inserted as textContent
  // will have its escaped version in innerHTML.
  tempEl.textContent = value;
  return tempEl.innerHTML;
}

escapeSpecialCharacters('<'); // &lt;

Although I just realised this won't work exactly right in Node.js where I use JSDom, so I'll have to move stuff around a bit.

@ssorallen
Copy link
Contributor Author

The fix works great in browser.

Are the past couple fixes worthy of a minor version release or a patch release?

@Daniel15
Copy link
Member

Yeah I think I'll do a patch release, just want to wait a few days and see
if any other small bug fixes come up.

Regards,
Daniel Lo Nigro
http://dan.cx/ | Twitter http://twitter.com/Daniel15 | Facebook
http://www.facebook.com/daaniel

On Sat, Jan 10, 2015 at 12:13 PM, Ross Allen notifications@github.com
wrote:

The fix works great in browser.

Are the past couple fixes worthy of a minor version release or a patch
release?

Reply to this email directly or view it on GitHub
#11 (comment).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants