GoteoFoundation / goteo

Goteo Version 3, the Open Source Crowdfunding Platform
http://goteo.org
GNU Affero General Public License v3.0
188 stars 133 forks source link

Support independent video platforms #150

Open ekaitz-zarraga opened 4 years ago

ekaitz-zarraga commented 4 years ago

Hi,

As only Youtube and Vimeo are supported for video sharing I open this issue as a discussion for adding support for any independent video platforms (like peertube, Archive.org or any other).

I've been taking a look to the code and I found some possible ways to solve this I'd like to discuss:

This first file looks like it's a Video player or similar: https://github.com/GoteoFoundation/goteo/blob/0caff6df3ce81942615c9feb7ce64eb8e473109c/public/assets/vendor/summernote/src/js/base/module/VideoDialog.js#L133 It literally says you don't know what to do if the video doesn't belong to any of those platforms (which are quite a lot more than the officially supported). My suggestion is to remove the big if it has on top and just admit any src attribute for the iframe. That way any video platform with support for iframes will be available to use. In order to do that, the content of the identifier should be changed from just the id to the full URI, but I don't think that's a problem. Problem may arise when removing the custom style that each of the platforms have, but I don't see big differences between them so adding fixed common style should be more than enough. If needed, a regexp based approach can be taken on the full src and adjust the style depending on the platform and leaving it like it is if the platform can't be deduced (this approach doesn't limit the origin of the videos).

Same thing here: https://github.com/GoteoFoundation/goteo/blob/ad05c0d9140a207c61102a332d7eeacb8e5bc5ef/src/Goteo/Model/Project/Media.php#L16 Accepting a full src should be more than enough and would simplify many of the logic.

What I don't really know is if there's any external limitation for this. Could you clarify if there's any?

The only thing I can think about is the database model and how to handle the migration to a src based approach rather than an id based one.

Best, Ekaitz

microstudi commented 4 years ago

In principle, it is a good idea, in practical terms we might encounter some problems:

First, Some platforms may need to specify custom attributes in the iframe, not just for the styles, if scripts need to handle with this kind of content, that may be platform-specific. So, the "big if", will stay. What I would agree is to create a standard iframe src, just like you suggest, in the event that no platform is found.

One thing, however, summernote is an independent project, the correct way would be to make that contribution there, and then bring it here.

Second, Media is an internal library, so we can modify it directly. However I would stick with the same approach: Create a "fallback" iframe in the event that no provider is found.

About your generic question, why are those provider specified individually? This has to do with the ability to detect several cases referring to the same object (for instance if the user puts http:// instead of https://). Also, this way you can optimize some iframes according to the provider, which is important if you need to deal with it with scripts.

We will welcome any PR related to this, and further discuss it over the code. Thanks!

ekaitz-zarraga commented 4 years ago

In principle, it is a good idea, in practical terms we might encounter some problems:

Yeah, that's why I started the discussion because I'm was not aware of the details of the platform. Thanks for elaborating

One thing, however, summernote is an independent project, the correct way would be to make that contribution there, and then bring it here.

Or make a fork and keep it separated here. 😄

First, Some platforms may need to specify custom attributes in the iframe, not just for the styles, if scripts need to handle with this kind of content, that may be platform-specific. So, the "big if", will stay. What I would agree is to create a standard iframe src, just like you suggest, in the event that no platform is found. ... About your generic question, why are those provider specified individually? This has to do with the ability to detect several cases referring to the same object (for instance if the user puts http:// instead of https://). Also, this way you can optimize some iframes according to the provider, which is important if you need to deal with it with scripts.

And why not ask the user for the full iframe declaration including all the tags? Most of the video platforms have an embed button that shows the full iframe declaration, as you do here with the project widget. Users could copy-paste the full declaration and it could be stored as-is. That relies in the platform rather than the user and might be more secure, keep all the attributes set and be simpler for us as developers. The only problem I can find is security (injection of arbitrary 3rd party JS) but as far as the project is moderated and controlled by the administrators of the site and the users have to be registered to publish a project it's not a big deal. This feature should be well documented because it can be tricky, but that's not a big deal neither.

We will welcome any PR related to this, and further discuss it over the code.

Great. I'll try to arrange something if I find some free time.