hardcode allowing to embed videos from ovs#2987
Conversation
frontends/main/src/page-components/TiptapEditor/extensions/node/MediaEmbed/lib.ts
Outdated
Show resolved
Hide resolved
| if (hostname === "video.odl.mit.edu") { | ||
| return url |
There was a problem hiding this comment.
Bug: The convertToEmbedUrl function returns video.odl.mit.edu URLs as-is, but they require transformation to an embed format (e.g., adding /embed/) to work in an iframe.
Severity: HIGH
Suggested Fix
Modify the convertToEmbedUrl function to handle video.odl.mit.edu URLs. The logic should extract the video ID from the path and construct a new URL in the correct embed format, https://video.odl.mit.edu/videos/{VIDEO_ID}/embed/, similar to how YouTube and Vimeo URLs are transformed.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location:
frontends/main/src/page-components/TiptapEditor/extensions/node/MediaEmbed/lib.ts#L48-L49
Potential issue: The `convertToEmbedUrl` function identifies URLs from
`video.odl.mit.edu` but returns them without modification. For these videos to be
embeddable in an iframe, the URL must be transformed into the format
`https://video.odl.mit.edu/videos/{VIDEO_ID}/embed/`. The current implementation will
pass the original, non-embeddable URL directly to the iframe's `src` attribute. This
will result in a broken or blank video embed for any user attempting to embed a video
from this service using a standard video page URL.
There was a problem hiding this comment.
People will paste the embed video, we may end up allowing m3u8 directly also subsequently, so let's leave this as is
| } | ||
|
|
||
| // --- ODL VIDEO EMBED --- | ||
| if (hostname === "video.odl.mit.edu") { |
There was a problem hiding this comment.
Might change / better organization this validation. See https://github.com/mitodl/mit-learn/pull/2968/changes#r2860467411 for discussion.
is an effort to support video embeddings directly in the article editor without iframes. We need this soon, though, and we need it with transcripts and cover images, so we are going to use the OVS video embed page for now.
There was a problem hiding this comment.
I guess I should mention that the thumbnail and captions aren't actually working at https://video.odl.mit.edu/videos/5444a783abb94d1fb773ff390d21a603/embed/ (The data exists, but isn't hooked up to videojs quite right), but Matt is working on fixing that and it seems like a much smaller lift than adding m3u8+Captions+cover images here.
|
I wasn't able to get this working. Firstly, the 2026-02-26.15-53-46.mp4I rebuilt all my containers before this on this branch, so it should have everything it needs in theory. |
gumaerc
left a comment
There was a problem hiding this comment.
After getting the correct embedding URL (https://video.odl.mit.edu/videos/5444a783abb94d1fb773ff390d21a603/embed/) I was able to successfully embed the video. It wouldn't play back in the article editor, but after publishing I was able to click on it and play it back, resize the window, etc. and everything worked fine.
What are the relevant tickets?
None
Description (What does it do?)
This PR adds video.odl.mit.edu as a potential media embedding source for the New editor
Screenshots (if appropriate):
How can this be tested?
/news/new