🇨🇦Canada @shaundychko

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.