Give videos a max height

[robintown]

Context: friends send portrait videos which, due to their height being unbounded, often end up being 1.5 to 2 times the viewport height! While #5682 sets a max height as well, the situation in the meantime is practically unusable, so I'm proposing to at least give them the same max height as images currently have.

Before	After

Type: defect

---

Here's what your changelog entry will look like:

🐛 Bug Fixes

• Give videos a max height (#6172). Contributed by robintown.

[jryans]

Code wise it's fine of course, so largely a Design choice.

[MadLittleMods]

@robintown Can you add some before/after's to make this easier for design to review?

[robintown]

Before:

After:

[janogarcia]

This doesn't seem to be the case anymore.

I just tested it with a regular 16:9 portrait video (1072 × 1920, 1:1.77) and an extremely narrow one (320 × 1280, 1:4).

Both, when rendered, are getting their HTML height attribute set to height="360", so I guess this has been updated at some point.

<video class="mx_MVideoBody" title="video-vertical-slice.MP4" controls="" preload="none" height="360" width="90" poster="blob:vector://vector/0000-0000" src="blob:vector://vector/0000-0000"></video>

The CSS height property for .mx_MVideoBody is still set to auto but that doesn't need to be updated there, given that we're rendering the proper HTML attributes for the content.

I'm closing this one, but let's reopen it if for some reason some <video> elements are not getting their HTML attributes set correctly.

[robintown]

@janogarcia Interesting, I can still reproduce the 'Before' photo using the same media on the latest develop version. However, it's probably better to leave this closed anyways, and instead come up with a solution to size videos like this.

[robintown]

Oh, and one more note: The height="360" probably came from the thumbnail metadata that was in the video event. To reproduce this, I believe you need a video event without any dimensions specified in its metadata.

[janogarcia]

So, I guess it's maybe a bug on the server side, where the video attributes couldn't be parsed properly on upload for some reason? The screenshot I shared is from an encrypted room in the current production desktop app (macOS).

If we provide a CSS height fallback value, to counteract the side effects of assets missing the HTML attributes, then that value should track/match that of the default value we're using for the HTML height attribute, instead of defining it in two separate places.

Thanks for pointing #19498 out!

[janogarcia]

@robintown Sorry, I didn't notice your latest comment before posting.

[robintown]

Yeah, the specific reason this video did not have dimensions specified was because it was sent over a bridge, but it's something that can happen in general since the video event is generated by clients, not the server.

[janogarcia]

I'm not sure if it's matching the video/poster dimension in any way or it's just a hard constraint set in place.

One of those videos I used for testing is 1072 × 1920, and the generated poster image is 335x600. None of those being 360 or a round multiple. Same happens for a ridiculously tall 320 × 1280 video. And, I just got the same result after uploading this square format video, a 360x360 player widget was generated.

So I guess this is best fixed in the source, where that 360 height constraint is being set instead of patching it somehow in CSS.

[janogarcia]

If that attribute is only available on videos uploaded directly through Element, but not on those coming from a bridge then I'd suggest figuring out how to sync that hard cap of 360 so that we have always the same value for both manual uploaded videos and bridged videos, or just about any video.

I mean, having a single source of truth for that value. That's why I wouldn't recommend just patching it in CSS without tying it back to where that 360 magic number is being defined for uploads. But that's out of my scope. @jryans