Refactor video plugin

[7Ds7]

After checking the PR #2110 i decided to take a look at Shaarli codebase to get myself familiar with it and check if i could help in anyway.

For the project to run properly on my machine i had to update a few packages (both composer and yarn) which might be a good thing as now the packages pass the audits (outdated libraries with CVEs)

Indeed the mentioned plugin although simple it is quite obfuscated with minified versions of other plugins within it, i usually do not use this plugin in my instance but i think this refactored version has the same functionality as the current one.

• adds a button to the top bar to play videos
• creates a "modal" with a player
• lists next/prev in the videos
• closes the modal

TLDR: This PR refactors the video plugin to a more modernized and non obfuscated code as well as updates packages that were outdated and/or failed audits.

[thican]

Hello, thanks for your contribution, does it also work with an hardened CSP, as you saw in PR #2110?

[thican]

Also can you squash some commits, as the revert only fix the modification introduced in this PR.

[nodiscc]

updates packages that were outdated and/or failed audits.

Please keep these unrelated changes for a separate PR, it makes reviewing easier (I think you could just discard/revert 99499b9)

[7Ds7]

Hello, thanks for your contribution, does it also work with an hardened CSP, as you saw in PR #2110?

Yes @thican it makes the same requests as before gets the script from youtube.com, and plays the videos within a iframe from youtube.com

On a different note and if this ends up being accepted/merged you might want to cherry pick your commit over here

[7Ds7]

updates packages that were outdated and/or failed audits.

Please keep these unrelated changes for a separate PR, it makes reviewing easier (I think you could just discard/revert 99499b9)

Made a separate PR for the packages

[nodiscc]

Thank you, great work rewriting this plugin (basically from scratch?)
I have looked at the diff and see nothing wrong, however this needs to be tested, I will run some basic tests when time allows (other testers are also welcome)

Regarding feature parity with the old version of the plugin, not much is expected, click the plugin button, a video embed/overlay appears with working video controls, next/previous buttons, that's about it

[nodiscc]

Works fine, thank you!
There's an old issue about adding soundcloud (and/or other hosts) support to that plugin, if you feel up for it #1454

feelsgood.jpg

[nodiscc]

I'm sorry, one last thing. Can you please drop 345a61228282c2c4ae487cb150bffe33b362c995 (merge commit) so that this PR only contains the 2 relevant commits?

I'd like to keep the history as linear as possible (I have also considered switching to rebasing/squashing all PRs but that's for another day)

[thican]

I'd like to keep the history as linear as possible (I have also considered switching to rebasing/squashing all PRs but that's for another day)

I also prefer rebasing but only done by the main author, so stuff like signatures remain valid.

[thican]

I'm sorry, one last thing. Can you please drop 345a61228282c2c4ae487cb150bffe33b362c995 (merge commit) so that this PR only contains the 2 relevant commits?

And even the "2 relevant commits" should 100% be squashed.

[7Ds7]

@nodiscc @thican

The history for this plugin was already lost more than once due to all the squashing, i am pretty sure that there are better flows than this, i am in favor of not loosing any history and i am in favor of merging the main branch often but that is on my own/team projects. Maybe there should be a consideration of adding some of these code guidelines upon here ? or maybe it was me that did not found them and they do exist?

Same goes for linting, and linting rules, this already broke the plugin due to the enforce of 'class-methods-use-this' (my bad for not spotting it before) in which i do not think it should be enforced at all due to the nature of the project i just spotted it and disabled this rule inline and removed the static methods from the class here

Regarding the soundcloud/vimeo situation, i added for now at least, the shortened youtu.be urls and sort of prepared code-wise so that we can add more.

There seems to be soundcloud widget and a vimeo sdk that we can integrate and they both seem to have a event for "media ended" that we can use to keep playing the media after this one is ended.
I can probably take a look at it when i have the time but i would do it on another pull request and probably after some discussion about it, some questions that come from the top of my mind are:

• will the plugin be replaced for example media_playlist?
• will the users that already use youtube playlist feel awkward that will now play other media?
• should this services be opt-in ex: the user chooses which they want to be included ? (might be a bit of a pickle to do that)

[nodiscc]

Discussions about merge style should go in a separate issue, let's not overcomplicate this one. Thanks for dropping the merge commit.

Regarding possible vimeo/soundcloud integration, I wll reply in #1454

Thanks again