Automated Drupal 10 compatibility fixes

Hello Team,

Created Patch compatible in Drupal 9 (9.5.7 ) and Drupal 10 (10.0.8).

Also uploaded the drupal-check status, all critical errors have been resolved.

Please review.

I have applied and tested the patch in comment #3, its getting applied successfully.
There are no drupal check and upgrade status errors found , also the functionality is working as expected.
The module is now compatible to D10 , thus moving the issue to RTBC.

It is working in Drupal 9.5.7 and it is also applying the patch for Drupal 10.0.0. But, it is not applying css in Drupal 10. So I am creating new ticket for Drupal 10 and marking this as RTBC. Please release alpha version of Drupal 10.

Patch applied successfully for me. Alpha version with Drupal 10 support would be great.

@NishaVaghela Is it possible that the CSS issue is caused by the Admin theme being used in Drupal 10 versus Drupal 9.5?
Drupal 9 uses Bartik as its default front-end theme, and Seven as its default admin theme.
In Drupal 10, Olivero will replace Bartik and Claro will replace Seven.

Is there a plan to have a Drupal 10 and PHP 8.1 release soon? We're hoping to beat the Drupal 9 end of life by updating in August and this is the last piece.

The problem with using a bunch of patches is that I can't have a composer file manage it. It will try and fail to reconcile the release H5P version that says it only works with Drupal 9, with my attempt to move up to Drupal 10.

@ryanrobinson_wlu I can see module having some issues in Drupal 10 as I cannot check functionality in Drupal 10, I will chck and update the patch if required..

I reviewed and tested and found issue with jQuery deprecated function"Uncaught TypeError: $(...).once is not a function" in drupal 10.

Fixed system.performance aggregation css issue

Fixed css issue come with aggregation

Fixed the following error.

ArgumentCountError: Too few arguments to function Drupal\Core\Asset\JsCollectionOptimizerLazy::optimize(), 1 passed in /app/web/modules/contrib/h5p/src/H5PDrupal/H5PDrupal.php on line 241 and exactly 2 expected in Drupal\Core\Asset\JsCollectionOptimizerLazy->optimize() (line 65 of /app/web/core/lib/Drupal/Core/Asset/JsCollectionOptimizerLazy.php).

It's early since applying it, but so far patch in #14 is working for me.

There seem to be two Merge Requests trying to make H5P ready for Drupal 10. This one, and the one in https://www.drupal.org/project/h5p/issues/3348114. Would it make sense to combine the two? Or better to simply get one of them ready and then see about cleaning up the other one?

Is there anything I can do to move this along? We want to upgrade to Drupal 10 in a few days, and now I see that we can't without heavily patching this module. End-of-life for Drupal 9 is in 8 days! Please let me know what the next step would be. I'm more than happy to help out with testing this and getting the MR ready.

I wholeheartedly second @Ammaletu's comment,

We have less than a week now before Drupal 9 will no longer have any further security updates and it has become quite site-builder-unfriendly to understand exactly which patches need to be applied in what order in order to get the H5P module to pass the 'Upgrade Status' tests.

This is concerning because H5P is such a key module and so brilliant that it's really not something that we can 'go without'!

Would it be at all possible for someone in the H5P maintenance team to find time to do either of the following (preferably no.1):

1. Release a new fully-patched up version that will pass the test.
2. Provide an idiots' guide (for idiots like me) to work through to get our local copies to pass.

Thank you with fingers crossed.

While I have commit access to this project, I am not really one of the maintainers, however the actual maintainers from H5P.org do not appear to be working on Drupal 10 compatibility and have previously said they'd be OK with me committing patches in the past (although I've not actually done that yet, this all happened with Drupal 8->9 compatibility). I need to update a client running the H5P module to Drupal 10 so am in the same situation as other people commenting here, just with the extra permissions to do a bit more about it.

I'm going to go ahead and close #3348114: Drupal 10 support since this patch seems a lot more viable (especially the once changes).

Still need to test this patch myself, but I've had confirmation from @jhuhta that they are running it on production (against 9.5) so that helps with confidence a bit.

Once I've been able to test, I'll aim to merge this MR, so that at least the dev release is Drupal 10 compatible.

If that is successful, I'll also tag an alpha release a bit later on. I do not really want to take responsibility for release candidate/stable releases and other changes to the project at the moment, since I would prefer the H5P team to do that. However, will see how it goes once the absolute minimum is done.

Brilliant, thank you @catch.
Blimey, what a profile, I am but worm by comparison ;-)

I cannot understand why on earth the actual maintainers are not working on it, it's seems mad given that it has such enormous potential for everyone, the HP5 Team included?

As soon as you get that dev release out there, I'll give it a good old test locally and report back.

Looks good to me, also tested the libraries and library update pages.

@SirClickalot - "Provide an idiots' guide (for idiots like me) to work through to get our local copies to pass."

I am an Acquia Cloud Next Customer on D10 with PHP 8.1 here is what I needed for a composer project to get a working h5p Installation to pass all my testing.

Modify your composer.json as follows.

Near the top in "repositories" exclude h5p and sent the issue fork as a repo source.

"repositories": [{
            "type": "composer",
            "url": "https://packages.drupal.org/8",
            "exclude": [
                "drupal/h5p"
            ]
        },
        {
            "type": "vcs",
            "url": "https://git.drupalcode.org/issue/h5p-3329297.git"
        },

I also needed the following patches, which are added to the "patches" area (assumes you have cweagans/composer-patches requires if not google it.)

            "drupal/h5p": {
                "fix broken images": "https://www.drupal.org/files/issues/2021-01-26/h5p-fix-keeping-h5p-content-id-3194707-0.patch"
            },
            "h5p/h5p-editor": {
                "PHP8 notice https://www.drupal.org/project/h5p/issues/3260094": "https://www.drupal.org/files/issues/2022-01-24/Update_H5P-editor_PHP8_compatible.patch"
            },

Then simply run `composer require "drupal/h5p:dev-3329297-drupal-10"` which should update your composer file to "drupal/h5p": "dev-3329297-drupal-10", the prefixed "dev-" tells composer its a branch and the "3329297-drupal-10" is the name of the branch.

That should get you a working h5p in D10

Thanks NicholasS, that's a really great of you to help.

I actually had at least some of that stuff in composer.json - the cweagans patches bit and an associated H5P patch but updating as you describe then threw up some other errors - damn it! ...

First of all, when I attempted to use composer require "drupal/h5p:dev-3329297-drupal-10 I hit a conflict with another H5P related module that I use...

  Problem 1
    - Root composer.json requires drupal/h5p dev-3329297-drupal-10, found drupal/h5p[dev-3329297-drupal-10] but these were not loaded, likely because it conflicts with another require.
  Problem 2
    - drupal/h5p_math_input is locked to version 1.1.0 and an update of this package was not requested.
    - drupal/h5p_math_input 1.1.0 requires drupal/h5p ^1.0@RC|^2.0.0@alpha -> found drupal/h5p[2.0.0-alpha1, 2.0.0-alpha2, 2.0.0-alpha3, 2.0.0.x-dev] but it conflicts with your root composer.json require (dev-3329297-drupal-10).

An attempt to then use drush cr resulted in...

PHP Warning:  require(C:\Users\nick\Sites\bit-by-bit.org\public_html\vendor\composer/../h5p/h5p-editor/h5peditor.class.php): Failed to open stream: No such file or directory in C:\Users\nick\Sites\bit-by-bit.org\public_html\vendor\composer\autoload_real.php on line 45
PHP Fatal error:  Uncaught Error: Failed opening required 'C:\Users\nick\Sites\bit-by-bit.org\public_html\vendor\composer/../h5p/h5p-editor/h5peditor.class.php' (include_path='C:\Users\nick\Sites\bit-by-bit.org\public_html\vendor/pear/archive_tar;C:\Users\nick\Sites\bit-by-bit.org\public_html\vendor/pear/console_getopt;C:\Users\nick\Sites\bit-by-bit.org\public_html\vendor/pear/pear-core-minimal/src;C:\Users\nick\Sites\bit-by-bit.org\public_html\vendor/pear/pear_exception;.;C:\php\pear') in C:\Users\nick\Sites\bit-by-bit.org\public_html\vendor\composer\autoload_real.php:45
Stack trace:

Which I WAS ABLE TO FIX with composer dump-autoload.

But I then hit the following...

Drupal\Component\Plugin\Exception\PluginNotFoundException: Unable to determine class for field type 'h5p' found in the 'field.storage.paragraph.field_h5p_interaction' configuration in Drupal\field\FieldStorageConfigStorage->mapFromStorageRecords() (line 167 of core\modules\field\src\FieldStorageConfigStorage.php)."

Which I soon realised was because I had been left with NO h5P module at all because of that first failure.

I've reverted everything back now and I managed to get everything back in action locally but it's all a bit hit and miss :-(

The drupal/h5p_math_input is crucial one for me as we use MathJax to render LaTeX in places on the site and several times within H5P objects.

What's a bit infuriating is that the H5P Math input module (https://www.drupal.org/project/h5p_math_input) claims (probably quite rightly!) to be D10 compliant but it ain't no use if I can't get it all wired together.

I should add that I'm on Drupal 9.5.11 and that I'm simply trying to get the site the pass the Upgrade Status tests at this point.

Might the use of mglaman/composer-drupal-lenient be another approach to get me there?

Thanks again, I'm really grateful for you attempt to help here but I am still blocked :-(

Nick

@SirClickAlot - So in short if this issue gets merged your life would be easier, I had some composer hell as well like your having.

If you have to fix this today, I think your only option its to head over to drupal/h5p_math_input 1.1.0, create an issue/fork and temporarily change h5p version requirement of that module. https://git.drupalcode.org/project/h5p_math_input/-/blob/1.x/composer.js... I might just remove it entirely actually (or some kind of wild card? no idea if dev-* would work).

Then when h5p_math_input has a fork you do the same process of excluding it from Drupals packagist, and then add your own vcs/git info to use the issues forked branch instead. And do the same require "dev-xxxxx-branch" to tell composer to use that branch for your math module.

I unfortunately have had to do this for quite a few projects that have low maintainer activity.

Regarding lenient I didn't try it but I think it could work with d10 give it a shot and let us know
https://www.ladrupalera.com/en/drupal/development/modules/install-contri...

Committed/pushed to the 2.0.0 branch (yes, branch not tag) which seems to be where the last alpha was tagged from.

If you'd like an alpha4 with Drupal 10 support, please test the dev release!

Actually to enable a dev release, I had to push a 2.0.x branch, so now both branches have the change, but I think things should continue on the 2.0.x branch - Drupal.org doesn't like non .x branch names.

You should be able to get the dev release with composer require 'drupal/h5p:2.0.x-dev@dev'

Ok, thanks again all for helping out here.

Results

@catch, I have installed the dev version (composer require 'drupal/h5p:2.0.x-dev@dev').
The result was a downgrade of the recently-updated laminas/laminas-servicemanager but all seems OK...

Removing drupal/h5p (2.0.0-alpha3)
Downgrading laminas/laminas-servicemanager (3.22.1 => 3.22.0)
Installing drupal/h5p (dev-2.0.x d6dfe84)

H5P seems to pass the Upgrade status and seems to be working fine in both H5P display and H5p editing.
Hooray!

A couple of following up questions.

These might be relevant to help others trying to battle with the beast...

Updating H5P libraries

As I said above I have tested a whole variety of H5P types both using and editing and all seems good.
The only thing I have noticed is that there are two updates: Interactive video and Flashcards and why I try to update those via the UI, I get a timeout...

Interactive video
Failed to download the requested H5P.
cURL error 28: Operation timed out after 30001 milliseconds with 0 bytes received (see https://curl.haxx.se/libcurl/c/libcurl-errors.html)Get help

Flashcards
Failed to download the requested H5P.
cURL error 28: Operation timed out after 30000 milliseconds with 0 bytes received (see https://curl.haxx.se/libcurl/c/libcurl-errors.html)Get help

This may be purely incidental but I thought I would flag it up juts in case it has some relevance to the dev module release.

.git folder present in the dev module folder?

I do not understand why there is a .gitfolder in the root of the installed dev module? ...

Is that just normal behaviour when using a dev module?
I assume that I do not need to keep it locally and certainly don't need to push it up to the production site.

Thanks again @NicholasS / @catch, we are feeling like H5P is something that we can carry on with now as we move to D10 and that's a great relief.

Interactive video
Failed to download the requested H5P.
cURL error 28: Operation timed out after 30001 milliseconds with 0 bytes received (see

This might be incidental but it would be good if others can check, and also to test with the non-patched version. However I wouldn't expect any of the changes here to have affected it. We're running a patch where H5P libraries can no longer be updated via the UI.

The .git folder you should delete, especially if you're codebase in general is in git, otherwise you'll end up with a submodule. I think it depends whether the tar.gz has been packaged for the release or not, but otherwise yes it's harmless.

Thanks for testing!

I also got the development branch and checked it under Drupal 9.5.11. No problems so far, although we don't use that many H5P modules. I will test this with Drupal 10 later when I have the time.

The module works fine with Drupal 10.1.6 but only if the performance "Aggregate JavaScript files" is not on.
So I have the dev version with this patch:
https://www.drupal.org/files/issues/2022-09-14/content-upgrade-3299839.p...

Which is the correct way to make this module work with D10?
Should I use the alpha3 or dev version but which patches?

@Anaconda777 you can use the dev release with no patches applied.

If a couple of people confirm they're running the dev release on Drupal 10 successfully, I'll tag a new alpha.

@catch I am currently running dev release (2.0.x-dev@dev) in production, and for my setup it does not display the h5p content when the site "aggregate JS" is selected in the performance page. This is with Drupal 10.1.6

@Anaconda777 in that case please open new bug report and link it from here.

@catch I just tested the dev version under Drupal 10.1-dev and PHP 8.1. I didn't find any errors, once I installed these two patches:
* #32 from https://www.drupal.org/project/h5p/issues/3260094
* #6 from https://www.drupal.org/project/h5p/issues/3372528

@ammaletu how are you able to apply those patches, when I put them both to composer, either of them applies, here is my composer result after composer install:

Gathering patches for dependencies. This might take a minute.
- Installing drupal/h5p (dev-2.0.x 43daa83): Cloning 43daa83a9d from cache
- Applying patches for drupal/h5p
https://www.drupal.org/files/issues/2023-04-28/libary_patch_0.patch (d10 library)
Could not apply patch! Skipping. The error was: Cannot apply patch https://www.drupal.org/files/issues/2023-04-28/libary_patch_0.patch
https://www.drupal.org/files/issues/2023-08-08/fixed-asset-optimization-... (h5p)
Could not apply patch! Skipping. The error was: Cannot apply patch https://www.drupal.org/files/issues/2023-08-08/fixed-asset-optimization-...

@Anaconda777 Patch 3260094-#32 applies for me, at least the version of it that we have committed in our repository since April 2023. Some bits of it look different, but in the end it's a 5-character-change. :-) Patch 3372528-#6 did not apply, but it was easy to look through the files and see which lines were not changed. Somebody rerolled this an hour ago. Please test the patch from 3372528-#13 and leave a comment there if it works for you.