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

www: Correct UX of the copy button #2398

Merged
merged 1 commit into from
Jul 8, 2020

Conversation

BeniCheni
Copy link
Contributor

@BeniCheni BeniCheni commented Jul 7, 2020

Fixes #2397.

Triaged. It seems that the click on the copy button would run into a console error about CSP (Content Security Policy) Unsafe_inline_script:

Refused to execute inline event handler because it violates the following Content Security Policy directive: "script-src 'self'". Either the 'unsafe-inline' keyword, a hash ('sha256-...'), or a nonce ('nonce-...') is required to enable inline execution.

Attempt to fix by a JavaScript solution

@pietroalbini
Copy link
Member

Our CSP is defined in www/website_config.json, so eventual changes should be done there.

I'd prefer not to tweak the CSP: we intentionally made it as strict as possible, and this PR allows way more stuff than necessary (including unsafe-inline which kinda defeats the point of the CSP). If the goal is fixing the copy button not working, the better solution would be to move the onclick to rustup.js:

var buttons = document.querySelectorAll(".copy-button");
for (int i = 0; i < buttons.length; i++) {
    buttons[i].addEventListener("click", handle_copy_button_click);
}

@BeniCheni BeniCheni changed the title www: Add CSP to support copy button www: Correct UX of the copy button Jul 7, 2020
@BeniCheni
Copy link
Contributor Author

the better solution would be to move the onclick to rustup.js

I've tried out @pietroalbini's suggestion, and tested locally using "test" keyword to validate the style & functionality of the copy button.

platform-instructions-unix

Untitled-unix

platform-instructions-win32

Untitled-win32

platform-instructions-win64

Untitled-win64

platform-instructions-unknown

Untitled-unkown

platform-instructions-default

Untitled-default

@kinnison
Copy link
Contributor

kinnison commented Jul 8, 2020

So, I personally have no reason to believe this won't work, providing @BeniCheni has checked it on both chrome and firefox, and via an HTTP service with a similar CSP to the one on rustup.rs. @pietroalbini do you think this is OK?

@pietroalbini
Copy link
Member

Yeah this looks correct.

@kinnison kinnison merged commit b2384db into rust-lang:master Jul 8, 2020
AJ-Ianozi pushed a commit to AJ-Ianozi/getada-download that referenced this pull request Mar 9, 2024
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.

Style for the snippet displayed on rustup.rs is slightly broken
3 participants