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

Fix Line Breaks and HTML Escaping #1667

Merged
merged 2 commits into from
Sep 16, 2015
Merged

Fix Line Breaks and HTML Escaping #1667

merged 2 commits into from
Sep 16, 2015

Conversation

rizwanreza
Copy link

ℹ️ Please read.

Whenever we have input that's not contenteditable, we should use double braces ({{ }}) with .preserve-line-breaks class.

When using contenteditable input, feel free to use triple braces ({{{ }}}). The browsers that support contenteditable boxes already escape any HTML that's injected by the user.

zdennis and others added 2 commits September 15, 2015 11:55
This removes the display-line-break.js and line-break-to-tag.js as we can rely on CSS to preserve whitespace handling.

In many cases we can use HTMLBars triple brace {{{...}}} to render content as-is. Modern browsers support not executing <script> tags. Reference: https://developer.mozilla.org/en-US/docs/Web/API/Element/innerHTML . BUT this is breakable with content like:

     <iframe src=""srcdoc="<script>alert('hi');</script>"></iframe>

Currently, some user input-able content is stored as plain text on the backend. Other times, it's converted to simple HTML on the front-end and saved in the backend.

This means a bunch of the {{{...}}} in this PR may not be able to be trusted. Need to circle back with team and find a uniform way of handling user inputtable content.
rizwanreza added a commit that referenced this pull request Sep 16, 2015
@rizwanreza rizwanreza merged commit 0900d4b into acceptance Sep 16, 2015
@zdennis zdennis deleted the spike/linebreaks branch October 14, 2015 18:34
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

Successfully merging this pull request may close these issues.

2 participants