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

Removes inline scripts and inline styles to make it compatible with the newly added security headers #4369

Open
wants to merge 9 commits into
base: develop
Choose a base branch
from

Conversation

ubaskota
Copy link

@ubaskota ubaskota commented Dec 5, 2024

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.

Copy link
Contributor

@jonathan343 jonathan343 left a 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.

docs/source/_templates/layout.html Outdated Show resolved Hide resolved
docs/source/_templates/page.html Outdated Show resolved Hide resolved
docs/source/_templates/page.html Outdated Show resolved Hide resolved
docs/source/_static/css/custom.css Outdated Show resolved Hide resolved
docs/source/_static/js/loadShortbread.js Outdated Show resolved Hide resolved
docs/source/_static/js/loadShortbread.js Outdated Show resolved Hide resolved
docs/source/_templates/layout.html Outdated Show resolved Hide resolved
docs/source/_templates/head_css_variables.html Outdated Show resolved Hide resolved
docs/source/_templates/head_css_variables.html Outdated Show resolved Hide resolved
docs/source/_templates/base.html Outdated Show resolved Hide resolved
Copy link
Contributor

@jonathan343 jonathan343 left a 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/css/custom.css Outdated Show resolved Hide resolved
// Functions to run after the DOM loads.
function runAfterDOMLoads() {
expandSubMenu();

Copy link
Contributor

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?

docs/source/_templates/layout.html Outdated Show resolved Hide resolved
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