omeka / Omeka

A flexible web publishing platform for the display of library, museum and scholarly collections, archives and exhibitions.
http://omeka.org
GNU General Public License v3.0
472 stars 193 forks source link

Add iframe allowfullscreen attribute to HTML purifier #1015

Closed dicksonlaw583 closed 5 months ago

dicksonlaw583 commented 6 months ago

This prevents the allowfullscreen attribute from being erased in <iframe> tags, which is needed for many types of embedded viewers and players.

Resolves #942

zerocrates commented 6 months ago

That's an upstream library that gets updated, so we don't want to have changes directly to it.

There's options for us to allow the attribute without modifying the library. You might consider making this suggestion/contribution more in the form you have it here to HTMLPurifier itself though.

dicksonlaw583 commented 6 months ago

@zerocrates:

I tried allowing the attribute by adding iframe.allowfullscreen or *.allowfullscreen to Admin > Settings > Security > Allowed HTML Attributes, but even then the attribute keeps being erased after saving. Please see #942 for details. Though that ticket came from April 2021, I am still able to replicate it in 3.1.2. Do you know of any other ways to work around this?

zerocrates commented 5 months ago

So the change you're making, or something like it, is what needs to happen: if HTMLPurifier doesn't know about an attribute, you can't add it using the security settings.

What I'm saying is, we don't want to go editing our copy of HTMLPurifier as it complicates future upgrades to newer versions of the library. Getting HTMLPurifier itself to support the attribute is one way around that problem, as we'd then just pull in the updated version from them. Their legacy support for object has a flag for allowing fullscreen, so I could imagine they'd accept something similar for iframes.

Now, staying within Omeka itself, there's still an option for varying the allowed attributes without modifying the library. Our code that uses the purifier lives in Omeka_Filter_HtmlPurifier, and you can use HTMLPurifier's API for adding an attribute there.

dicksonlaw583 commented 5 months ago

I see. While working on the issue last week, I settled on creating a quick plugin for it to solve my use case: iFrame Allow Fullscreen

Since this is an upstream dependency not related to Omeka's core, I will close this PR for now.