Problem/Motivation

This tracks the need for a local library version instead of the CDN version, e.g. for privacy / GDPR reasons.

At videojs the request is tracked here and was added to the roadmap:
https://github.com/videojs/v10/issues/828

For Drupals privacy focus this would be very helpful! I'd even vote to make it default and opt-into the CDN to be safe. But that's a maintainers decision.

Steps to reproduce

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

Comments

anybody created an issue. See original summary.

anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented
Title: [4.x] Provide library locally (as alternative to the CDN) » [4.x] Load videojs v10 html library locally (as alternative to the CDN)
semiaddict’s picture

semiaddict commented

Hi,

I just pushed a commit on my 4.x branch to allow loading local library files from /libraries/videojs/dist.

Here's how I managed to generate the dist files for Video.Js :

  1. Install pnpm
  2. Clone the videojs v10 repository
  3. Run "pnpm build:cdn"

Once the build done, make sure to copy all the files from packages/html/cdn to libraries/videojs/dist.
You should then have a bunch of files in there including audio-minimal.js, audio.js, video-minimal.js and video.js.

anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented

Whao VERY mice @semiaddict!

Maybe you could put this information into the README.md of your branch additionally?
And maybe you'd like to apply as co-maintainer in this module?

yannickoo’s picture

yannickoo
Location Berlin
commented
Status: Active » Needs review

Hey 👋 I have added a MR against the 4.x branch which makes use of the npm-asset/videojs--html Composer dependency so people don't need to build anything manually – https://git.drupalcode.org/issue/videojs-3577815/-/merge_requests/1

anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented
Status: Needs review » Needs work

Nice work @yannickoo that looks great! Did you already test it manually?
I'm wondering if there's any way we can add a test to ensure it works.

And I think

"merge-plugin": {
    "include": [
        "web/modules/contrib/*/composer.libraries.json"
    ]
},

documentation should be added to the README.md?

In modules like photoswipe we're also using the status report to show the presence of the library or using the CDN. We might also want to add a "Use CDN" checkbox and use the local library by default to ensure you don't use CDN accidentally?

yannickoo’s picture

yannickoo
Location Berlin
commented

Yeah I had those two topics on my radar but realized the complete README.md needs an overhaul :D

I have tested this locally and then realized I need more customization to show a responsive thumbnail which made me creating my own "lazy-loaded video player" Single-Directory Component to achieve that.

There are many things we could provide here in this module to make it work in a really nice way but first we need to make that initial 4.x dev version happen.

anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented
Title: [4.x] Load videojs v10 html library locally (as alternative to the CDN) » [10.x] Load videojs v10 html library locally (as alternative to the CDN)
_shy’s picture

_shy
Primary language Ukrainian
Location Ukraine, Lutsk 🇺🇦
commented
Version: 3.x-dev » 10.x-dev
anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented
Status: Needs work » Postponed

We need to wait for the VideoJS beta27 release: https://github.com/videojs/v10/tags
GitHub issues say that it should contain the sources we need.

grevil’s picture

grevil commented
Status: Postponed » Fixed
Issue tags: -GDPR

@anybody v27 released and the asset is now included in the composer.libraries.json and seperate manual installation is also documented. We shouldn't use the "npm-asset/videojs--html" for now, as it ships with unnecessary dependencies on the rest of the videojs v10 eco-system.

Fixed through #3577815: [10.x] Prepare 10.x branch for Video.js 10 implementation.

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

grevil’s picture

grevil commented

Added contribute record for @yannickoo for the base implementation! Thanks! 🙂👍

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.