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.

Sign up for GitHub

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

Jump to bottom

Fix Preview Mode in <iframe> #11638

Merged
merged 1 commit into from
Apr 3, 2020
Merged

Fix Preview Mode in <iframe> #11638

merged 1 commit into from
Apr 3, 2020

Conversation

Timer
Copy link
Member

@Timer Timer commented Apr 3, 2020 •
edited
Loading

Chrome now requires you use SameSite=None for a cookie to be present in an <iframe>.

Source: https://web.dev/samesite-cookies-explained/

@ijjk
Copy link
Member

ijjk commented Apr 3, 2020

Stats from current PR

Default Server Mode
General Overall increase ⚠️
zeit/next.js canary Timer/next.js hotfix/allow-iframe Change
buildDuration 10.9s 10.9s ⚠️ +78ms
nodeModulesSize 47.7 MB 47.7 MB ⚠️ +832 B
Client Bundles (main, webpack, commons)
zeit/next.js canary Timer/next.js hotfix/allow-iframe Change
main-HASH.js gzip 6.25 kB 6.25 kB ✓
webpack-HASH.js gzip 746 B 746 B ✓
de003c3a9d30..5272.js gzip 10.2 kB 10.2 kB ✓
framework.HASH.js gzip 39.1 kB 39.1 kB ✓
Overall change 56.3 kB 56.3 kB ✓
Client Bundles (main, webpack, commons) Modern
zeit/next.js canary Timer/next.js hotfix/allow-iframe Change
main-HASH.module.js gzip 4.78 kB 4.78 kB ✓
webpack-HASH..dule.js gzip 746 B 746 B ✓
de003c3a9d30..dule.js gzip 6.75 kB 6.75 kB ✓
framework.HA..dule.js gzip 39.1 kB 39.1 kB ✓
Overall change 51.4 kB 51.4 kB ✓
Legacy Client Bundles (polyfills)
zeit/next.js canary Timer/next.js hotfix/allow-iframe Change
polyfills-HASH.js gzip 26.3 kB 26.3 kB ✓
Overall change 26.3 kB 26.3 kB ✓
Client Pages
zeit/next.js canary Timer/next.js hotfix/allow-iframe Change
_app.js gzip 1.24 kB 1.24 kB ✓
_error.js gzip 3.15 kB 3.15 kB ✓
hooks.js gzip 664 B 664 B ✓
index.js gzip 222 B 222 B ✓
link.js gzip 2.03 kB 2.03 kB ✓
routerDirect.js gzip 279 B 279 B ✓
withRouter.js gzip 278 B 278 B ✓
Overall change 7.86 kB 7.86 kB ✓
Client Pages Modern
zeit/next.js canary Timer/next.js hotfix/allow-iframe Change
_app.module.js gzip 594 B 594 B ✓
_error.module.js gzip 2.08 kB 2.08 kB ✓
hooks.module.js gzip 370 B 370 B ✓
index.module.js gzip 212 B 212 B ✓
link.module.js gzip 1.48 kB 1.48 kB ✓
routerDirect..dule.js gzip 271 B 271 B ✓
withRouter.m..dule.js gzip 270 B 270 B ✓
Overall change 5.28 kB 5.28 kB ✓
Client Build Manifests
zeit/next.js canary Timer/next.js hotfix/allow-iframe Change
_buildManifest.js gzip 61 B 61 B ✓
_buildManife..dule.js gzip 61 B 61 B ✓
Overall change 122 B 122 B ✓
Rendered Page Sizes
zeit/next.js canary Timer/next.js hotfix/allow-iframe Change
index.html gzip 917 B 917 B ✓
link.html gzip 926 B 926 B ✓
withRouter.html gzip 915 B 915 B ✓
Overall change 2.76 kB 2.76 kB ✓
Serverless Mode (Decrease detected ✓)
General Overall increase ⚠️
zeit/next.js canary Timer/next.js hotfix/allow-iframe Change
buildDuration 11.6s 11.7s ⚠️ +176ms
nodeModulesSize 47.7 MB 47.7 MB ⚠️ +832 B
Client Bundles (main, webpack, commons)
zeit/next.js canary Timer/next.js hotfix/allow-iframe Change
main-HASH.js gzip 6.25 kB 6.25 kB ✓
webpack-HASH.js gzip 746 B 746 B ✓
de003c3a9d30..5272.js gzip 10.2 kB 10.2 kB ✓
framework.HASH.js gzip 39.1 kB 39.1 kB ✓
Overall change 56.3 kB 56.3 kB ✓
Client Bundles (main, webpack, commons) Modern
zeit/next.js canary Timer/next.js hotfix/allow-iframe Change
main-HASH.module.js gzip 4.78 kB 4.78 kB ✓
webpack-HASH..dule.js gzip 746 B 746 B ✓
de003c3a9d30..dule.js gzip 6.75 kB 6.75 kB ✓
framework.HA..dule.js gzip 39.1 kB 39.1 kB ✓
Overall change 51.4 kB 51.4 kB ✓
Legacy Client Bundles (polyfills)
zeit/next.js canary Timer/next.js hotfix/allow-iframe Change
polyfills-HASH.js gzip 26.3 kB 26.3 kB ✓
Overall change 26.3 kB 26.3 kB ✓
Client Pages
zeit/next.js canary Timer/next.js hotfix/allow-iframe Change
_app.js gzip 1.24 kB 1.24 kB ✓
_error.js gzip 3.15 kB 3.15 kB ✓
hooks.js gzip 664 B 664 B ✓
index.js gzip 222 B 222 B ✓
link.js gzip 2.03 kB 2.03 kB ✓
routerDirect.js gzip 279 B 279 B ✓
withRouter.js gzip 278 B 278 B ✓
Overall change 7.86 kB 7.86 kB ✓
Client Pages Modern
zeit/next.js canary Timer/next.js hotfix/allow-iframe Change
_app.module.js gzip 594 B 594 B ✓
_error.module.js gzip 2.08 kB 2.08 kB ✓
hooks.module.js gzip 370 B 370 B ✓
index.module.js gzip 212 B 212 B ✓
link.module.js gzip 1.48 kB 1.48 kB ✓
routerDirect..dule.js gzip 271 B 271 B ✓
withRouter.m..dule.js gzip 270 B 270 B ✓
Overall change 5.28 kB 5.28 kB ✓
Client Build Manifests
zeit/next.js canary Timer/next.js hotfix/allow-iframe Change
_buildManifest.js gzip 61 B 61 B ✓
_buildManife..dule.js gzip 61 B 61 B ✓
Overall change 122 B 122 B ✓
Serverless bundles Overall decrease ✓
zeit/next.js canary Timer/next.js hotfix/allow-iframe Change
_error.js gzip 233 kB 232 kB -390 B
404.html gzip 1.32 kB 1.32 kB ✓
hooks.html gzip 956 B 956 B ✓
index.js gzip 233 kB 233 kB -42 B
link.js gzip 243 kB 243 kB ⚠️ +118 B
routerDirect.js gzip 241 kB 241 kB ⚠️ +54 B
withRouter.js gzip 241 kB 241 kB ⚠️ +40 B
Overall change 1.19 MB 1.19 MB -220 B

ijjk
ijjk approved these changes Apr 3, 2020
View reviewed changes
Copy link
Member

@ijjk ijjk left a comment

Choose a reason for hiding this comment

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

LGTM, seems a couple of the windows tests had unrelated install fails

@Timer Timer merged commit b747a36 into vercel:canary Apr 3, 2020
@Timer Timer deleted the hotfix/allow-iframe branch April 3, 2020 17:18
@bzin
Copy link

bzin commented Apr 4, 2020

Hi @Timer, just wanna let you know that I tested this out within Craft CMS live preview and the cookies are still not set.

Let me know if I can help debugging this.

@dohomi
Copy link

dohomi commented Apr 15, 2020

@Timer same counts for Storyblok CMS. The preview data is not set after upgrading to NextJS 9.3.5with the previewMode

This was referenced Apr 17, 2020
Merged
Merged
@jecassis
Copy link

Hello @Timer, also tested using Prismic in 9.3.5 by attempting to enter Preview Mode in development.

It is attempting to set the cookies. Works in Firefox 73, but Chrome 83 blocks them with this Tooltip error:
This Set-Cookie was blocked because it had the "Secure" attribute but was not received over a secure connection.

Managed to get the cookies out:
Set-Cookie: __prerender_bypass=b3b0ec4be899f2a69a; Path=/; HttpOnly; Secure; SameSite=None
Set-Cookie: __next_preview_data=eyJhbGciOiJIUzI1NiJ9; Path=/; HttpOnly; Secure; SameSite=None

The cookies are set to "Secure" and if using an HTTP server on localhost it fails. Using an HTTPS server works.

I added a printout inside the setPreviewData function (directly in node_modules/) and process.env.NODE_ENV returns undefined however it prints development inside api/preview.js right after returning. I assume the intention of api-utils.ts in development is that "SameSite" should be lax and "Secure" should not be present. And yet, I am unsure of how or where NODE_ENV was unset and reset.

@snyk-bot snyk-bot mentioned this pull request Apr 20, 2020
Closed
@dohomi
Copy link

dohomi commented Jul 29, 2020

@Timer is there any news on this issue? I tried to check out the most recent 9.5.0 but the example with Storyblok as CMS and the iframe is still not working.
The current example of https://github.com/vercel/next.js/tree/canary/examples/cms-storyblok does actually not show the power of the visual composer.
With the preview the props of getStaticProps still will be undefined for preview/previewData

@dohomi
Copy link

dohomi commented Jul 29, 2020

The issue only remains on localhost, deployed via Vercel everything seems fine

@damianfrizzi
Copy link

We're also experiencing issues with the preview mode with Prismic since this change. Preview mode works fine with 9.3.5-canary.0 and breaks with 9.3.5-canary.1. Preview mode doesn't work in Chrome production mode following the Prismic example. @Timer Would it help to create a new issue with a minimal repro?

@vercel vercel locked as resolved and limited conversation to collaborators Jan 30, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@ijjk ijjk ijjk approved these changes

@lfades lfades Awaiting requested review from lfades

@timneutkens timneutkens Awaiting requested review from timneutkens

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@Timer @ijjk @bzin @dohomi @jecassis @damianfrizzi