Ability to add attributes to iframe

humhub / custom-pages

Create custom pages and widgets and share them with your users. Take advantage of a wide range of editing options, including HTML and Markdown.

27 stars 51 forks source link

Ability to add attributes to iframe #178

Open superseby2 opened 3 years ago

superseby2 commented 3 years ago

Hello,

It would be nice to be able to add custom attributes to iframe.

My use case : I need to add allow="camera"

It would be convenient as it is for adding css classes.

Thx !

marc-farre commented 2 years ago

Thanks @sebmennetrier for the PR. Is it planned to be integrated in the module? @luke- would you like me to test this PR and and see if there are possible enhancements?

luke- commented 2 years ago

Sorry for my late reply here.

I would prefer in this usecase (needed additional iframe attributes) that simply create a HTML page with an extended customized IFrame.

The current PR adds a bit too much complexity in my opinion (e.g. ~30+ new translations) for a very exotic benefit (e.g. Allow USB access). Otherwise I would prefer a more lightweight approach, e.g. add a field with "Additional IFrame Attributes". (security?)

Also, with the PR, it's unfortunately a bit problematic that the translation files are comitted. (I'll update the documentation here.).

marc-farre commented 2 years ago

It's indeed possible to insert an iframe in a HTML page (e.g. editing the source code), but the iframe page type has setSize() in Javascript allowing to show the iframe in full screen. So I suggest to keep using the iframe page type.

What to you think about this way of adding attributes?

Then, we need to save theses attributes in the database. Should we:

Add a column in the custom_pages_page, e.g. iframe_attr?
Or create a new table related to the custom_pages_page, e.g. custom_pages_page_iframe?

I can send a PR for that.

luke- commented 2 years ago

This way for the attributes would be so good!

Are there any security concerns if regular space admins could do something like this? e.g. onClick="..."

Maybe we should enable the field for global admins only?

Cleaner would be of course really a separate table for custom_pages_page_iframe.

marc-farre commented 2 years ago

@luke- https://github.com/humhub/custom-pages/pull/243 I named the table custom_pages_iframe_attr (with object_modeland object_id columns) and not custom_pages_page_iframe because otherwise we would need 4 tables (for custom_pages_container_page and 2 more for snippets). If you think to a better name or something else to change don't hesistate!

luke- commented 2 years ago

@funkycram Thanks for the PR. The extra table really adds more complexity than I thought. I'm thinking if it wouldn't be easier and more maintainable in the future to just add a field "iframe_attrs".

What do you think?

marc-farre commented 2 years ago

Yes! This is the new PR: https://github.com/humhub/custom-pages/pull/246