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

fix(injectBaseStylesheet): address style injection issue #605

Closed
wants to merge 3 commits into from
Closed
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
279 changes: 147 additions & 132 deletions index.html
Original file line number Diff line number Diff line change
@@ -1,134 +1,149 @@
<!doctype html>
<!DOCTYPE html>
<html lang="en">

<head>
<meta charset="utf-8">
<meta http-equiv="X-UA-Compatible" content="IE=edge,chrome=1">
<meta name="designer" content="imgix">
<meta name="developer" content="imgix">
<meta name="viewport" content="width=device-width,initial-scale=1,maximum-scale=1,maximum-scale=1">
<meta http-equiv="Accept-CH" content="DPR, Width, Viewport-Width">

<title>Drift Playground</title>
<link rel="stylesheet" media="screen, projection" href="./dist/drift-basic.css">
<style type="text/css">
body {
font-family: Helvetica Neue, Arial, sans;
margin-top: 2em;
background: #FAFAFA;
}

.wrapper {
margin: 0 auto;
width: 860px;
}

.drift-demo-trigger {
width: 40%;
float: left;
}

.detail {
position: relative;
width: 55%;
margin-left: 5%;
float: left;
}

h1 {
color: #013C4A;
margin-top: 1em;
margin-bottom: 1em;
}

p {
max-width: 32em;
margin-bottom: 1em;
color: #23637f;
line-height: 1.6em;
}

p:last-of-type {
margin-bottom: 2em;
}

a {
color: #00C0FA;
}

.ix-link {
display: block;
margin-bottom: 3em;
}

@media (max-width: 900px) {
.wrapper {
text-align: center;
width: auto;
}

.detail,
.drift-demo-trigger {
float: none;
}

.drift-demo-trigger {
max-width: 100%;
width: auto;
margin: 0 auto;
}

.detail {
margin: 0;
width: auto;
}

p {
margin: 0 auto 1em;
}

.responsive-hint {
display: none;
}

.drift-bounding-box {
display: none;
}
}
</style>
</head>

<body>
<div class="wrapper">
<img class="drift-demo-trigger" data-zoom="http://assets.imgix.net/unsplash/lighthouse.jpg?w=1200&amp;ch=DPR&amp;dpr=2" src="http://assets.imgix.net/unsplash/lighthouse.jpg?w=400&amp;ch=DPR&amp;dpr=2">

<div class="detail">
<section>
<h1>Drift Demo</h1>
<p>This is a demo of Drift, a simple, lightweight, no-dependencies JavaScript "zoom on hover" tool from
<a href="http://imgix.com">imgix</a>. Move your mouse over the image (or touch it) to see it in action.</p>
<p>This demo uses the simple included theme, but it's very easy to extend and customize to fit your needs. You can
<a href="https://github.com/imgix/drift">learn more and download it here</a>.</p>
<p class="responsive-hint">(Psst… try making your browser window smaller!)</p>
</section>

<a href="https://imgix.com" class="ix-link">
<img src="https://assets.imgix.net/presskit/imgix-presskit.pdf?page=3&amp;fm=png&amp;w=320&amp;dpr=2" width="160" height="60"
alt="imgix">
</a>
</div>
</div>

<script src="dist/Drift.js"></script>
<script>
new Drift(document.querySelector('.drift-demo-trigger'), {
paneContainer: document.querySelector('.detail'),
inlinePane: 900,
inlineOffsetY: -85,
containInline: true,
hoverBoundingBox: true
});
</script>
</body>

<head>
<meta charset="utf-8" />
<meta http-equiv="X-UA-Compatible" content="IE=edge,chrome=1" />
<meta name="designer" content="imgix" />
<meta name="developer" content="imgix" />
<meta name="viewport" content="width=device-width,initial-scale=1,maximum-scale=1,maximum-scale=1" />
<meta http-equiv="Accept-CH" content="DPR, Width, Viewport-Width" />
<!-- If CSP style policies, ensure SHA, nonce, or unsafe-inline are enabled -->
<meta
http-equiv="Content-Security-Policy"
content="style-src 'self' 'sha256-OdI56ZW769BKGaLghLr3uZVaaWfqyRwyccMiu8gXf0w=' 'sha256-D2blx2qCCtVP48luzhEjUKKIcmHy77Ma+9/c6fv8UGM=' "
/>

<title>Drift Playground</title>
<link rel="stylesheet" media="screen, projection" href="./dist/drift-basic.css" />
<style type="text/css">
body {
font-family: Helvetica Neue, Arial, sans;
margin-top: 2em;
background: #fafafa;
}

.wrapper {
margin: 0 auto;
width: 860px;
}

.drift-demo-trigger {
width: 40%;
float: left;
}

.detail {
position: relative;
width: 55%;
margin-left: 5%;
float: left;
}

h1 {
color: #013c4a;
margin-top: 1em;
margin-bottom: 1em;
}

p {
max-width: 32em;
margin-bottom: 1em;
color: #23637f;
line-height: 1.6em;
}

p:last-of-type {
margin-bottom: 2em;
}

a {
color: #00c0fa;
}

.ix-link {
display: block;
margin-bottom: 3em;
}

@media (max-width: 900px) {
.wrapper {
text-align: center;
width: auto;
}

.detail,
.drift-demo-trigger {
float: none;
}

.drift-demo-trigger {
max-width: 100%;
width: auto;
margin: 0 auto;
}

.detail {
margin: 0;
width: auto;
}

p {
margin: 0 auto 1em;
}

.responsive-hint {
display: none;
}

.drift-bounding-box {
display: none;
}
}
</style>
</head>

<body>
<div class="wrapper">
<img
class="drift-demo-trigger"
data-zoom="http://assets.imgix.net/unsplash/lighthouse.jpg?w=1200&amp;ch=DPR&amp;dpr=2"
src="http://assets.imgix.net/unsplash/lighthouse.jpg?w=400&amp;ch=DPR&amp;dpr=2"
/>

<div class="detail">
<section>
<h1>Drift Demo</h1>
<p>
This is a demo of Drift, a simple, lightweight, no-dependencies JavaScript "zoom on hover" tool from
<a href="http://imgix.com">imgix</a>. Move your mouse over the image (or touch it) to see it in action.
</p>
<p>
This demo uses the simple included theme, but it's very easy to extend and customize to fit your needs. You
can <a href="https://github.com/imgix/drift">learn more and download it here</a>.
</p>
<p class="responsive-hint">(Psst… try making your browser window smaller!)</p>
</section>

<a href="https://imgix.com" class="ix-link">
<img
src="https://assets.imgix.net/presskit/imgix-presskit.pdf?page=3&amp;fm=png&amp;w=320&amp;dpr=2"
width="160"
height="60"
alt="imgix"
/>
</a>
</div>
</div>

<script src="dist/Drift.js"></script>
<script>
new Drift(document.querySelector(".drift-demo-trigger"), {
paneContainer: document.querySelector(".detail"),
inlinePane: 900,
inlineOffsetY: -85,
containInline: true,
hoverBoundingBox: true,
});
</script>
</body>
</html>
3 changes: 2 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@
"description": "Easily add \"zoom on hover\" functionality to your site's images. Lightweight, no-dependency JavaScript.",
"contributors": [
"Frederick Fogerty <frederick.fogerty@gmail.com> (https://github.com/frederickfogerty)",
"Sherwin Heydarbeygi <sherwin@imgix.com> (https://github.com/sherwinski)"
"Sherwin Heydarbeygi <sherwin@imgix.com> (https://github.com/sherwinski)",
"Luis Ball <lball@imgix.com> (https://github.com/luqven)"
],
"main": "lib/Drift.js",
"module": "es/Drift.js",
Expand Down
9 changes: 5 additions & 4 deletions src/js/injectBaseStylesheet.js
Original file line number Diff line number Diff line change
Expand Up @@ -55,13 +55,14 @@ export default function injectBaseStylesheet() {
return;
}

// create the injected styles
const styleEl = document.createElement("style");
styleEl.type = "text/css";
styleEl.classList = "drift-base-styles";

styleEl.textContent = RULES;
const head = document.head;

// prepend them to the document head's first child
const head = document.head;
const allHeadElements = head.getElementsByTagName("*");
allHeadElements.innerHTML = styleEl + allHeadElements.innerHTML;
const firstItem = allHeadElements.item(0);
firstItem.outerHTML = styleEl.outerHTML + firstItem.outerHTML;
Comment on lines +63 to +67
Copy link
Contributor

Choose a reason for hiding this comment

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

If we can't get around the CSP violation anyway, is it cleaner to go back to using .insertBefore() rather than this method? i.e. reverting the changes introduced here?

Copy link
Contributor Author

@luqven luqven Mar 1, 2021

Choose a reason for hiding this comment

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

That's a good point. I do think this read easier that way..

It might then be better to close this PR altogether and create two new ones:

  1. rolling back that change and then
  2. adding the CSP to index.html as an example / check for handling those issues.

I'll discuss w/ @sherwinski offline and circle back.

}