-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
Removes inline scripts and inline styles to make it compatible with the newly added security headers #4369
base: develop
Are you sure you want to change the base?
Conversation
…and scripts to make them compatible with the new security headers
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.
Hey @ubaskota, thanks for opening this PR!
This is a great start! I've left some comments mainly related to improving maintainability.
The main concern I have is with the docs/source/_templates/head_css_variables.html
file. I don't understand how it works and a bit skeptical of the change.
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.
Requesting some final changes. This PR looks good to me once those are addressed.
docs/source/_static/js/custom.js
Outdated
// Functions to run after the DOM loads. | ||
function runAfterDOMLoads() { | ||
expandSubMenu(); | ||
|
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.
nit - Just curious, why does this empty line exist? What this change added by you or the formatter?
Due to the recently added Content Security Policy(CSP), all inline scripts are automatically blocked. In order to fix this, we had to remove inline scripts and styles, and keep them in separate files. This change performs all necessary edits to remove the inline scripts and keep them in separate files.