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

escape attribute values in SSR #1155

Merged
merged 1 commit into from
Feb 7, 2018
Merged

Conversation

Conduitry
Copy link
Member

Opening this up instead of #1142. (Thanks @tav!) I'm fairly certain that all we need to escape are the same things that __escape already escapes. In particular, we do not need to worry about the IE behavior where attribute values can be enclosed in backticks because we are already going to be enclosing everything in double quotes - so the browser will see that first, before any potential backtick.

@codecov-io
Copy link

codecov-io commented Feb 7, 2018

Codecov Report

Merging #1155 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1155   +/-   ##
=======================================
  Coverage   91.48%   91.48%           
=======================================
  Files         126      126           
  Lines        4524     4524           
  Branches     1461     1461           
=======================================
  Hits         4139     4139           
  Misses        163      163           
  Partials      222      222
Impacted Files Coverage Δ
...ndering/visitors/shared/stringifyAttributeValue.ts 100% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 29a1569...c481c8d. Read the comment docs.

@Rich-Harris Rich-Harris merged commit 0ef8229 into master Feb 7, 2018
@Rich-Harris Rich-Harris deleted the ssr-escape-attribute-values branch February 7, 2018 13:09
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.

3 participants