Skip to content

fix: upgrade hls.js to version 1.6.15 and update related imports in video player tests#57

Merged
papphelix merged 4 commits intoedx:release-ulmofrom
papphelix:fix/hls-js-upgrade
Dec 8, 2025
Merged

fix: upgrade hls.js to version 1.6.15 and update related imports in video player tests#57
papphelix merged 4 commits intoedx:release-ulmofrom
papphelix:fix/hls-js-upgrade

Conversation

@papphelix
Copy link
Member

  • This Pull request is for testing if hls player upgrade workes in stage, if works the actual request will be opened to openedx and will be cherry-picked to edx release-ulmo

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR upgrades the hls.js library from version 0.14.17 to 1.6.15 and updates test references from HLS to Hls to match the exported name convention. This is a test PR to validate the upgrade works in staging before creating the actual PR for Open edX.

Key changes:

  • Upgraded hls.js package from 0.14.17 to ^1.6.15
  • Updated test spy calls from HLS.isSupported to Hls.isSupported
  • Added ES6 import statements (though they conflict with existing RequireJS patterns)

Reviewed changes

Copilot reviewed 2 out of 3 changed files in this pull request and generated 3 comments.

File Description
package.json Updated hls.js version from 0.14.17 to ^1.6.15
package-lock.json Updated hls.js dependency tree, removed eventemitter3 and url-toolkit dependencies that were required by v0.14.17 but not v1.6.15
xmodule/js/spec/video/video_player_spec.js Updated test spies to use Hls instead of HLS, but introduced problematic ES6 imports that conflict with RequireJS pattern
Comments suppressed due to low confidence (2)

xmodule/js/spec/video/video_player_spec.js:3

  • Unused import VideoPlayer.
import VideoPlayer from "../../../assets/video/public/js/03_video_player.js";

xmodule/js/spec/video/video_player_spec.js:1104

}(require, define));

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot AI review requested due to automatic review settings December 8, 2025 10:18
@pganesh-apphelix
Copy link

@papphelix Is local testing possible, or do we need to verify this in staging instead?

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 4 out of 5 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (1)

xmodule/js/spec/video/video_player_spec.js:1100

}(require, define));

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@papphelix
Copy link
Member Author

@papphelix Is local testing possible, or do we need to verify this in staging instead?

Local testing was not working. I tried local file storage to upload a file and test, I had tried remote sample mp4 also a stream m3u8 file, but error happens loading the video player. So to quick fix i have raised this to stage and test. I am also checking how to fix this in local setup.

@papphelix papphelix merged commit 0a400fb into edx:release-ulmo Dec 8, 2025
64 checks passed
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