Enhance sandbox component to accept sandbox prop

[Imran92]

What?

Adds an optional allowPopups prop to the SandBox component, letting consumers opt into allow-popups in the iframe's sandbox attribute.

Why?

The iframe in the SandBox component does not include allow-popups in its sandbox value. Because of this, links inside sandboxed content don't take users anywhere when clicked — the navigation is blocked.

Our actual goal is to use this in the Jetpack repo to fix the VideoPress blocks. Jetpack consumes the @wordpress/components npm library, so this needs to be released on npm before we can move on with the next part of the fix.

How?

Since this PR was opened, SandBox was significantly refactored (#77212) and is now split into two internal variants, dispatched via the allowSameOrigin prop:

• IsolatedSandBox (default) — sandbox="allow-scripts allow-presentation". allow-same-origin was deliberately dropped for user-controlled content (e.g. the HTML block) as a security hardening.
• SameOriginSandBox (allowSameOrigin) — sandbox="allow-scripts allow-same-origin allow-presentation", for server-fetched oEmbed previews.

Neither variant includes allow-popups. Rather than expose a raw sandbox string override — which would let a caller pass allow-same-origin back into the isolated, user-controlled variant and quietly undo that hardening — this PR adds a targeted, opt-in allowPopups?: boolean prop (@default false). When set, allow-popups is appended to whichever variant's default sandbox attribute. This keeps with trunk's boolean-flag design (alongside allowSameOrigin) and preserves the existing security guarantees.

Testing Instructions

1. Check the code and tests.
2. Make sure any block or other consumer of SandBox doesn't break (default behavior is unchanged when allowPopups is not passed).
3. Render a SandBox with allowPopups and confirm the iframe's sandbox attribute includes allow-popups, and that a link inside the sandboxed content can open.

Testing Instructions for Keyboard

N/A

Screenshots or screencast

N/A

[github-actions]

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: Imran92 <imranh920@git.wordpress.org>
Co-authored-by: obenland <obenland@git.wordpress.org>
Co-authored-by: jsnajdr <jsnajdr@git.wordpress.org>
Co-authored-by: Mamaduka <mamaduka@git.wordpress.org>
Co-authored-by: mirka <0mirka00@git.wordpress.org>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

[Mamaduka]

Thanks, @Imran92!

The https://github.com/Automattic/videopress/issues/1217 link leads to 404 packages. Please don't use private links or dead links for other repositories as references.

[Imran92]

Thanks @Mamaduka , I've removed the links

[obenland]

I went ahead and refreshed the PR and update the approach to use allowPopups and not undo the changes from #77212.

@Mamaduka Would you be a good person to ask for a review of this PR?

[mirka]

I see there are many allow-* options that a consumer could want to customize. It doesn't seem practical to add new props for each one. Are popups indeed a very special case, or should the API account for the possibility of all the other allow-* options?

[Mamaduka]

We could allow passing additional sandbox props, which will be merged into the existing ones. Enabling overriding the entire iframe sandbox prop is also an option, but it's probably better to keep the defaults.

[jsnajdr]

We could allow passing additional sandbox props, which will be merged into the existing ones.

The sandbox prop syntax is very similar to className, even to the point that we could use clsx to build the value:

const sandbox = clsx( 'allow-scripts', 'allow-same-origin', 'allow-presentation', {
  'allow-popups': allowPopups,
} );

But handling such a string list inside the component is a bit unwieldy. We'd have to parse it, fork the behavior completely on presence of allow-same-origin, and add the allow-scripts and allow-presentation which are enabled by default. That's another issue: how to day that a token that's present by default should be disabled?

Just defining a new allowPopups prop is much easier. And we could also have allowScripts and allowPresentation that are true by default, but are overridable.

There is the downside that @mirka mentions: there are currently 14 allow-* tokens and new ones will be probably introduced. I think it's an OK compromise though.

[jsnajdr]

I think that API-design-wise the allowPopups prop is OK, although not ideal. I'm approving to signal that the entire PR is OK. But let's continue discussing if there are disagreements.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[obenland]

let's continue discussing if there are disagreements.

@mirka @Mamaduka Any other thoughts before this can move forward?

[Mamaduka]

The clsx suggestion by @jsnajdr will be a nice improvement.

I would prefer improving general support for allow-* tokens, but understand allowPopups is an easier route, and we don't really have requests for other tokens.

Let's rebase, resolve conflicts and pipe tokens through clsx. Then I think it's good to land.

[obenland]

Thanks @Mamaduka! Clumsily rebased onto trunk, resolved the conflicts, and the sandbox attribute is now built with clsx in both sandbox variants.

[Mamaduka]

Looks good to me 🚢