PHP 8.0 deprecated syntax warnings for h5p-editor

Looks to me like this PR has been merged but I see no more recent alpha release and do official dev version release.

Am I mistaken?

Thanks all.

Hi everyone,

Drupal 9.4 just got released yesterday with a minimum php compatibility of 7.4. That's the highest php version the h5p editor module can handle. As mentioned in #2, the compatibility issue does appear to have been resolved, but no up-to-date release has been issued for it yet.

This isn't a major problem as it seems sites can still function with php7.4 for a while. But, at least in my case, it would appear that php 8 compatibility for h5p editor is the only thing holding up an upgrade of my sites.

Would be grateful if a new release could be made soon for an otherwise awesome module.

Thanks!

Agreed 100%.

The H5P module is a crucial player in educational sites and come December it would be good to a void a scrabble to a compatibility fix.

Like Tam says, not crucial right now but why not deal with this plenty ahead of time?
Maintainers of this great module, a call to action ;-)

#Thanks

I can verify this issue when I upgraded my local environment to PHP 8. Tangibly speaking, how does this impact the H5P editor on an environment with PHP 8? I tested the editor, and it didn't seem to break after a cursory observation.

Acquia is ending support for PHP 7.4 October 3rd, so I'll throw my support in for resolving this ahead of schedule to avoid a scramble :-).

would we not also want to re-arrange the params, since required now needs to be first with optional after:

Required parameter $defaultLanguage follows optional parameter

$prefix and $fileDir are optional params

public function getLibraryData($machineName, $majorVersion, $minorVersion, $languageCode, $defaultLanguage, $prefix = '', $fileDir = '')

I found that Update_H5P-editor_PHP8_compatible_3260094_2.patch works for the initial error. But then I was getting an error with the vendor directory. In order to resolve the vendor error, I applied h5p-h5p-editor-h5peditor-class-php.patch

my composer.json file looks like this:

"h5p/h5p-editor": [
    "patches/h5p-h5p-editor-h5peditor-class-php.patch"
],
"drupal/h5p": {
    "php 8 compatiblity": "patches/Update_H5P-editor_PHP8_compatible_3260094_2.patch"
},
...
"require-dev": {
    "cweagans/composer-patches": "^1.7",
    "symplify/vendor-patches": "^11.0"
},

get an error with the patch from #8
which i tried to apply like this :

"drupal/h5p": {
                   "php 8 compatiblity": "https://www.drupal.org/files/issues/2022-06-27/h5p-h5p-editor-h5peditor-class-php_0.patch"
            },

this is the error :

https://www.drupal.org/files/issues/2022-06-27/h5p-h5p-editor-h5peditor-class-php_0.patch (php 8 compatiblity)
   Could not apply patch! Skipping. The error was: Cannot apply patch https://www.drupal.org/files/issues/2022-06-27/h5p-h5p-editor-h5peditor-class-php_0.patch

patch number 7 alone works for me with php 8:

 "drupal/h5p": {
             
                "php8 compatible": "https://www.drupal.org/files/issues/2022-06-27/Update_H5P-editor_PHP8_compatible_3260094_2.patch"
             },

Patch at #8 won't apply. Patch at #7 does not address deprecation error in PHP 8.0.19

For me, Patch at comment #7 fixes the issue for deprecation error message.
Drupal 9.5.x
PHP 8.0
Steps performed:
1- Using composer add module to project.
2- Go to any page in site, the error says: deprecated: Required parameter $defaultLanguage follows optional parameter $prefix in /Users/narendra.rajwar/Documents/sites/contribution/drupal/vendor/h5p/h5p-editor/h5peditor.class.php on line 376
3- Now enable the module using drush, the error says:

PHP Deprecated:  Optional parameter $prefix declared before required parameter $defaultLanguage is implicitly treated as a required parameter in /Users/narendra.rajwar/Documents/sites/contribution/drupal/vendor/h5p/h5p-editor/h5peditor.class.php on line 376

Deprecated: Optional parameter $prefix declared before required parameter $defaultLanguage is implicitly treated as a required parameter in /Users/narendra.rajwar/Documents/sites/contribution/drupal/vendor/h5p/h5p-editor/h5peditor.class.php on line 376
PHP Deprecated:  Optional parameter $fileDir declared before required parameter $defaultLanguage is implicitly treated as a required parameter in /Users/narendra.rajwar/Documents/sites/contribution/drupal/vendor/h5p/h5p-editor/h5peditor.class.php on line 376

Deprecated: Optional parameter $fileDir declared before required parameter $defaultLanguage is implicitly treated as a required parameter in /Users/narendra.rajwar/Documents/sites/contribution/drupal/vendor/h5p/h5p-editor/h5peditor.class.php on line 376

This one worked for me on the 2.0.0 branch.

Here is the commit from the official repository that fixes said issue: https://github.com/h5p/h5p-editor-php-library/commit/2edfd9b9e72d20e3b61...

Below is an example taken from the composer.json file of a project (NB! Changed the previous faulty example):

    "patches": {
            "h5p/h5p-editor": {
                "Fix whitelist check to be case insensitive": "https://github.com/h5p/h5p-editor-php-library/commit/bd871a3181f3e4a343351ae6a22d773a74d928b7.diff",
                "Fix PHP 8 deprecation issue": "https://github.com/h5p/h5p-editor-php-library/commit/2edfd9b9e72d20e3b61f17a6836da21edb370624.diff"
            }
        }

The dependency itself already has a newer version, but current module is locked to the previous one. See this for more details.

NB! Patch #13 changes the signature of the method and can not really work. Yes, the deprecation warning will probably disappear, but the caller will still be passing the arguments in the previous order.

I cannot figure out how to apply the patch above to the editor, because it seems to be added by the composer.json file in the module. Have tried some different things, some with errors and some seemingly apply it, but in the end it won't apply it, I think because it is loaded from the module's composer.json?

Would be nice to have a fix to these deprecation notices without having to struggle with patches to sub-components!

@bob.hinrichs It seems that I've removed too much from the copied part of the composer.json file. Will change the original comment to fix that. Below is the correctly working patches example.

    "patches": {
            "h5p/h5p-editor": {
                "Fix whitelist check to be case insensitive": "https://github.com/h5p/h5p-editor-php-library/commit/bd871a3181f3e4a343351ae6a22d773a74d928b7.diff",
                "Fix PHP 8 deprecation issue": "https://github.com/h5p/h5p-editor-php-library/commit/2edfd9b9e72d20e3b61f17a6836da21edb370624.diff"
            }
        }

Thank you Pjotr!

But this is the problem: h5p/h5p-editor is part of the drupal h5p module's composer packages and it installs into the module's own vendor directory. If I put "h5p/h5p-editor" into our composer file, it does not install h5p editor there. Is what we need a patch to patch the patches in the module? Or is there a need for a special installer-path (a pain)?

BTW
I am coming across another php 8 deprecation notice as well:
"Deprecated function: json_decode(): Passing null to parameter #1 ($json) of type string is deprecated in Drupal\h5peditor\H5PEditor\H5PEditorDrupalStorage->getLibraries() (line 130 of /app/web/modules/contrib/h5p/modules/h5peditor/src/H5PEditor/H5PEditorDrupalStorage.php)."

@bob.hinrichs It is a bit strange, but I do see that h5p-editor is installed into the root, so to say, vendor directory. That one is surely patched.

I'm not the best of the best composer specialist, but going through the H5P module code does give a hint on how it all really works. Please note that I'm using the 2.0.0 branch as that is what I have installed locally.

I read those lines as follows:

• Check if H5PCore class exists.
• Require the local version of autoload file if it does not.

You can clearly see that the local vendor catalog is part of the code repository and will be used if the modules are not managed using composer. It is possible to check why are the packaged dependencies being used and maybe even patch the H5P module to comment out or remove those lines. That is surely a bad decision as you should determine why are those sub-modules not installed. It does not matter that those are nit the top level dependencies as composer is not really installing anything into the h5p module and that one is just packaged with the module code itself. The dependencies of dependencies are evaluated by composer and installed as usual. There is --with-all-dependencies key, but that is mostly used when updating something.

As for the deprecation warning you mentioned, well, that seems to be worse than just a deprecation warning. Do see the corresponding line for yourself.
That is the result of type hinting for arguments and it also affects the built-in functions. The value for metadata_settings is equal to NULL for some of the libraries (I would even say most, at least that is what I currently see in my local instance). The line should be rewritten to something like this: $library->metadataSettings = !is_null($details->metadata_settings) ? json_decode($details->metadata_settings) : NULL;

Hope that helps you enough to figure out what is wrong with your setup.

Thank you again! You made me see that I needed to delete the files in the already-installed vendor directory before running the revised install with h5p-editor in the main project vendor directory.

Also thank you for elaborating on that appears to be another fix necessary in this php 8 h5p editor ticket.

I can confirm as well that applying the patches noted in #16 worked great.

Patch in #16 works for me as well! Whether this is the best long-term solution, I wouldn't be able to say.

Facing the same issue in PHP 8.0 and PHP 8.1

Any chance of getting this RTBC and commited soon? PHP 7.4 is EOL in January.

Echoing the question in #23.

Any updates would be much appreciated.

I was having problems with this fix but had to edit my comment, I realized I was trying the patch linked in the issue but the actual patch is in #16.

Just hit this issue trying to install the Opigno Distro. Looks like all has been resolved in Alpha3 less the "$defaultLanguage" issue.

Looks like this was solved in h5p-editor 1.25 by correcting $defaultLanguage as optional as well rather than changing the order:

-  public function getLibraryData($machineName, $majorVersion, $minorVersion, $languageCode, $prefix = '', $fileDir = '', $defaultLanguage) {
+  public function getLibraryData($machineName, $majorVersion, $minorVersion, $languageCode, $prefix = '', $fileDir = '', $defaultLanguage = '') {

On that note, maybe updating to h5p-editor 1.25 is the way to go? Again, I'm completely new to this module, that could be a very bad idea.

Free to test if need be. Let me know what to try. Thanks,

I didn't see correct patch created here, so I created one myself. This patch works for me fine. Feel free to use.

Hi all, I have created a new patch #28 for the latest version of h5p module for the compatibility of Drupal 10 compatibility.

Hello Team,

I have same issues getting on my drupal project.I but i tried this patches but error not solved.

Updated patch for latest version of h5p.

#30 works fine. Thanks

I have a problem applying the patch #30, for the structure of my project the patch can't be applied for that I take a different way creating a patch directly into the library and add the reference into my composer.json.

If you have the same issue do the next steps:

- Add the reference of your library directly in your composer

"require": {
     "h5p/h5p-editor": "1.24.4"
}

- Add my patch into the patch section

"patches": {
          "h5p/h5p-editor": {
                "PHP 8 compatibility": "patch/libary_patch.patch"
            },
}

update:

There is confusion between h5p/vendor directory and the one managed in my install via composer.

I ended up going with what is outlined in #14 and applying the commits to the repo to h5p/h5p-editor library.

it got rid of this error:
Error: Optional parameter $fileDir declared before required parameter $defaultLanguage is
implicitly treated as a required parameter in
drupal9.5.9/vendor/h5p/h5p-editor/h5peditor.class.php,
line 376

drupal 9.5.9
php8.1

This was for another issue. I am sorry for the noise:
Workaround:
not using the patch I added the following to my composer file

"h5p/h5p-core": "1.24.4 as 1.24.2",
"h5p/h5p-editor": "1.25 as 1.24.4",

Could someone please fix this annoying bug ASAP? Thanks!

Patch in #32 worked for me against the new Drupal 10 dev branch, PHP 8.1.

I just tried the current dev version of H5P with Drupal 10.1 and PHP 8.1 and got these deprecation notices. The patch from #32 fixes them.

This needs to be fixed in https://github.com/h5p/h5p-editor-php-library if it's not already.

If the upstream library already has the fix, an MR to update the version would be good.

It's fixed in the upstream version, see line 376 of: https://github.com/h5p/h5p-editor-php-library/blob/master/h5peditor.clas...

h5p/h5p-editor:1.25 supports PHP 8. Applying the patch at #3420268: Support h5p/h5p-core:1.26 and h5p/h5p-editor:1.25 updates the module to use version 1.25 of h5p-editor.