Remove sandbox attribute from iframe

[bgrgicak]

What is this PR doing?

It removes the sandbox attribute from remote.html.

What problem is it solving?

It allows PDF previews in Chrome.

How is the problem addressed?

In #1298 we enabled PDF loading in Playground, but it still didn't allow PDF previews in Chrome while it works in Firefox (try this blueprint).

@dennisnissle mentioned that it works without sandbox options.

After some testing, it turns out that any sandbox option will block PDFs from loading. I couldn't find any documentation on it but a comment from StackOverflow suggests the same.
There is a note in the MDN documentation about embedded documents: it is strongly discouraged to use both allow-scripts and allow-same-origin, as that lets the embedded document remove the sandbox attribute. I assume that Chrome does something different.

I also tried adding all sandbox options and just having a sandbox="". Both kept returning the same error, so the only solution I could find was to remove it.

Testing Instructions

• Chekout this branch
• Open this blueprint
• Confirm the PDF loads

[adamziel]

AFAIR the sandbox attribute was added to prevent links with target=„top” reloading the entire Playground tab (and failing to load WordPress). I think Sandbox also affected what’s window.top. It would be helpful to dig up the historical context before just removing it.

[bgrgicak]

AFAIR the sandbox attribute was added to prevent links with target=„top” reloading the entire Playground tab (and failing to load WordPress). I think Sandbox also affected what’s window.top.

Correct. With the sandbox attribute, target=top links fail, without it they open in the main window.

If this is the only reason we need the sandbox attribute. We could try to work around it by using beforeunload and preventing it. This would also allow us to show a custom error message, or even a modal to allow the user to decide what to do.

It would be helpful to dig up the historical context before just removing it.

The initial sandbox was added without context. It had one update to allow modals.

[adamziel]

If this is the only reason we need the sandbox attribute. We could try to work around it by using beforeunload and preventing it.

That would solve the navigation problem, but sandbox also prevents the JavaScript inside the iframe from escaping. For example, a WordPress plugin cannot call window.parent.history.push() – either on purpose or accidentally. I'm also worried about all the different ways of reloading the page, e.g. links, forms, scripts, touchpad gestures etc.

Let's file this as an issue and return to it once the major puzzle pieces are stable enough. PDF support would surely be nice, but there may be other solutions, e.g. conditionally removing the sandbox attribute when we're about to render a PDF.

The initial sandbox was added without context. It had one #679.

Aha! Sorry, my bad for not documenting it then.

[bgrgicak]

Here is the issue #1314
I'm closing the PR.