🇺🇸United States @illeace

Regarding 1 above -- if you turn off the "Use H5P Hub" setting in Admin > Configuration > H5P and use the H5P Editor field widget, the UI when creating an H5P changes to show a dropdown list of the installed H5P interactive content types. It seems likely that the code that renders that field could be repurposed to restrict the list or even hard-code it and set it as hidden.

I added a related (existing) issue about the scenario in comment #2.

Try enabling the "Session" detection option for Admin > Configuration > Region and Language > Languages > Detection and Selection.

For more info see this comment:
https://www.drupal.org/project/h5p/issues/3548236#comment-16277364 🐛 H5P Editor fails to use chosen language Active

OK, the fix for H5P editor global translation is ready for review.

A bit of extra info:

• The H5P Editor has two translatable parts: global editor fields like the H5P Title, and library-specific fields like the "Correct Answer" field in the True/False interactive.
• These two translatable sections work somewhat independently.
• Drupal's language configuration provides multiple ways of detecting a user's language. The default is to use the path. For example, the value /fr/ is prepended to site URLs if you set your language to French.
• The global H5P Editor language detection works with this default detection method.
• The library-specific detection does not. It seems to require the "Session" detection method be enabled.

For the purposes of this issue, I'm only trying to fix the H5P Editor global language issue, which was introduced recently. I will write up a different issue to consider making the H5P library translation detection more robust, but that is a long-standing issue AFAICT.

illeace → created an issue.

illeace → created an issue.

Minor updates:

• We've got a clear repro case for 🐛 Can't save host entity after adding more while using H5P Editor when missing required fields Active
• Adding 🐛 H5P content lost when collapsing/opening in Paragraphs Active as another related issue

Ah yes -- I had forgotten that I did that. Thanks for clarifying!

Adding one last (fixed) issue to beta1 🐛 The 'system.css_js_query_string' state is deprecated in drupal:10.2.0 Active

Moving [ #3216239] to beta2 since its status in unclear.

I have also encountered this issue, but have not been able to generate a consistent repro case. This issue is related to 🐛 Main title validation not working on AJAX detach leading to data loss Needs work and may have been solved by that fix. For now I'm marking this postponed until we have a repro case or some additional evidence that it's fixes.

@socialnicheguru - I'm not sure I see the connection between these issues. Am I missing something?

Thanks @jozek.wawa. This looks like a duplicate of 🐛 Error when uploading an .h5p file through upload form widget Active that was fixed recently, merged into 2.0.x-dev and will be in the next release. Can you take a look at that issue and mark this "Closed (duplicate)" if it is?

This looks good, marking RTBC. Things that have been tested in both Drupal 10 and Drupal 11:

Check asset mirroring works:

• Make sure the vendor assets haven't been mirrored to /sites/default/files/h5p, clear cache, then view a node with an H5P field.
• Same as above, but visit the H5P editor for a node with an H5P field.
• Same as above, but view an H5P embed route

Check paths have been updated:

• For each of: H5P node view, H5P edit view, and H5P embed views
• Open dev console network tab
• Ensure the only CSS/JS files loaded with "vendor" in them are part of Drupal's core vendor folder
• Ensure H5P CSS/JS files load with the "sites/default/files/h5p/h5p-core/" and "sites/default/files/h5p/h5p-editor/" paths
• Ensure no H5P CSS/JS files return a 404 status (with the exception of the file mentioned in 🐛 Content Hub Metadata Cache Route Not Defined Active

Aggregation

• Turn Drupal CSS/JS aggregation on and check that H5Ps load
• Turn Drupal CSS/JS aggregation off and check thatn H5Ps load

This is ready for review now.

For reference, here's a similar issue from Webform 📌 The 'system.css_js_query_string' state is deprecated in drupal:10.2.0. Fixed
... and a link to the MR diff

illeace → created an issue.

Adding 🐛 Content Hub Metadata Cache Route Not Defined Active to the beta2 list.

OK, I think this is working. It needs review - both to confirm the H5P Hub shared content feature works and also to double-check that code I ported has been updated to D10+ standards.

This is an old and answered support question that can be closed now.

illeace → created an issue.

illeace → made their first commit to this issue’s fork.

+1 to this suggestion. One other consequence of the current implementation is that things like Google Analytics / Tag Manager don't get included on the embed page, so embedded traffic isn't recorded.

Unsaved H5P editor field data can be lost if not valid during AJAX rebuild

Steps to reproduce

• Ensure you have a content type with an H5P editor field that allows multiple values.
• Add a new multi-h5p node
• Put text in the Drupal node's title field
• Choose the "Find the Words" H5P type
• Add text to the H5P editor's Title field
• Add an item to the "Word list" field that includes a period (or any other non-alpha character)
• Click the "Add another item" button to create a second H5P field instance
• After the rebuild, the "Word list" field has reverted to the default.

This happens because the H5P editor getParams() function will not return the value of a field it fails validation. That means the hidden "h5p-content-parameters" field doesn't get update with the invalid value. Clicking the "Add another item" causes an AJAX form rebuild, which uses the the content of that "h5p-content-parameters" field to regenerate the H5P editor state. Anything not in the field can't be preserved.

Clicking Save twice bypasses H5P validation

Steps to reproduce:

• Never mind
• While I swear I was seeing this when testing 🐛 Main title validation not working on AJAX detach leading to data loss Needs work , now I can't reproduce it. Leaving this here in case somebody else discovers the repro steps.

Issue: Chrome browser ignores H5P validation in some cases

Steps to reproduce

• Using Chrome browser
• Ensure you have a content type with an H5P editor field that allows multiple values.
• Add a new multi-h5p node
• Put text in the Drupa node's title field
• Choose "Multiple Choice" for first H5P
• Leave the H5P's required Title and Question fields blank
• Add text for the Option 1 and Option 2 fields
• Click “Add another item” to create an additional H5P field
• Repeat the steps above for the second H5P item
• Click the Save button
• The node saves without triggering the Title validation.
• Note: This doesn’t seem to happen with the other H5P interactive types I've tried or other browsers.

illeace → created an issue.

With this fix, we've somewhat redefined the issue being fixed, which is that any unsaved H5P edits are lost when adding or removing an H5P field in the multi-H5P field scenario. This patch largely fixes that and is a meaningful enough improvement that I'm marking it RTBC, despite the fact that there is still a fair amount of jank in the validation of H5P items, especially in a multi-field environment. Some examples of remaining issues:

• 🐛 Can't save host entity after adding more while using H5P Editor when missing required fields Active
• In single-value H5P fields, clicking Save twice bypasses H5P validation.
• In multi-value fields during AJAX rebuild (adding/removing an H5P field) unsaved changes are lost if those changes aren't valid (e.g., try adding a period to the "Word list" field in a "Find the words" H5P, then click the "Add another item" button).
• On Chrome browser, you are able to save empty, required H5P fields when you have two "Multiple Choice" items.

I think the interaction between Drupal form validation and H5P item/field validation needs a bit of a refactor. I will create a new issue along those lines and include this detail in that issue.

I agree with @catch that the logic for div embedding in much simpler. It would be great if we could rely on that alone, but very few of the H5P interactive libraries claim to support it. I checked the items in the cart above that are marked "div" and most of these are iframe-only. In my tests, the ones marked "div" above that actually support div are:

• collage
• multi-hotspot
• find the hotspots
• mark the words
• memory game
• Twitter feed (...but it's discontinued because of Twitter API changes)

A prerequisite for this issue is updating the individual H5P interactive libraries to support div embedding. Offhand, I have no idea how easy or hard that would be, but the list is pretty long.

This is ready for review.

OK, I've got a fix for this and will push to the issue branch shortly. There were a few things going on:

• Drupal core and H5P core libraries were getting added to the aggregation that didn't need to be there.
• Excluding the `h5p.content` library solved this, but errors were still appearing in the console.
• Turning off aggregation resolved those errors.
• I don't fully understand this, but think it's a good idea anyway because this seems to be what the h5p.js init code for iframes is expecting./li>
• ...further, I've seen cases where specific H5P libraries inspect this array (see: https://github.com/otacke/h5p-game-map/blob/f8631762f36d794823ee296d92b5...).

I'll take a look at this issue. I'm not sure yet what the solution is, but I'm familiar with the problem from working on 🐛 D11 Console Errors Active

This looks good. I've tested:
- before and after the fix
- on a D10 and D11 site
Everything works as expected and the fix seems reasonable. Marking RTBC.

I've review the patch from @pjotr.savitski and confirmed:

• Prior to the patch, when viewing a node with an H5P or editing an H5P, the first CSS/JS alter file was included, but not the 2nd or 3rd.
• After the patch, additional alter files beyond the first are correctly included.
• Note: this change does not affect the embed method of viewing an H5P, which continues to work correctly.

I've created a new feature branch / MR in the Issue fork, which is ready to merge.

The help simplify testing a bit, here's the test I've been using:

1. Make sure Drupal's caching is enabled
2. Create a Question Set H5P, or any other interactive that has a "Finish" button to submit results.
3. Clear the Drupal cache.
4. Make sure you are a logged in user and load a page that displays that H5P (it could be the normal node page or the embed URL). Start a timer as soon as the page loads.
5. Open your browser's network inspector, set it to filter for "set-finished.json" to exclude any network activity aside from that callback URL, which is where your H5P "finished" data gets posted to.
6. Complete the H5P and click the Finish button. You should see an item appear in the network inspector.
7. Click that network item and ensure the response is JSON data with contents of {"success":true}
8. Now the boring par ... wait 12 hours and reload the page.
9. Repeat the steps above to ensure the "set-finished.json" response is still {"success":true}

OK, as @pjotr.savitski suggests, I've created an issue fork that sets the max age to align with the token expiration time. This ensures that the Drupal cache never serves an H5P with an already-expired token. This is ready to be tested, but a bit time-consuming since it involves waiting for 12 hours for the cache to expire.

I've reviewed the code changes and tested before and after behavior on a D11 site. Everything looks good from my perspective. As discussed above, anyone who has been using alpha builds and has a role with this permission will lose the permission after updating and will need to re-add the renamed permission. We need to be sure the release notes for the next release include documentation about the change.

I can confirm the original issue is fixed. The warnings mentioned in comment #5 still occurr, but are not specific to the course presentation, and seem to happen whenever a new library is installed. Marking this Closed (outdated). A new issue should be added for the Undefined Property warning.

I tested the Content > H5P Library > Upload form as well as the H5P upload field widget on both D10 and D11 using both the 2.0.0-alpha6 release and the issue branch. In D10, both work fine. In D11, I confirmed the bug exists in 2.0.0-alpha6 and is fixed in the issue branch. Marking this RTBC.

I checked out this branch and the changes worked for me as well. Setting to RTBC.

@drupalfan2 Two things:
- I've tried but cannot reproduce this issue in any of the version mentioned. Does the interactive need to be configured in a particular way.
- There have been a number of fixes in the 2.0.x-dev branch. Does the problem still happen to you in that branch? If so, I could use more specific steps to generate the issue.

Thanks!

Adding a few more related issues.

Removing 💬 Upload button missing? Can't add h5p files on new D8 install Active – this is a feature request, but the behavior as-is works.

Now that #3309418: H5P Upload widget PHP notices → has a fix, I can test this. It works OK even without the button, but you have to save the content (submit the entire form) to upload the H5P. I also agree with @paalj -- the H5P Editor widget also an "Upload" option and has generally a better experience. I'm changing this to "Feature request" but bumping down the priority.

@id.aleks I've rebased the issue branch off of 2.0.x from origin and it shows as mergeable now. I've also checked out the issue branch, rolled back the code before you commits and reproduced the issue. Then I pulled in your commits and retested, confirming the issue no longer occurs. I'm not sure if it's kosher, but I'm going to go ahead and mark this RTBC, but I could get somebody else to test if desired.

You bet. I'll work on that now.

Actually, there is one more console error that I can see was caused by the same refactor. I'm moving this back to active to try and resolve that as well.

This issue fork / merge request is ready to be reviewed now

Pushed a fix to the issue fork branch with the changes mentioned in comment #9

Assigning to myself and updating status to active

Further digging reveals that the unexpected contents in $h5p_integration['contents'][$content_id_string]['scripts'] is what's breaking the Game Map interactive. Adding a bit of extra checking and path formatting to this code:

        foreach ($all_js as $asset) {
          $jsFilePaths[] = $asset['data'];
        }

For example to this:

        $file_url_generator = \Drupal::service('file_url_generator');
        foreach ($all_js as $asset) {
          if (is_string($asset['data'])) {
            $jsFilePaths[] = $file_url_generator->generateAbsoluteString($asset['data']);
          }
        }

The change above removes all but one of the D11 console errors I see and allows the Game Map interactive to load properly. I'll work on a patch.

A little more information about the jquery pathing error:

• In src/Plugin/Field/FieldFormatter/H5PDefaultFormatter.php line 113, the gets an array of JS files via
  $scripts = $this->assetResolver->getJsAssets($assets, TRUE);
• Farther down (line 133), the 'data' value gets added to the $jsFilePaths array.
• For the jquery library, this path is relative -- modules/contrib/h5p/vendor/h5p/h5p-core/js/jquery.js
• Other path values are absolute, starting with /sites/default/files/js/...
• This info gets added to the $h5p_integration variable, in line 165
  $h5p_integration['contents'][$content_id_string]['scripts'] = $jsFilePaths;
• All of this is passed into drupalSettings and eventually used the vendor/h5p/h5p-core/js/h5p.js in the H5P.getHeadTags function to generate this markup:
  <script src="modules/contrib/h5p/vendor/h5p/h5p-core/js/jquery.js"></script>
• Because the path is still relative, the browser appends that path to the current path (e.g., /node/24/), which results in the jquery loading errors people are noticing.

One way to eliminate the error would be to make sure any script paths added to the $h5p_integration variable are converted to absolute. However, I'm not certain this is the correct solution. A few questions I still have:

1. This jquery library is already being loaded by Drupal, so this script tag seems redundant (at least for me in this case).
2. One of the array elements returned by $this->assetResolver->getJsAssets in line 113 is a "drupalSettings" array. This array gets passed all the way to h5p.js and the H5P.getHeadTags function which results in the markup <script src="[object Object]"></script> that generates another of the loading errors. Presumably that item should never get passed to the JS

I can confirm that I see this error with the Game Map interactive (1.5.4) using 2.0.0-alpha6 and D11. The jquery error is a common one across different interactives, but so far I have not seen the Uncaught TypeError: e.includes is not a function error with any other type except for Game Map, which leads me to think that part of the issue may be the JS in that library. I'll investigate more.

This patch still applies as of the release of 2.0.0-alpha6, and seems to fix the problem indicated. I did a bit of testing using the H5P file upload widget within a paragraph and also in a standard node, using single and mulit-value fields. Everything I tried worked. Marking RTBC.

Are either of you able to test if the issue still exists in 2.0.0-alpha6? If so, do you have a simple case that reproduces the error? I've tried to recreate the problem and haven't been able to so far.

Patch #5 no longer applies to 2.0.x-dev so created a new patch file off the latest dev code. @id.aleks - please take a look and make sure I didn't miss anything.

I have used similar steps to the ones you suggest for testing, and in my tests can confirm that the patch resolves the issue of showing a different user with the same role's cached progress. I also confirmed that the additional user context is only added when the Save content state option option is enabled.

What I found using Drupal 11 and 2.0.0-alpha6 is that my H5P CSS customizations worked when viewing the H5P embed link, and also were visible when editing the H5P, but not when viewing the node directly. After applying this patch, I also see those CSS customizations when viewing the node. I'm marking this RTBC.

Removing ✨ Prevent embedded content from anonymous users Active from Beta list. This is a bit trickier than it seemed at first, and might involve more refactoring at this stage than is wise.

Using Drupal 11 and 2.0.0-alpha6, I am able to follow these steps and the upgrade-confirm page successfully loads (no error messages on the screen or in the developer console. Does this still happen for you?

I think this is a nice idea, but here are a few things to consider:

• Since the primary purpose of the embed feature is to use these H5P with external sites (like LMSs), we would want to preserve the default behavior of having these publicly visible (so we don't break existing installs)
• Adding the option to apply Drupal permissions to those embed paths sounds relatively straightforward
• ...but, all the individual *.h5p files that live in /sites/default/files/h5p/exports/ (i.e., in the public files directory) are going to be downloadable regardless, and they have very guessable names.

So, if the goal is to really protect the content of the H5Ps from anonymous users, then this becomes a bigger challenge.

Could you share more about how you are able to export an H5P with an empty title. I tried to recreate this using the Question Set H5P type, but was not able to. The title was successfully exported and imported for me.

I did some light testing using Drupal 11 and the 2.0.0-alpha6 release and I couldn't reproduce the issue. From the error, I expect the problem to have occurred in src/H5PDrupal/H5PEvent.php in the saveStats() function. If this is still happening to you, I'd love to hear more. Maybe there is an extra step or particular interactive type that is required to trigger this?

This sounds like a useful feature. I haven't tested applying the patch, but glancing at it suggests it will need to be updated a bit to work with the latest 2.0.x-dev branch.

For clarity, the difference in embed paths is expected. Drupal's pattern is "[domain-name]/h5p/[entity-id]/embed". I'm using Drupal 11 and release 2.0.0-alpha6, and I have not seen this issue. If the original poster is still active and experiencing this, please let us know. Otherwise, given the age of the issue I suggest we mark it as "Closed (outdated)".

I'm using Drupal 11 and 2.0.0-alpha6, and can't reproduce this problem. Given it's age, I suggest we mark it "Closed (outdated)".

In 2.0.0-alpha6, this link has been added to h5p.links.task.yml. This can be marked "Closed (outdated)".

This seems like an issue with the core H5P Editor PHP Library (rather than the Drupal-specific module). Perhaps it's related to issue #260 in that library's issue queue?

I don't fully understand the request. I understand there is a desire to see the H5P as if you are another user. For what purpose is this exactly? To see how they've answered certain questions or for some other reason?

I do not see this behavior using Drupal 11 and the 2.0.0-alpha6 release. I suggest we mark "Closed (outdated)".

Using Drupal 11 and the 2.0.0-alpha6 module, I believe this is fixed. I've created a view displaying H5Ps and also used the embed code to embed an H5P in the body of other content. In both cases, when I resize the browser window to make it smaller and then larger, I see the iframe shrink and expand as expected. Unless I am misinterpreting this, I suggest we mark this "Closed (outdated)".

Thank you for the suggestion. That sounds like a feature that would need to be added to that particular H5P interactive type, and not the Drupal H5P library. If this is something you are still interested in, I would suggest adding your feature request to the Issue list for that H5P Question Set project here: https://github.com/h5p/h5p-question-set/issues

One suggestion: Use the Advanced HTML/Text element and then paste the H5P embed code into that field. If that doesn't work, please share more details about what you are trying to accomplish.

I'd love to review this issue, but I'm having a hard time reproducing the scenario. I'm working on fresh local install of D11 and 2.0.0-alpha6. I thought I could upload an .h5p file using an old library from another site I have access to. However, when I do that, Drupal seems to automatically update the H5P interactive to the latest library version. That's cool, but doesn't help me reproduce this issue and test this patch. Any suggestions (other than creating H5Ps and waiting for an actualy library update)?

As of 2.0.0-alph6 in Drupal 11, this still happens. It's a bad enough issue that I wouldn't want to use this configuration (H5P and multi-value field) on a production site.

I can confirm that in Drupal 11 and release 2.0.0-alph6, this issues still happens and the patch applies and appears to fix it for me. It would be great to get this into the next release.

Dropping in to say this patch still seems to apply cleanly to the 2.0.0-alpha6 release.

The patch makes sense, though the line numbers makes me think it needs to be rebuilt for 2.0.0-alpha6. I'd be happy to do that and test it, but I am not able to see this particular error. Are there any additional steps necessary? Is it only relevant for PHP 7.4? I'm using Drupal 11 and PHP 8.3.

Two things:

• This patch modifies a file in the H5P Editor PHP library (not the H5P Drupal module). That project has it's own issue queue on Github: https://github.com/h5p/h5p-editor-php-library.
• When using 2.0.0-alpha6 release in Drupal 11, there is some available translation of these fields. In a quick test, I switched my site's language to French and the summary and description values when editing an H5P were translated to French.

Given that, I think it makes sense to close this issue.