Commit 7d9dd51cf4ac5947bbf15ff5d2b263ec8e1f72e9 refactors the sanitize method so that width and height attributes are no longer removed from img tags alone but instead are blacklisted and therefore removed from all tags. This change means embedded iframes, such as YouTube, no longer size well. On Edge (Chromium) and Firefox ESR iframes rendered in TT-RSS end up being 150 pixels tall, which is far too short to be useful when viewing. I’m not sure how this change affects other browsers.
I can change this with a plugin, of course, but I’m wondering if the decision to remove these attributes from all tags also considered iframes (and probably video) tags as well?
e: In thinking about it, removing these attributes is a good idea. A solution could be implemented through CSS to better size iframe and video tags, no?
i was thinking mostly about things like feeds setting absurd width/height and breaking layout, which probably isn’t a big enough deal. maybe iframe and video tags should be excluded from this blacklisting altogether?
not sure about this. iframes could be of any size though and contain whatever, not just a video player, it’s really hard to size them properly (and with a correct aspect).
Yeah, I guess iframes could be non-video; I don’t come across a lot of those so it slipped my mind.
Nevertheless, while I can’t speak for every site, ones like YouTube will maintain an aspect ratio within the iframe and just letterbox the video. Keeping a 16:9 or 4:3 aspect ratio should be usable.
Frameworks like Bootstrap wrap embedded objects so they can responsively scale. This would require specific handling when rendering the article though.
btw, do we need this for videos? i haven’t noticed them behaving weirdly.
that’s an interesting find, i wonder if this would work properly with tt-rss, it shouldn’t be hard to integrate then (basically just add to CSS and add a small sanitize tweak to wrap the elements).
that base CSS is seemingly not enough, you also need to use aspect ratios or something, this seems it won’t fit us:
one thing that messes with iframes is min/max-width in cdm.css, it might need to be removed.
e: my only feed with iframes in it doesn’t set any width/height attributes at all so without aforementioned stuff in cdm.less iframe shows up properly (i.e. not squished) but small.
forcing something like this could work but it would be probably better placed in user css if needed
I’m coding a few other things at the moment, but I’ll take a look at this when I’m done. I’ve used the Bootstrap styling before (albeit awhile ago using v3, not v4) with success but I’m not sure how much work it is to bring it into an existing project.
I’ll tinker and post back with the results.
e: I just threw in the video tags when I was posting this, sorry I wasn’t more clear. My thought was that if we’re making iframes size responsively it would make sense to include video tags in there as well. I wasn’t sure how video tags would show up (I don’t have any feeds that make frequent use of those tags). Based on your feedback I guess it would make sense if those actually worked properly since they’re rendered just like any other block element, whereas the video in iframes is constrained by the size of the iframe and is somewhat more isolated.
I did experiment with handling video tags like this as well but, as you mentioned, they scale fine already. (The only difference with video tags is on a large enough screen videos stop scaling up once the maximum width is reached, whereas iframes (with these changes) will always fill the available width.)
The change requires moving the iframe tag inside a div, which I did after sanitizing but before highlighting words. It needs to happen after sanitizing since it needs the CSS class on the div.
