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 5 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions docs/source/_static/css/custom.css
Original file line number Diff line number Diff line change
Expand Up @@ -103,3 +103,7 @@ h3.admonition-title::before {
mask-image: var(--icon-admonition-default);
mask-repeat: no-repeat;
}
/* Apply SVG from inline page.html */
.hidden-svg {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See comment below about renaming this to something more generic.

display: none;
}
20 changes: 20 additions & 0 deletions docs/source/_static/css/dark_light_mode.css
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
body {
--color-code-background: #f8f8f8;
--color-code-foreground: black;
}

/* Dark theme styles */
@media not print {
body[data-theme="dark"] {
--color-code-background: #272822;
--color-code-foreground: #f8f8f2;
}

/* For users who prefer dark color scheme */
@media (prefers-color-scheme: dark) {
body:not([data-theme="light"]) {
--color-code-background: #272822;
--color-code-foreground: #f8f8f2;
}
}
}
27 changes: 27 additions & 0 deletions docs/source/_static/js/loadShortbread.js
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is already logic in custom.js that handles running functions when the DOM is loaded or ready. I think this file can have a function like:

function loadShortbread() {
  ...
}

Then in custom.js we can import this function as shown below and add it to the list of functions we're already calling after the DOM is leaded.:

import loadShortbread from "./loadShortbread.js";

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Putting it inside a function and calling that function from custom.js didn't work.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should also run all JavaScript changes through a formatter. Nothing is strictly validating this, but we should make best effort to format them for readability/maintainability.

Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
// Load shortbread
document.addEventListener('DOMContentLoaded', function() {

document.body.dataset.theme = localStorage.getItem("theme") || "auto";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This logic isn't related to shortbread so we should move this out of this file and directly into custom.js. To follow the same pattern of calling functions after the DOM is loaded, we can create one for this:

function loadThemeFromLocalStorage(){
  document.body.dataset.theme = localStorage.getItem("theme") || "auto";
}

Then the runAfterDOMLoads function would be updated to something like:

function runAfterDOMLoads() {
	expandSubMenu();
	makeCodeBlocksScrollable();
	setupKeyboardFriendlyNavigation();
        // Newly added functions from this PR
        loadThemeFromLocalStorage();
        loadShortbread();
}


if (typeof AWSCShortbread !== 'undefined') {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In what cases would this condition fail?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If somehow shortbread.js isn't loaded properly from CDN.

const shortbread = AWSCShortbread({
domain: ".amazonaws.com"
// domain: ".cloudfront.net"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit - remove this comment

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add it as a comment with one sentence description instead of this.

});

// Check for cookie consent
shortbread.checkForCookieConsent();

// Add event listener for 'Cookie preferences' link read from page.html
const cookiePreferencesLink = document.getElementById('cookie-button-link');
if (cookiePreferencesLink) {
cookiePreferencesLink.addEventListener('click', function(event) {
event.preventDefault();
shortbread.customizeCookies();
});
}
console.log("AWSCShortbread loaded successfully loaded!!!")
} else {
console.error("AWSCShortbread failed to load!!!")
}
});
3 changes: 0 additions & 3 deletions docs/source/_static/shortbreadv1.js

This file was deleted.

107 changes: 107 additions & 0 deletions docs/source/_templates/base.html
Copy link
Contributor

@jonathan343 jonathan343 Dec 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The only reason I see for porting over base.html from furo is to add extra content to the <head> element.
We can prevent having to port over this entire file and instead add the following to page.html.

{%- block extrahead %}
<script src="https://prod.assets.shortbread.aws.dev/shortbread.js"></script>
<link href="https://prod.assets.shortbread.aws.dev/shortbread.css" rel="stylesheet">
{% endblock %}

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To be discussed...

Original file line number Diff line number Diff line change
@@ -0,0 +1,107 @@
<!doctype html>
<html class="no-js"{% if language is not none %} lang="{{ language }}"{% endif %} data-content_root="{{ content_root }}">
<head>
<script src="https://prod.assets.shortbread.aws.dev/shortbread.js"></script>
<link href="https://prod.assets.shortbread.aws.dev/shortbread.css" rel="stylesheet">
<script type="text/javascript" src="{{ pathto('_static/js/loadShortbread.js',1) }}"></script>
{%- block site_meta -%}
<meta charset="utf-8"/>
<meta name="viewport" content="width=device-width,initial-scale=1"/>
<meta name="color-scheme" content="light dark">

{%- if metatags %}{{ metatags }}{% endif -%}

{%- block linktags %}
{%- if hasdoc('about') -%}
<link rel="author" title="{{ _('About these documents') }}" href="{{ pathto('about') }}" />
{%- endif -%}
{%- if hasdoc('genindex') -%}
<link rel="index" title="{{ _('Index') }}" href="{{ pathto('genindex') }}" />
{%- endif -%}
{%- if hasdoc('search') -%}
<link rel="search" title="{{ _('Search') }}" href="{{ pathto('search') }}" />
{%- endif -%}
{%- if hasdoc('copyright') -%}
<link rel="copyright" title="{{ _('Copyright') }}" href="{{ pathto('copyright') }}" />
{%- endif -%}
{%- if next -%}
<link rel="next" title="{{ next.title|striptags|e }}" href="{{ next.link|e }}" />
{%- endif -%}
{%- if prev -%}
<link rel="prev" title="{{ prev.title|striptags|e }}" href="{{ prev.link|e }}" />
{%- endif -%}
{#- rel="canonical" (set by html_baseurl) -#}
{%- if pageurl %}
<link rel="canonical" href="{{ pageurl|e }}" />
{%- endif %}
{%- endblock linktags %}

{# Favicon #}
{%- if favicon_url -%}
<link rel="shortcut icon" href="{{ favicon_url }}"/>
{%- endif -%}

<!-- Generated with Sphinx {{ sphinx_version }} and Furo {{ furo_version }} -->

{%- endblock site_meta -%}

{#- Site title -#}
{%- block htmltitle -%}
{% if not docstitle %}
<title>{{ title|striptags|e }}</title>
{% elif pagename == master_doc %}
<title>{{ docstitle|striptags|e }}</title>
{% else %}
<title>{{ title|striptags|e }} - {{ docstitle|striptags|e }}</title>
{% endif %}
{%- endblock -%}

{%- block styles -%}

{# Custom stylesheets #}
{%- block regular_styles -%}
{%- for css in css_files -%}
{% if css|attr("filename") -%}
{{ css_tag(css) }}
{%- else -%}
<link rel="stylesheet" href="{{ pathto(css, 1)|e }}" type="text/css" />
{%- endif %}
{% endfor -%}
{%- endblock regular_styles -%}

{#- Theme-related stylesheets -#}
{%- block theme_styles %}
{% include "head_css_variables.html" with context %}
{%- endblock -%}

{%- block extra_styles %}
{%- endblock -%}

{%- endblock styles -%}

{#- Custom front matter #}
{%- block extrahead -%}{%- endblock -%}
</head>
<body>
{% block body %}
<script>
document.body.dataset.theme = localStorage.getItem("theme") || "auto";
</script>
{% endblock %}

{%- block scripts -%}

{# Custom JS #}
{%- block regular_scripts -%}
{% for path in script_files -%}
{{ js_tag(path) }}
{% endfor -%}
{%- endblock regular_scripts -%}

{# Theme-related JavaScript code #}
{%- block theme_scripts -%}
{%- endblock -%}

{%- endblock scripts -%}
</body>
</html>
13 changes: 13 additions & 0 deletions docs/source/_templates/head_css_variables.html
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you explain how the changes to this file work, it doesn't make sense to me? Where is the declare_css_variables called?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To be discussed...

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, we should add this file to a new partials directory to mimic the structure of furo (as previously mentioned, icons.html will also be added to this directory. The name should also be changed to _head_css_variables.html (currently missing an underscore).

Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
{#
This is where user-provided CSS variables get converted into actual values.
#}
{% macro declare_css_variables(user_customisations, pygments_theme) -%}
<link rel="stylesheet" href="{{ pathto('../_static/css/dark_light_mode.css', 1) }}">
--color-code-background: {{ pygments_theme.background }};
--color-code-foreground: {{ pygments_theme.foreground }};
{% if user_customisations -%}
{% for name, value in user_customisations.items() -%}
--{{ name }}: {{ value }};
{% endfor %}
{%- endif %}
{%- endmacro %}
61 changes: 61 additions & 0 deletions docs/source/_templates/layout.html
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file seems to be a port of the partials/icons.html file in furo. Is there a reason why this is being added to layout.html? Furo also has a layout.html file with different contents.

Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
{# Adapted from Just the Docs #}
<svg xmlns="http://www.w3.org/2000/svg" class="hidden-svg">
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change to class="hidden" if you decide to change the class name.

<symbol id="svg-toc" viewBox="0 0 24 24">
<title>Contents</title>
<svg stroke="currentColor" fill="currentColor" stroke-width="0" viewBox="0 0 1024 1024">
<path d="M408 442h480c4.4 0 8-3.6 8-8v-56c0-4.4-3.6-8-8-8H408c-4.4 0-8 3.6-8 8v56c0 4.4 3.6 8 8 8zm-8 204c0 4.4 3.6 8 8 8h480c4.4 0 8-3.6 8-8v-56c0-4.4-3.6-8-8-8H408c-4.4 0-8 3.6-8 8v56zm504-486H120c-4.4 0-8 3.6-8 8v56c0 4.4 3.6 8 8 8h784c4.4 0 8-3.6 8-8v-56c0-4.4-3.6-8-8-8zm0 632H120c-4.4 0-8 3.6-8 8v56c0 4.4 3.6 8 8 8h784c4.4 0 8-3.6 8-8v-56c0-4.4-3.6-8-8-8zM115.4 518.9L271.7 642c5.8 4.6 14.4.5 14.4-6.9V388.9c0-7.4-8.5-11.5-14.4-6.9L115.4 505.1a8.74 8.74 0 0 0 0 13.8z"/>
</svg>
</symbol>
<symbol id="svg-menu" viewBox="0 0 24 24">
<title>Menu</title>
<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 24 24" fill="none" stroke="currentColor"
stroke-width="2" stroke-linecap="round" stroke-linejoin="round" class="feather-menu">
<line x1="3" y1="12" x2="21" y2="12"></line>
<line x1="3" y1="6" x2="21" y2="6"></line>
<line x1="3" y1="18" x2="21" y2="18"></line>
</svg>
</symbol>
<symbol id="svg-arrow-right" viewBox="0 0 24 24">
<title>Expand</title>
<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 24 24" fill="none" stroke="currentColor"
stroke-width="2" stroke-linecap="round" stroke-linejoin="round" class="feather-chevron-right">
<polyline points="9 18 15 12 9 6"></polyline>
</svg>
</symbol>
<symbol id="svg-sun" viewBox="0 0 24 24">
<title>Light mode</title>
<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 24 24" fill="none" stroke="currentColor"
stroke-width="1.5" stroke-linecap="round" stroke-linejoin="round" class="feather-sun">
<circle cx="12" cy="12" r="5"></circle>
<line x1="12" y1="1" x2="12" y2="3"></line>
<line x1="12" y1="21" x2="12" y2="23"></line>
<line x1="4.22" y1="4.22" x2="5.64" y2="5.64"></line>
<line x1="18.36" y1="18.36" x2="19.78" y2="19.78"></line>
<line x1="1" y1="12" x2="3" y2="12"></line>
<line x1="21" y1="12" x2="23" y2="12"></line>
<line x1="4.22" y1="19.78" x2="5.64" y2="18.36"></line>
<line x1="18.36" y1="5.64" x2="19.78" y2="4.22"></line>
</svg>
</symbol>
<symbol id="svg-moon" viewBox="0 0 24 24">
<title>Dark mode</title>
<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 24 24" fill="none" stroke="currentColor"
stroke-width="1.5" stroke-linecap="round" stroke-linejoin="round" class="icon-tabler-moon">
<path stroke="none" d="M0 0h24v24H0z" fill="none" />
<path d="M12 3c.132 0 .263 0 .393 0a7.5 7.5 0 0 0 7.92 12.446a9 9 0 1 1 -8.313 -12.454z" />
</svg>
</symbol>
<symbol id="svg-sun-half" viewBox="0 0 24 24">
<title>Auto light/dark mode</title>
<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 24 24" fill="none" stroke="currentColor"
stroke-width="1.5" stroke-linecap="round" stroke-linejoin="round" class="icon-tabler-shadow">
<path stroke="none" d="M0 0h24v24H0z" fill="none"/>
<circle cx="12" cy="12" r="9" />
<path d="M13 12h5" />
<path d="M13 15h4" />
<path d="M13 18h1" />
<path d="M13 9h4" />
<path d="M13 6h1" />
</svg>
</symbol>
</svg>
22 changes: 7 additions & 15 deletions docs/source/_templates/page.html
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,12 @@
furnished to do so, subject to the following conditions:
#}

{%- extends "!page.html" %}
{% extends "base.html" %}


{% block body -%}
<script>
document.body.dataset.theme = localStorage.getItem("theme") || "auto";
</script>
{% include "partials/icons.html" %}
<svg xmlns="http://www.w3.org/2000/svg" style="display: none;">
{% include 'layout.html' %}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Based on the discussion in the related comment above, we'll need to update this to {% include "partials/icons.html" %}

<svg xmlns="http://www.w3.org/2000/svg" class="hidden-svg">
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should choose a more generic name than hidden-svg, maybe instead use hidden. If we ever decide to apply this to another element that isn't an SVG, this class wouldn't make sense.

<symbol id="svg-close" viewBox="0 0 24 24">
<title>Close Menu</title>
<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 24 24" stroke="currentColor"
Expand Down Expand Up @@ -176,15 +174,9 @@
{%- endif %}
{# start of AWS modification of Furo template #}
<div class="legal-info">
<script type="text/javascript" src="{{ pathto('_static/shortbreadv1.js',1 ) }}"></script>
<script type="text/javascript">
const shortbread = AWSCShortbread({
domain: ".amazonaws.com",
});
shortbread.checkForCookieConsent();
</script>
<a href="https://aws.amazon.com/privacy">Privacy</a> | <a href="https://aws.amazon.com/terms">Site Terms</a> | <a
href="#" onclick="shortbread.customizeCookies();">Cookie preferences</a>
<a href="https://aws.amazon.com/privacy">Privacy</a> |
<a href="https://aws.amazon.com/terms">Site Terms</a> |
<a href="#" id="cookie-button-link">Cookie preferences</a>
</div>
{# end of AWS modification of Furo template #}
</div>
Expand Down
Loading