Conversation
script.js: - Fix undefined `crossOrigin` variable reference (should be `settings.crossOrigin`) video.js: - Fix unsafe regex match on data URLs (null check before array access) - Fix missing error parameter in videoElement.onerror callback - Normalize import path (./../../ → ../../) fontface.js: - Fix missing error parameter in font.load() rejection handler - Fix incomplete JSDoc example (missing opening bracket) Tests: - Add onerror tests for invalid image, JSON, and binary sources - Add inline TMX data return value test (covers tmx.js fix from prior commit) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes several correctness issues in the asset loader parser implementations, improving error propagation and handling of edge cases (notably data URLs), and adds regression tests to ensure loader error callbacks fire reliably.
Changes:
- Fix
scriptparser CORS handling by usingsettings.crossOrigin(instead of an undeclared variable). - Harden
videoparser data URL MIME parsing (null-safe) and pass the event/error argument throughvideoElement.onerror. - Fix
fontfaceparser rejection handler to forward the rejection error and correct a broken JSDoc example. - Add loader tests covering
onerrorbehavior for invalid sources and inline TMX resource counting.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| packages/melonjs/tests/loader.spec.js | Adds regression tests for onerror callbacks (image/json/binary) and inline TMX load() return count. |
| packages/melonjs/src/loader/parsers/video.js | Normalizes import path, avoids unsafe regex match access for data URLs, and forwards the onerror argument. |
| packages/melonjs/src/loader/parsers/script.js | Correctly applies crossOrigin from loader-provided settings. |
| packages/melonjs/src/loader/parsers/fontface.js | Forwards the actual font loading rejection error to onerror and fixes invalid JSDoc example syntax. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
crossOriginvariable reference — was checking an undeclared variable instead ofsettings.crossOriginvideoElement.onerrorcallback, normalize import pathfont.load()rejection handler, fix JSDoc example syntaxTest plan
🤖 Generated with Claude Code