@chamilsanjeewa thank you for trying out the fork. I don't have commit access to this module so I can't merge into it. The intention is to keep the fork only while waiting for the maintainers to commit the patches to the module, and then I would delete the fork. The error messages you posted are related to downloading the H5P libraries since your site is searching for files in the ./default/files/h5p directory.
shaundychko → created an issue.
With the 2.0.x version of this module, Drupal 10.4.1, and updated h5p dependencies, this is no longer an issue.
shaundychko → created an issue. See original summary → .
💬 Support h5p/h5p-core 1.27 and h5p/h5p-editor 1.25 in H5PDrupal.php Active is the current place to support upgrading the dependencies.
Duplicate of 🐛 PHP 8.3 dynamic properties deprecated warnings Active .
This works great, thank you very much.
Yes, the composer.lock would have that effect of locking the version despite changing composer.json. That issue didn't show up for me since I also applied 📌 Remove vendor directory and composer.lock from project repository Active . The vendor dependencies should not be committed to the module.
This is a duplicate of 🐛 Remove the a11y_title field when updating H5PContent Active , but the patch here is better so I closed the other issue.
A new issue with a better patch was opened here: 🐛 Support a11y_title from h5p-core 1.27 Needs review . Closing this one of favour of that.
Duplicate of 💬 Support h5p/h5p-core 1.27 and h5p/h5p-editor 1.25 in H5PDrupal.php Active .
Thank you very much for this.
Already updated in 2.0.x
This patch fixes issues in an old version of the h5p/h5p-core dependency. That has already been fixed in later versions of the dependency. To use the latest version of the dependencies apply patches from the merge requests at 💬 Support h5p/h5p-core 1.27 and h5p/h5p-editor 1.25 in H5PDrupal.php Active and 📌 Remove vendor directory and composer.lock from project repository Active . The latter adds a build step using Drush to copy the current version of JS/CSS assets into the module directory.
This issue is outdated. The patch here is trying to two things at once: remove the vendor directory, and update the h5p dependencies. This is too much for one patch since opinions might vary on how the vendor directory should be removed, whereas updating the h5p dependencies should be needed by most sites by now due to current versions of PHP not supporting older versions of h5p/h5p-core without warnings.
Apply the patches from the merge requests from 📌 Remove vendor directory and composer.lock from project repository Active , and 💬 Support h5p/h5p-core 1.27 and h5p/h5p-editor 1.25 in H5PDrupal.php Active .
Merge request 27 removes the vendor directory and adds a build step using a custom Drush command drush h5p:copy-assets to copy assets, such as JS and CSS, to ./assets/h5p-core and ./modules/h5peditor/assets/h5p-editor. The Drush command outputs every source/destination copied. The Readme.md is updated to document the Drush command.
This MR doesn't deal with updating the version of the h5p dependencies. It focuses exclusively on removing the vendor directory and creating a post-install build step using Drush.
The merge request adds to the provided patch by also increasing the version requirement of h5/h5p-core in composer.json in order to automatically retrieve a version of \H5PFrameworkInterface that is compatible with the updated H5PDrupal class.
shaundychko → made their first commit to this issue’s fork.
Thank you all for your help.
shaundychko → created an issue.
Re-rolled to support 3.0.0-beta5. Submitting a patch or MR for the 4.x branch is a remaining task.
shaundychko → changed the visibility of the branch 3312958-authentication-locking-prevent-multiple to hidden.
MR 19 should be a different issue since it seeks to address a performance issue rather than this issue of duplicate users.
shaundychko → changed the visibility of the branch 3312958-users-with-duplicate to hidden.
ShaunDychko → created an issue.
Jquery is included separately when aggregation is enabled.
These deprecations are fixed in the core h5p library version 1.26. Here's the github issue: https://github.com/h5p/h5p-php-library/pull/148. After sorting out how to update the core library used in this module (see 🐛 Support h5p/h5p-core:1.26 and h5p/h5p-editor:1.25 Needs work ), this issue will be fixed.
ShaunDychko → created an issue.
Nevermind, I was looking at the wrong branch.
ShaunDychko → created an issue.
Created a patch from the merge request above so that it can be applied automatically by composer.
`h5p_site_type` no longer exists and was removed here: https://git.drupalcode.org/project/h5p/-/commit/78c2fba9f9c8c0a02e9c7e15...
The updated patch adds only h5p_first_runnable_saved.
Yes, that makes sense. There are lots of code style issues and it might be easier to address them when the module is more stable.
The solution here of checking for the route name is more reliable than using regex as proposed in the related issue here: https://www.drupal.org/project/h5p/issues/2612208#comment-12476851 →
The D7 issue is #2612208: embed.php Cross-domain policy option →
ShaunDychko → created an issue.
Consolidate patches into 1 file.
'git format-patch' output this as two separate patch files that need to be applied in sequence, starting with support-h5p-1.26-3420268-10-1.patch.
The embed.php files needs to still be in the module's /vendor/h5p/h5p-core directory since a reference to it is hardcoded in \Drupal\h5p\Controller\H5PEmbed.
It's a low priority since this module doesn't have automated testing, but best practice is to inject the Drupal service instead of calling it statically. This is more of a code style issue (and maybe I should have opened a new issue.)
🐛 Support h5p/h5p-core:1.26 and h5p/h5p-editor:1.25 Needs work updates h5p dependencies to support PHP 8.
Version 1.26 of h5p/h5p-core has this function. This dependency is added in 🐛 Support h5p/h5p-core:1.26 and h5p/h5p-editor:1.25 Needs work .
h5p/h5p-editor:1.25 supports PHP 8. Applying the patch at 🐛 Support h5p/h5p-core:1.26 and h5p/h5p-editor:1.25 Needs work updates the module to use version 1.25 of h5p-editor.
Does applying the patch #8 at 🐛 Support h5p/h5p-core:1.26 and h5p/h5p-editor:1.25 Needs work close this issue?
Since this can be postponed until Drupal 11 support, marking it as Normal instead of Critical.
The latest 2.0.x already support PHP 8. The issue of removing libraries is a duplicate of 📌 Remove vendor directory and composer.lock from project repository Active .
After applying the patch here 🐛 Support h5p/h5p-core:1.26 and h5p/h5p-editor:1.25 Needs work , to use H5P 1.26 on the 2.0.x-dev version of this module, there doesn't seem to be an issue with serving over SSL. I also couldn't find any URL's starting with 'http://' hardcoded in the module folder. It doesn't seem like this patch is necessary any more.
The previous patch created using git diff didn't apply correctly. The patch in comment #8 was created using git format-patch.
Here's a patch file for composer to automatically apply.
The injected file_system service is used in the merge request.
ShaunDychko → changed the visibility of the branch 3266331-use-of-deprecated to hidden.
ShaunDychko → made their first commit to this issue’s fork.
Duplicate of 📌 Remove vendor directory and composer.lock from project repository Active
After applying the merge request at 🐛 Support h5p/h5p-core:1.26 and h5p/h5p-editor:1.25 Needs work H5P is working on a new install, tested with creating an interactive video, on Drupal 10 with PHP 8 and H5P 1.26.
The /h5p/vendor/ directory can't really be deleted since not only are so many CSS and JS files referenced in code, but it's also in the resize JS link in embeds, so removing it would somewhat break other sites that have embedded an H5P. Also, the composer.json needs to lock down a specific version of the dependencies in order to have a match between the dependencies included in the module's /vendor and the dependencies installed by composer in the project's root /vendor. The merge request in 🐛 Support h5p/h5p-core:1.26 and h5p/h5p-editor:1.25 Needs work deletes the autoloader and all php files included in the module, and keeps the composer.lock file in order for the version installed in the project /vendor to match the CSS and JS shipped with the module's /vendor directory.
ShaunDychko → changed the visibility of the branch 3420268-support-h5ph5p-core1.26-and to hidden.
The /h5p/vendor/ directory can't really be deleted since not only are so many CSS and JS files referenced in code, but it's also in the resize JS link in embeds, so removing it would somewhat break other sites that have embedded an H5P. Also, the composer.json needs to lock down a specific version of the dependencies in order to have a match between the dependencies included in the module's /vendor and the dependencies installed by composer in the project's root /vendor. The patch in 🐛 Support h5p/h5p-core:1.26 and h5p/h5p-editor:1.25 Needs work deletes the autoloader and all php files included in the module, and keeps the composer.lock file in order for the version installed in the project /vendor to match the CSS and JS shipped with the module's /vendor directory.
ShaunDychko → created an issue.
The latest 2.0.x dev release supports Drupal 10.
Closing since fixed in 2.0.x
See https://git.drupalcode.org/project/h5p/-/blob/2.0.x/modules/h5peditor/sr...
and https://git.drupalcode.org/project/h5p/-/blob/2.0.x/modules/h5peditor/sr...
The fix here is included already in
🐛
json_decode(): Passing null to parameter #1 ($json) of type string is deprecated
Needs review
Compare the commit to 2.0.x https://git.drupalcode.org/project/h5p/-/commit/43daa83a9d3a2988730965c4... with the patch in this issue.
Thank you very much for spotting that.
Fixed in 📌 Automated Drupal 10 compatibility fixes Fixed
ShaunDychko → made their first commit to this issue’s fork.
ShaunDychko → created an issue.