Skip to content

fix: patch echo360.js on the fly to avoid error in strict mode#4377

Open
mindcrunch4u wants to merge 8 commits intoLumieducation:masterfrom
mindcrunch4u:master
Open

fix: patch echo360.js on the fly to avoid error in strict mode#4377
mindcrunch4u wants to merge 8 commits intoLumieducation:masterfrom
mindcrunch4u:master

Conversation

@mindcrunch4u
Copy link

Root Cause:

  • echo360.js uses bad syntax, which causes uglify-js to throw error in strict mode.

Changes:

  • Add a piece of logic to patch echo360.js during export.

cc Original Author: @sr258


Regarding this issue:

Another solution would be to contact the maintainer of H5P.Video in hope they fix their script to comply in strict mode. I think that would be the most elegant solution

I've created a PR here: link, let's hope otacke fixes this issue soon.

Meanwhile I'd like this propose a more flexible fix (this PR):

  • Since it's impractical to make h5p authors to write perfect code, we could instead fix faulty code on-the-fly before passing them through uglify-js.
  • With this kind of design, we can respond to our user as soon as possible by delivering patches (instead of waiting for h5p).
  • Of course, we could file an issue to h5p's library at the same time, and if the issue gets fixed, then we could remove our patches in subsequent releases.

@mindcrunch4u
Copy link
Author

Adding @JPSchellenberg for review since this also affects Lumi users.

@otacke
Copy link
Contributor

otacke commented Feb 15, 2026

@mindcrunch4u Just noticed my name here by chance: I agree that this should be fixed in H5P.Video, but I am not the maintainer of that repository and cannot push anything. You will need to reach out to H5P Group.

@sr258
Copy link
Member

sr258 commented Feb 18, 2026

@mindcrunch4u Thank you for your PR. I've picked this up and generalized the patch mechanism. Unfortunately, I can't reproduce the issue to check whether the issue is actually solved by the PR. Can you give me a reproducible instruction how to cause the bug?

@mindcrunch4u
Copy link
Author

mindcrunch4u commented Feb 24, 2026

@sr258 to reproduce, I simply added a video to a basic H5P project, then export as SCORM. See this issue.

Your LibraryPatcher looks good! If it works, then Lumi should function like this: see video in this comment.

To verify using Lumi, this is my write-up: link.

Basically:

  1. Upload the patched version of H5P-Nodejs-library to npm
  2. Get Lumi source code
    nvm install 24
    git clone https://github.com/Lumieducation/Lumi.git
    
  3. Then edit package.json, change @lumieducation/h5p-html-exporter to your new npm repo (containing the updated H5P-Nodejs-library)
  4. npm install and npm run build then npm start.

@mindcrunch4u
Copy link
Author

@sr258 I've done the verification. My steps:

  • upload your (@sr258 ) version of h5p-html-exporter to npm: link
  • update Lumi's package.json to point to the updated h5p-html-exporter

Here's the comparison.

Without this patch (containing your commits), Lumi hangs when exporting the project:

before-video.mp4

With the patch:

after-video.mp4

Note: because of changes to HtmlExporter (I saw you added options?: IHtmlExporterOptions), I did a bit of hack here to make the code run (removed options when calling HtmlExporter from Lumi).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants