-
-
Notifications
You must be signed in to change notification settings - Fork 54
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
Restrict new windows in replay + improved service worker loading + better error messages #126
Conversation
- support target rewriting with named iframe as per webrecorder/wombat#94 - refactor sw init: detect additional errors, such as CORS issues in nested iframes - support cross-origin replaybase for embeds - add 'requirecrossoriginiframe' to restrict embed to only be loaded in subdomain iframes, otherwise showing - rename noSandbox -> sandbox, not enabled by default docs: add example embed to docs bump to 1.7.0-beta.1
update to latest wabac.js
add missing tweet-example.wacz
add embed.html to test embeds
@matteocargnelutti |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ikreymer Looks good to me! Tested on warc-embed
using a local dev build of embed-cors-check
.
I understand the reasoning behind the transition from noSandbox
to sandbox
.
Although most folks likely use versioned imports of ui.js
and sw.js
via jsdelivr
, I feel like an additional warning should be given, as this change has security implications.
Maybe a console.warn
could be issued when sandbox
is absent?
It could also be worth adding one for when noSandbox
is defined, as a deprecation notice.
Thanks for taking my feedback into account, excited for this new version of replayweb.page
.
@matteocargnelutti added a console.warn if |
noSandbox
->sandbox
, don't make default to support window target reuserequireSubdomainIframe
embed attribute to only allow embed if loaded from a cross-origin / subdomain iframe.This change makes the top-frame have a name, which allows the target rewriting to force new windows to open in the top frame instead. However, this can not work if sandbox attribute is set, so removing it by default, for better behavior of opening new windows in the same frame.