webrecorder / wombat

Wombat.js client-side rewriting library
GNU Affero General Public License v3.0
81 stars 31 forks source link

Iframes `allow` attribute gets overwriten with `"autoplay 'self'; fullscreen 'self'"` #133

Closed acelaya closed 4 months ago

acelaya commented 5 months ago

We have just noticed wombat is overwriting the allow attribute on iframes, instead of adding permissions on top of it. See https://github.com/webrecorder/wombat/blob/e75780e50aae36f0cb72ec0f41f2f5e208d03658/src/wombat.js#L1658

In our case, we have some logic that creates iframes with other permissions that needs to be kept. Ideally (unless I'm missing something) wombat should add autoplay 'self' and fullscreen 'self' on top of whatever is already set.

We can provide a PR changing this logic, unless there's a reason for this behavior.

acelaya commented 5 months ago

For others ending up here, we have found two workarounds for this:

The first one could potentially throw if using strict mode, when wombat tries to set the attribute, but it's easier to reason about and document.

ikreymer commented 5 months ago

Yes, that was likely an oversight, can definitely just add to the existing allow attribute. Would welcome a PR if you have already tested this!

acelaya commented 5 months ago

Yes, that was likely an oversight, can definitely just add to the existing allow attribute. Would welcome a PR if you have already tested this!

Thanks! I'll get something ready and sent.

acelaya commented 4 months ago

@ikreymer I created the PR some time ago, as discussed https://github.com/webrecorder/wombat/pull/134

Let me know if that's the right way to go.

ikreymer commented 4 months ago

Sorry for the delay, made a comment on the PR now, just wondering if simpler approach would also work, otherwise can do that.

acelaya commented 4 months ago

Sorry for the delay, made a comment on the PR now, just wondering if simpler approach would also work, otherwise can do that.

Thanks! I tried to clarify the reason I implemented it in that way. Let me know what you think.

ikreymer commented 4 months ago

Thanks for the updated PR. Released in 3.7.1!