-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Add CSP tags to default layout #1630
Conversation
43a66e9
to
060ad3e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
@@ -23,6 +23,8 @@ By default, it renders: | |||
</title> | |||
<%= render "stylesheet" %> | |||
<%= csrf_meta_tags %> | |||
<%# Added in Rails 5.2 %> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we drop the comment? I don't think we need it as it'll be there now for a long time
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was thinking it would increase the chances that the defined? would be dropped when only >= 5.2 is supported. Should we inspect Rails::VERSION?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah yes. I think we're okay here: we should mention in the commit message that exists from Rails 5.2 and up.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I created an issue to look into compatibility detection: #1631
This is needed to get Trix and CSP to play together.
060ad3e
to
6a74fbc
Compare
This is needed to get Trix and CSP to play together.