search

Opencast-Moodle / moodle-filter_opencast

Filter to embed Opencast videos in Moodle content
GNU General Public License v3.0
9 stars 9 forks source link

More aggressive URL parsing from video element #35

Closed f0k closed 6 months ago

f0k commented 1 year ago

In my first attempt to add an OpenCast video to a Moodle text page, Moodle created the following HTML code:

<video controls="true">https://host/path/file.mp4</video>

Interestingly, Firefox renders this as a video element, even though the content of a <video> tag is only meant as a fallback for browsers who do not know or want to render the <video> element at all. However, the OpenCast filter does not recognize this pattern, so I (wrongly) assumed my university had done something wrong.

In all future attempts, Moodle created HTML code as follows:

<video controls="true"><source src="https://host/path/file.mp4">https://host/path/file.mp4</video>

This is the only pattern supported by the filter.

There is another valid pattern for videos:

<video controls="true" src="https://host/path/file.mp4">anything</video>

I.e., if there is only a single source, it can be specified as an attribute on the <video> element. This pattern is also unsupported.

There are four options to take:

  1. Leave everything as is, since Moodle should only create the second kind of pattern, and I cannot reproduce how I got the first pattern.
  2. Add support for the first pattern, since there is anecdotal evidence that Moodle can create this pattern.
  3. Add support for the third pattern, since this is a valid way of embedding a video, but do not support the first pattern, since technically it should not render a video at all.
  4. Add support for the first and third pattern to be most permissive.

I'm not advocating for any of these options, I just wanted to document that there is a potential problem. If I ever manage to reproduce the first pattern in Moodle, I'll report back here.

justusdieckmann commented 6 months ago

Hey @f0k,

thank you very much for writing this issue! I decided to implemented option 3 and additionally added support for replacement of links, so there is no problem if users forget to switch the tab in the 'embed media' popup. The parsing should now be a lot more reliable (especially with newlines within / between tags) and I think only fail in cases like <video title=">" src="..."></video>, which might be such an edgy edge case, that it is acceptable.

Cheers Justus