Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update H5P to latest #12993

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from

Conversation

rtibbles
Copy link
Member

@rtibbles rtibbles commented Jan 9, 2025

Summary

  • Tweaks our H5P vendoring script to be less restrictive in terms of the files it unzips
  • Updates H5P to the latest from upstream
  • Fixes CSS url handling for edge cases
  • Adds range request support to the zipcontent endpoint, so that we can serve audio and video files from inside archive files
  • Allows the zipcontent endpoint once again to serve all recognized archive formats, so we can defer to the zipcontent endpoint for H5P file fetches that we can't intercept to read from our blob URLs.

References

Diff h5p/h5p-php-library@eeefc12...fb5100a

Reviewer guidance

This should be checked against our comprehensive H5P testing channel - but can start with those we have in the QA channel.

@pcenov
Copy link
Member

pcenov commented Jan 10, 2025

Hi @rtibbles - unfortunately none of the H5P resources from the QA channel can be started. Getting the following error in the console:

627-aa1ed38517f4010eb1dc.bundle.js:1 Uncaught URIError: URI malformed
    at decodeURIComponent (<anonymous>)
    at 627-aa1ed38517f4010eb1dc.bundle.js:1:4162
    at Function.from (<anonymous>)
    at S (627-aa1ed38517f4010eb1dc.bundle.js:1:3679)
    at e.value (627-aa1ed38517f4010eb1dc.bundle.js:1:4591)
    at t.value (627-aa1ed38517f4010eb1dc.bundle.js:1:11771)
    at 627-aa1ed38517f4010eb1dc.bundle.js:1:13462
    at Array.map (<anonymous>)
    at 627-aa1ed38517f4010eb1dc.bundle.js:1:12492
    at 451-381dff277bf2e6336c0e.bundle.js:1:7339
2025-01-10_14-44-25.mp4

Logs: logs.zip

@radinamatic
Copy link
Member

Same here (Chrome and Firefox on Ubuntu 20.04)

2025-01-10_15-59-45

@rtibbles
Copy link
Member Author

Bah, thanks, will take a look.

@rtibbles
Copy link
Member Author

Hi @pcenov @radinamatic - I replicated the issue, slightly surprising that it hadn't occurred before this update, but it may be to do with recent updates in develop. Either way, I have fixed it here, as far as I can tell!

@pcenov
Copy link
Member

pcenov commented Jan 13, 2025

Hi @rtibbles, I confirm that the H5P resources are functioning correctly now.

The following 3 resources from our "H5P testing" channel (piman-fapum) are not loading in both the latest build from this PR and the previous version of H5P, and I'm just mentioning that for the record as it's not caused by the changes made here:

iframe

timeline

video

@rtibbles
Copy link
Member Author

Thanks @pcenov - I think I was aware of the 'interactive video' issue - as I suspect that is using an embedded youtube video that we disallow. Worth my taking a quick look at each of them just to confirm whether we can fix them or not though!

@github-actions github-actions bot added the DEV: backend Python, databases, networking, filesystem... label Jan 13, 2025
@rtibbles
Copy link
Member Author

OK, the iframe embedder should now be fixed. The timeline is working, but with broken images, as it is referencing images hosted on wikipedia, which are blocked/should be assumed missing as they are not locally available, and the interactive video is still broken, as it is referencing an externally hosted video file too.

@rtibbles
Copy link
Member Author

@pcenov this does some small changes to how we serve HTML5 app content as well, so a regression test of HTML5 apps would be helpful here as well (although the fact that it is working for H5P gives me confidence nothing is broken for HTML5 apps).

@pcenov pcenov self-requested a review January 14, 2025 11:54
Copy link
Member

@pcenov pcenov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @rtibbles, I confirm that the iframe embedder and the timeline are working now (broken images aside)!
No regressions observed on the HTML5 apps as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
DEV: backend Python, databases, networking, filesystem... DEV: frontend
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants