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

[react-error-overlay] Add an Optional Runtime Option Style iFrame with a Class #5943

Closed
wants to merge 1 commit into from

Conversation

dav-is
Copy link

@dav-is dav-is commented Dec 1, 2018

To be more CSP compliant, inline styles should be avoided. I added an alternative to using inline styles on the iframe itself so it doesn't violate CSP.

Screenshot of change
(Tested in Next.js)

Note: the tests are failing due to things outside the scope of this PR.

@mrmckeb
Copy link
Contributor

mrmckeb commented Dec 6, 2018

Thanks for this PR, @dav-is. Are you able to put together a quick demonstration project with this in place?

@dav-is
Copy link
Author

dav-is commented Dec 6, 2018

@mrmckeb I implemented it inside of next.js so it'd be hard to load all the changes locally, but basically I copied the styles that are added to the iframe <iframe styles= and moved it to <iframe class= if the option is set. The corresponding CSS becomes the app's responsibility and is inserted in a stylesheet from the server in dev mode.

https://github.com/dav-is/next.js/blob/d1017f8f84579169182ae9558cd9d25c8e496397/packages/next/pages/_document.js#L133
https://github.com/dav-is/next.js/blob/d1017f8f84579169182ae9558cd9d25c8e496397/packages/next/client/dev-error-overlay/hot-dev-client.js#L74

@mrmckeb
Copy link
Contributor

mrmckeb commented Dec 18, 2018

Sorry @dav-is, I missed your response here (lost in emails). It would be great to get a demo of this to test before merging, if you could put one together?

This is an area of CRA I'm not intimately familiar with, so that would help in testing.

@stale
Copy link

stale bot commented Jan 18, 2019

This pull request has been automatically marked as stale because it has not had any recent activity. It will be closed in 5 days if no further activity occurs.

@stale stale bot added the stale label Jan 18, 2019
@stale
Copy link

stale bot commented Jan 26, 2019

This pull request has been automatically closed because it has not had any recent activity. If you have a question or comment, please open a new issue. Thank you for your contribution!

@stale stale bot closed this Jan 26, 2019
@lock lock bot locked and limited conversation to collaborators Jan 31, 2019
@dav-is dav-is deleted the csp branch July 8, 2019 18:34
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants